-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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: escape bind-like strings in virtual table query #17111
Conversation
bcc066b
to
7a8f182
Compare
7a8f182
to
8f9c411
Compare
Codecov Report
@@ Coverage Diff @@
## master #17111 +/- ##
==========================================
- Coverage 76.88% 76.67% -0.22%
==========================================
Files 1031 1031
Lines 55197 55209 +12
Branches 7506 7506
==========================================
- Hits 42436 42329 -107
- Misses 12509 12628 +119
Partials 252 252
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Unfortunate edge case
(cherry picked from commit 434b576)
SUMMARY
When executing a chart query with a virtual table that contains a literal value with a colon suffixed by text characters, SQLAlchemy interprets these as being bind parameters when wrapped in a
text
element and compiled. This PR ensures that these are escaped using the same pattern that SQLAlchemy uses to detect bind parameters (the regex is available as a private variable inTextClause
). Relevant test cases are added (notice that the Postgres cast to::TIMESTAMP
is not escaped, as SQLAlchemy doesn't consider it to be a bind parameter). To my knowledge there is no cleaner way to do this, and I also found several threads where the author of SQLAlchemy instructed people to solve this issue by mechanically escaping the colon in the statement, which is what this proposal in practice does.Closes #17098
When executing the query
SELECT ':00 :abc' as abc, 123 as num
in SQL Lab, the query executes succesfully:BEFORE
When used as a virtual table in a chart query, SQLAlchemy interprets there to be two bind parameters present, namely
00
andabc
, resulting in the following error:AFTER
After escaping the bind-like strings, the query renders as expected.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION