-
Notifications
You must be signed in to change notification settings - Fork 71
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(targets): Quote column names in INSERT statement #2198
Conversation
singer_sdk/sinks/sql.py
Outdated
@@ -282,7 +282,7 @@ def generate_insert_statement( | |||
statement = dedent( | |||
f"""\ | |||
INSERT INTO {full_table_name} | |||
({", ".join(property_names)}) | |||
({", ".join([f'"{name}"' for name in property_names])}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not typically a python developer, but that change looks to be easy enough. I'll make sure to get those tests fixed up too.
CodSpeed Performance ReportMerging #2198 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2198 +/- ##
=======================================
Coverage 88.43% 88.44%
=======================================
Files 54 54
Lines 4714 4715 +1
Branches 961 961
=======================================
+ Hits 4169 4170 +1
Misses 384 384
Partials 161 161 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mattwill09!
You may want to update the test in
sdk/tests/samples/test_target_sqlite.py
Lines 490 to 495 in af3b678
dedent( | |
"""\ | |
INSERT INTO test_stream | |
(id, name) | |
VALUES (:id, :name)""", | |
), |
* Update sql.py * Update test_target_sqlite.py
@@ -282,7 +283,7 @@ def generate_insert_statement( | |||
statement = dedent( | |||
f"""\ | |||
INSERT INTO {full_table_name} | |||
({", ".join(property_names)}) | |||
({", ".join(quoted_name(name, True) for name in property_names)}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
({", ".join(quoted_name(name, True) for name in property_names)}) | |
({", ".join(quoted_name(name, True) for name in property_names)}) # noqa: FBT003 |
Superseded by #2200. Thanks @mattwill09! |
Thanks for finding a proper solution for this. Much appreciated! |
This is an update to fix how the insert into statements are being generated. If you have a source column that uses a reserved word then invalid SQL was being generated.
Previous:
Updated:
📚 Documentation preview 📚: https://meltano-sdk--2198.org.readthedocs.build/en/2198/