Split keywords into reserved and non-reserved sets#282
Merged
Conversation
Revives #275 rebased onto current master (post-#276/#277 renames) and addresses both review regressions. matchTokenKind previously coerced every keyword token to match TokenKindIdent, so any keyword could silently fill an identifier slot: `SELECT a FROM WHERE b = 1` parsed as `SELECT a AS FROM WHERE b = 1`. - New reservedKeywords set: statement/clause starters and expression operators no longer match identifier positions, so a missing name fails fast at the clause keyword. All other keywords stay usable as bare identifiers. - parseAnyKeyword accepts any keyword - reserved or not - in positions where context has already proven the token is a name: after AS, after a dot in a qualified name, or a lookahead-disambiguated select item. Review fixes on top of the original branch: - Operator keywords stay callable as ClickHouse functions when followed by '(': and(a, b), or(a, b), in(1, [1]), like(s, p), ilike(s, p). parseIdentOrFunction now accepts a keyword name in that case. - Reserved keywords are accepted as aliases after AS in expression lists (`SELECT (1 AS from)`, `sum(x AS from)`), ORDER BY (`ORDER BY x AS from`) and non-parenthesized CTEs (`WITH 1 AS from SELECT from`), per review suggestions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
git-hulk
commented
Jul 5, 2026
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f08bae41f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
git-hulk
commented
Jul 5, 2026
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.
Summary
Revives #275, rebased onto current master (the #276/#277 accessor renames caused the old branch to no longer apply), with both review regressions fixed.
matchTokenKindpreviously coerced every keyword token to matchTokenKindIdent, so any keyword could silently fill an identifier slot:SELECT a FROM WHERE b = 1SELECT a AS FROM WHERE b = 1WHERE: expected table nameSELECT NOT NOT aNOT (column \"NOT\") AS aSELECT 1 AS interval,SELECT a, from, b FROM tReview fixes from #275
Both regressions flagged in the review are addressed, with regression tests:
parser/keyword.goreview thread):and(a, b),or(a, b),in(1, [1]),like(s, p),ilike(s, p)all parse again —parseIdentOrFunctionaccepts a keyword name when the next token is(. Keywords with dedicated syntax (CAST, CASE, EXTRACT, INTERVAL...) keep their own paths, which run first.SELECT FROM (SELECT 1)behaves identically to master (verified against a master build).SELECT (1 AS from),sum(x AS from)),ORDER BY x AS from, and non-parenthesized CTEs (WITH 1 AS from SELECT from) all useparseAnyKeywordnow, exactly as suggested.Design
reservedKeywords: statement/clause starters and expression operators — these no longer match identifier positions, so a missing name fails fast instead of swallowing the clause keyword. Everything else (DATE, KEY, FIRST, ID, ...) stays usable as a bare identifier.parseAnyKeyword: for positions where context has proven the token is a name (after AS, after a dot, disambiguated select items) — accepts reserved keywords too.matchTokenKindis variadic:matchTokenKind(TokenKindIdent, TokenKindKeyword)where any keyword is acceptable.No golden fixture changed;
make testpasses.Closes the follow-up promised in #275.
🤖 Generated with Claude Code