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.

Sign up for GitHub

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

Jump to bottom

Qualification Tool add test for SQL Description escaping commas for csv #6120

Merged
merged 1 commit into from
Jul 27, 2022
Merged

Qualification Tool add test for SQL Description escaping commas for csv #6120

merged 1 commit into from
Jul 27, 2022

Conversation

tgravescs
Copy link
Collaborator

@tgravescs tgravescs commented Jul 27, 2022 •
edited
Loading

Qualification tool per sql output option - adds an extra test for sql description being properly escaped for CSV output.

adds to #5502
previous PR for implementation: #6092

I tested with the fix to escape and then without, the test fails without the fix to escape sql description.

Signed-off-by: Thomas Graves tgraves@nvidia.com

@tgravescs
Add test for SQL Description escaping commas for csv
24ad6c8 
Signed-off-by: Thomas Graves <tgraves@nvidia.com>
@tgravescs tgravescs added the test Only impacts tests label Jul 27, 2022
@tgravescs tgravescs added this to the July 22 - Aug 5 milestone Jul 27, 2022
@tgravescs tgravescs self-assigned this Jul 27, 2022
@tgravescs
Copy link
Collaborator Author

build

@tgravescs tgravescs changed the title Add test for SQL Description escaping commas for csv Qualification Tool add test for SQL Description escaping commas for csv Jul 27, 2022
amahussein
amahussein approved these changes Jul 27, 2022
View reviewed changes
Copy link
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

This Looks good.
I remember we have csv test files that check nestedTypes and readSchema. If those are not covering the delimeter's substitution, then we can file a followup later to create similar tests for rapids_4_spark_qualification_output.csv

@tgravescs tgravescs merged commit 85d29a2 into NVIDIA:branch-22.08 Jul 27, 2022
@tgravescs tgravescs deleted the sqldesctest branch July 27, 2022 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amahussein amahussein amahussein approved these changes

Assignees

@tgravescs tgravescs

Labels
test Only impacts tests
Projects
None yet
Milestone
July 22 - Aug 5
Development

Successfully merging this pull request may close these issues.
2 participants
@tgravescs @amahussein