fix: 89 non-reserved keywords now work as identifiers (#696) - #714
Open
dpsiderius wants to merge 1 commit into
Open
dpsiderius wants to merge 1 commit into
dpsiderius wants to merge 1 commit into
Conversation
…2 fallback (#696) We reserved all 146 keywords unconditionally; real SQLite's `parse.y:272` `%fallback ID` declares 89 of them non-reserved, retrying them as a plain ID wherever the grammar can't shift them as the keyword itself. That blocked schemas as ordinary as `CREATE TABLE p(namespace TEXT, key TEXT, value TEXT, PRIMARY KEY(namespace, key))` -- SQE's actual table. `parser::grammar`'s `identifier()` is the single choke point nearly every identifier-accepting production already funneled through (table/column names, PRIMARY KEY column lists, USING columns, aliases via `AS`, ...), so threading fallback acceptance through it is one change plus a mirror in `primary_expr`'s bare-column-reference arm (needed separately since that arm matches on `TokenKind` directly rather than calling `identifier()`). `tokenizer::is_fallback_keyword`/`keyword_text` hold the 89-word set and its reverse lookup back to source text. Deliberately not extended to a bare (no-`AS`) alias: doing so regressed `SELECT * FROM a NATURAL JOIN b` and `t LEFT JOIN (...)` by swallowing NATURAL/LEFT as a bare alias for the preceding table -- real SQLite resolves that via LALR shift/reduce precedence this recursive-descent parser has no equivalent for. `AS <fallback-keyword>` is the safe, unambiguous form and is what the ticket requires. tests/corpus/fallback_keyword_test.rs diffs all 89 fallback words and all 57 still-reserved words against the pinned oracle as column names, plus the SQE table end-to-end, the both-keyword-and-identifier-in-one-statement case (`PRIMARY KEY(key)` + `ORDER BY key`), and `AS first`. tests/corpus/extracted_sql_test.rs's SELECT_INVALID_BASELINE ratchet drops 2 -> 1: one previously-misclassified extracted SELECT now parses. .openspec/grammar/sqlite.ebnf's `identifier` rule gains the `fallback-keyword` alternative, annotated `[parse.y:272 fallback]`; `make check-grammar-drift` clean (the annotation isn't a Lemon nonterminal, so it isn't part of the rule-name coverage count, same as the file's existing `%token_class`/`%fallback` header note). Refs: 002/Req-2 spend: within the "medium" estimate.
7 tasks
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
1 participant
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.
Stacked on #698 (fix/698-leading-trailing-comments) — merge that first.
What
parse.y's
%fallbackset: 89 of our 146 reserved keywords are non-reservedin SQLite — they act as keywords where the grammar expects one and as
ordinary identifiers everywhere else. Threaded fallback acceptance through
identifier()(the single choke point) and the bare column-reference armof
primary_expr.Regression found and fixed during development: extending bare
(no-
AS) alias parsing to fallback keywords brokea NATURAL JOIN b/t LEFT JOIN (...)—NATURAL/LEFTgot swallowed as an alias. Revertedthat part;
AS <fallback-keyword>is the safe form and is what SQLite'sgrammar actually allows.
Oracle-measured
CREATE TABLE p(namespace TEXT, key TEXT, value TEXT, PRIMARY KEY(namespace, key))+ INSERT + SELECT — byte-identicalto the oracle.
(
PRIMARY KEY(key)+ORDER BY key) passes..openspec/grammar/sqlite.ebnf'sidentifierrule gained thefallback-keyword alternative,
[parse.y:272 fallback]annotated;make check-grammar-driftpasses.Bonus: fixed one previously-misclassified extracted SELECT baseline
(
SELECT_INVALID_BASELINEratcheted 2→1).Test plan
tests/corpus/— 5 new fallback-keyword tests, all oracle-diffed.make test,make lint,make test-corpus,make check-grammar-drift,make check-mvl-limit,make check-mod-files,make assuranceall pass.Refs: 002/Req-2, #678, #695
Closes #696
spend: matched estimate