Skip to content

Don't match the time zone literal as part of the TZCast keyword - #876

Open
dngr2 wants to merge 1 commit into
andialbrecht:masterfrom
dngr2:fix/keyword-case-tz-literal
Open

Don't match the time zone literal as part of the TZCast keyword#876
dngr2 wants to merge 1 commit into
andialbrecht:masterfrom
dngr2:fix/keyword-case-tz-literal

Conversation

@dngr2

@dngr2 dngr2 commented Aug 16, 2026

Copy link
Copy Markdown

keyword_case rewrites the contents of a time zone literal:

>>> sqlparse.format("SELECT ts AT TIME ZONE 'Asia/Tokyo'", keyword_case='upper')
"SELECT ts AT TIME ZONE 'ASIA/TOKYO'"

The cause is in the TZCast rule, which matches the literal as part of the keyword:

(r"(AT|WITH')\s+TIME\s+ZONE\s+'[^']+'", tokens.Keyword.TZCast),

"AT TIME ZONE 'Asia/Tokyo'" therefore arrives at KeywordCaseFilter as a single Token.Keyword and 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, so AT TIME ZONE 'a''b' is 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 fires. TIMESTAMP WITH TIME ZONE is 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:

(r"AT\s+TIME\s+ZONE\b", tokens.Keyword.TZCast),

Nothing downstream needed adjusting. group_tzcasts already accepts a following String.Single in valid_next, so the grouping layer was written expecting this token stream: ts AT TIME ZONE 'Asia/Tokyo' still groups into a single Identifier, with aliases and nested calls intact.

I took the narrower route first — teaching KeywordCaseFilter to 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 xpassed
  • Restoring the old regex fails 5 of the new assertions
  • 984 statements from sqlglot's fixture corpus: no lossy round trips, and no literal altered by keyword_case in either direction (before the change, one was)
  • TIMESTAMP WITH TIME ZONE and TIMESTAMP WITHOUT TIME ZONE confirmed to tokenize exactly as before
  • bench_lexer_delimiters, bench_grouping and bench_group_comments all still report linear
  • ruff check sqlparse/ reports nothing new

One thing worth a maintainer's eye: test_parse_tzcast asserted len(p.tokens) == 1 for a bare AT 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 one Identifier when there is an operand. Happy to restore it in some other form if you would rather keep it.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant