Fix named_parameters when string literals contain comment markers#2783
Open
JSap0914 wants to merge 1 commit into
Open
Fix named_parameters when string literals contain comment markers#2783JSap0914 wants to merge 1 commit into
JSap0914 wants to merge 1 commit into
Conversation
named_parameters stripped SQL comments before string literals in
separate passes. A string literal such as '-- TODO' would be treated
as the start of a line comment, swallowing the rest of the line and
hiding any named parameters that followed it. For example:
select * from t where note = '-- TODO' and id = :id
returned [] instead of ['id'], so the query parameter input form
would be missing the :id field.
Match comments and string literals in a single left-to-right pass so
that whichever construct starts first wins, matching how SQL is
actually tokenized.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
named_parameters()(and thereforederive_named_parameters()) stripped SQL comments before string literals, in separate regex passes. A string literal that contains a comment marker is then mistaken for the start of a comment.For example:
returned
[]instead of["id"]— the--inside the'-- TODO'string literal was treated as the start of a line comment, which swallowed the rest of the line and hid the:idparameter. In the query UI this means the parameter input form silently loses fields for any query whose string literals contain--or/* */.Fix
Match comments and string literals in a single left-to-right pass so that whichever construct starts first wins — this is how SQL is actually tokenized, so a
--inside a string is part of the string, and a quote inside a comment is part of the comment. This also keeps the existing behaviour of ignoring parameters that appear inside comments or string literals.Verification
Result:
22 passed. New parametrized cases cover string literals containing--//* */, parameters inside comments, and parameters inside string literals. The fulltests/test_utils.py(213 tests) and the query-page HTML tests still pass.