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

Fix handling of single quotes in RedshiftToS3Operator #35986

Merged
merged 5 commits into from
Dec 4, 2023

Conversation

vincbeck
Copy link
Contributor

Continue the work started in #32915.
Closes: #30287.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Ashwaq Reheman and others added 4 commits November 30, 2023 13:41
This test case ensures that the RedshiftToS3Operator correctly handles a custom select_query containing single quotes. It validates that the operator generates a proper UNLOAD query that handles single quotes, thereby confirming its functionality in such scenarios.
@boring-cyborg boring-cyborg bot added area:providers provider:amazon-aws AWS/Amazon - related issues labels Nov 30, 2023
@o-nikolas
Copy link
Contributor

CC @cjames23

Copy link
Contributor

@cjames23 cjames23 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@@ -141,8 +141,10 @@ def __init__(
def _build_unload_query(
self, credentials_block: str, select_query: str, s3_key: str, unload_options: str
) -> str:
# Un-escape already escaped queries
select_query = select_query.replace("''", "'")
Copy link
Member

Choose a reason for hiding this comment

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

Would this break empty string literals…?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ouh! That's a good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I addressed it in 7d06f1e259a695fc68e16e50e9562f514dbb757d. I also added a unit test for this use case

@vincbeck vincbeck merged commit 04a7816 into apache:main Dec 4, 2023
47 checks passed
@vincbeck vincbeck deleted the vincbeck/quotes branch December 4, 2023 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon-aws AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RedshiftToS3 Operator Wrapping Query in Quotes Instead of $$
6 participants