Don't match the time zone literal as part of the TZCast keyword - #876
Open
dngr2 wants to merge 1 commit into
Open
Don't match the time zone literal as part of the TZCast keyword#876dngr2 wants to merge 1 commit into
dngr2 wants to merge 1 commit into
Conversation
The rule was
(r"(AT|WITH')\s+TIME\s+ZONE\s+'[^']+'", tokens.Keyword.TZCast)
so "AT TIME ZONE 'Asia/Tokyo'" arrived as a single Token.Keyword with the
literal inside it. Three things followed from that.
The literal was recased along with the keyword, which changes data the caller
only asked to have reformatted:
>>> sqlparse.format("SELECT ts AT TIME ZONE 'Asia/Tokyo'", keyword_case='upper')
"SELECT ts AT TIME ZONE 'ASIA/TOKYO'"
'[^']+' cannot express a doubled quote, so AT TIME ZONE 'a''b' was split into
a TZCast holding "AT TIME ZONE 'a'" and a stray String token "'b'".
The WITH' alternative matches an apostrophe directly after WITH, which is not
valid SQL anywhere, so that branch never fired. TIMESTAMP WITH TIME ZONE is
tokenized as separate keywords and stays that way.
Match only the keyword and leave the literal to the string rule, which already
handles doubled quotes. group_tzcasts already accepts a following String.Single
in valid_next, so "ts AT TIME ZONE 'Asia/Tokyo'" still groups into one
Identifier, aliases and nested calls included.
The existing test used 'UTC', which reads the same in either case and contains
no quote, so it passed either way.
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.
keyword_caserewrites the contents of a time zone literal:The cause is in the TZCast rule, which matches the literal as part of the keyword:
"AT TIME ZONE 'Asia/Tokyo'"therefore arrives atKeywordCaseFilteras a singleToken.Keywordand is recased whole, changing data the caller only asked to have reformatted.Two more things follow from the same pattern:
'[^']+'cannot express a doubled quote, soAT TIME ZONE 'a''b'is split into a TZCast holding"AT TIME ZONE 'a'"and a strayStringtoken"'b'".WITH'alternative matches an apostrophe directly afterWITH, which is not valid SQL anywhere, so that branch never fires.TIMESTAMP WITH TIME ZONEis tokenized as separate keywords and is unaffected either way.The change
Match only the keyword and leave the literal to the string rule, which already handles doubled quotes:
Nothing downstream needed adjusting.
group_tzcastsalready accepts a followingString.Singleinvalid_next, so the grouping layer was written expecting this token stream:ts AT TIME ZONE 'Asia/Tokyo'still groups into a singleIdentifier, with aliases and nested calls intact.I took the narrower route first — teaching
KeywordCaseFilterto skip the quoted part — and discarded it. It works around a token the filter should never receive and leaves the other two problems standing.Verification
The existing coverage for this rule is
'UTC', which reads the same in either case and holds no quote, so it passes whether or not the literal is rewritten. The new cases use'Asia/Tokyo'and'a''b'.510 passed, 2 xfailed, 1 xpassedkeyword_casein either direction (before the change, one was)TIMESTAMP WITH TIME ZONEandTIMESTAMP WITHOUT TIME ZONEconfirmed to tokenize exactly as beforebench_lexer_delimiters,bench_groupingandbench_group_commentsall still report linearruff check sqlparse/reports nothing newOne thing worth a maintainer's eye:
test_parse_tzcastassertedlen(p.tokens) == 1for a bareAT TIME ZONE 'UTC'. That encodes the old tokenization rather than a behaviour to preserve, and the bare form is not a valid statement on its own, so I replaced it with assertions on the token types and the round trip, plus a new test that grouping still yields oneIdentifierwhen there is an operand. Happy to restore it in some other form if you would rather keep it.