Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop rows that fail validation #62

Merged
merged 2 commits into from Dec 1, 2022
Merged

Drop rows that fail validation #62

merged 2 commits into from Dec 1, 2022

Conversation

felix-oq
Copy link
Contributor

@felix-oq felix-oq commented Nov 29, 2022

Part of #58

Changes the implementation of LayoutValidator such that rows are dropped if they fail validation. To achieve this, the implementation was changed to a row-wise instead of a section-wise validation.

I also changed the grammar so that row sections can only be used to describe CSV headers because it simplified the implementation and I could not think of a reasonable use case where that would be needed.

Additionally I surrounded column names with " in the generated SQL statements which solves the error mentioned in #58.

The gas example now runs successfully while logging any validation errors that occurred. The resulting DB table only contains the first non-header row of the CSV file because all other rows miss some values and thus fail validation.

Changes the grammar such that row sections always describe headers.
@felix-oq felix-oq requested a review from rhazn November 29, 2022 15:47
Copy link
Contributor

@rhazn rhazn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how I feel about the more restrictive solution regarding rows... I am sure there is data out there where this will be an issue. But I think we can iterate on it when we actually encounter that data.

The other changes are great of course 👍 .

@felix-oq felix-oq merged commit a621fec into main Dec 1, 2022
@felix-oq felix-oq deleted the #58-drop-invalid-rows branch December 1, 2022 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants