Fix comment marker swallowed by preceding operator run - #878
Open
mayuriphad wants to merge 1 commit into
Open
Conversation
The operator regex [+/@#%^&|^-]+ greedily consumed a directly adjacent -- (or # ) comment marker, e.g. `||--comment` tokenized as a single Operator token instead of `||` followed by a Comment.Single token. This caused strip_comments to leave such comments in place, and could make sqlparse.split() misjudge statement boundaries when the comment contained a `;`. Add a negative lookahead so the operator regex stops before a comment marker, letting the comment regex handle it as before. Fixesandialbrecht#722
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 freeto 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
Fixes#722.
sqlparse's operator regex ([+/@#%^&|^-]+insqlparse/keywords.py) isgreedy and includes
-in its character class. When a--(or#)single-line comment marker immediately follows an operator with no
intervening whitespace, e.g.
||--comment, the operator regex consumes thewhole
||--run as a singleOperatortoken before the comment regex evergets a chance to match. The comment is then invisible to the tokenizer.
Repro:
This can also make
sqlparse.split()misjudge statement boundaries: a;that lives inside an un-recognized "comment" gets treated as a real
statement terminator only in other contexts, but more directly, comment
content that should have been dropped is not.
Fix
Add a negative lookahead to the operator regex so it stops consuming
characters right before a
--or#comment marker, letting the earliercomment regex handle it as intended.
Test plan
Added regression tests:
tests/test_format.py::TestFormat::test_strip_comments_single_no_space_before_operatortests/test_split.py::test_split_comment_directly_after_operatorBoth fail on
masterand pass with the fix. Ran the full test suitelocally:
Also ran
ruff check sqlparse/keywords.py— clean (pre-existing lintfindings elsewhere in the repo are unrelated to this change).