Skip to content

bug: 89 keywords are reserved that SQLite treats as identifiers (parse.y's %fallback) — blocks SQE's 'key' column #696

Description

@dpsiderius

Description

SQLite treats most of its keywords as non-reserved: they act as keywords
where the grammar expects one and as ordinary identifiers everywhere else. We
reserve all 146 words in our keyword table unconditionally, so any of them
used as a column, table or alias name is a syntax error.

This blocks the SQE integration outright, independently of the embedding
API.
SQE's iceberg_namespace_properties maps a namespace property key to
a value. Measured against the pinned 3.53.4 oracle:

$ sqlite3 o.db "CREATE TABLE p(namespace TEXT, key TEXT, value TEXT,
                               PRIMARY KEY(namespace, key));"
                                                    # created

$ sqlite-rs exec m.db "CREATE TABLE p(namespace TEXT, key TEXT, value TEXT,
                                      PRIMARY KEY(namespace, key))"
error: Invalid { message: "expected identifier, found Keyword(KEY)",
                 span: Span { line: 1, column: 32, offset: 31, len: 3 } }

We cannot create that table, and cannot read one the oracle created. No amount
of facade work (#695) changes that.

Scope: 89 of our 146 keywords

Tested every word in src/parser/tokenizer.rs's keyword table as a column
name, in both engines:

count
oracle accepts, we accept 0
oracle rejects, we reject 57
oracle accepts, we reject 89

Every keyword we tokenize is fully reserved. SQLite reserves 57.

ABORT ACTION AFTER ALWAYS ANALYZE ASC ATTACH BEFORE BEGIN BY CASCADE CAST
COLUMN CONFLICT CROSS CURRENT CURRENT_DATE CURRENT_TIME CURRENT_TIMESTAMP
DATABASE DEFERRED DESC DETACH DO EACH END EXCLUDE EXCLUSIVE EXPLAIN FAIL
FILTER FIRST FOLLOWING FOR FULL GENERATED GLOB GROUPS IF IGNORE IMMEDIATE
INDEXED INITIALLY INNER INSTEAD KEY LAST LEFT LIKE MATCH MATERIALIZED
NATURAL NO NULLS OF OFFSET OTHERS OUTER OVER PARTITION PLAN PRAGMA
PRECEDING QUERY RAISE RANGE RECURSIVE REGEXP REINDEX RELEASE RENAME
REPLACE RESTRICT RIGHT ROLLBACK ROW ROWS SAVEPOINT TEMP TEMPORARY TIES
TRIGGER UNBOUNDED VACUUM VIEW VIRTUAL WINDOW WITH WITHOUT

KEY, VALUE(-adjacent), MATCH, FIRST, LAST, ROW, RANGE, FILTER
— these are ordinary column names in real schemas, not exotic edge cases.

The mechanism is explicit in the pinned grammar

This is not something to infer. parse.y:272 (3.53.4, the pinned oracle)
declares it directly:

%fallback ID
  ABORT ACTION AFTER ANALYZE ASC ATTACH BEFORE BEGIN BY CASCADE CAST COLUMNKW
  CONFLICT DATABASE DEFERRED DESC DETACH DO
  EACH END EXCLUSIVE EXPLAIN FAIL FOR
  IGNORE IMMEDIATE INITIALLY INSTEAD LIKE_KW MATCH NO PLAN
  QUERY KEY OF OFFSET PRAGMA RAISE RECURSIVE RELEASE REPLACE RESTRICT ROW ROWS
  ROLLBACK SAVEPOINT TEMP TRIGGER VACUUM VIEW VIRTUAL WITH WITHOUT
  NULLS FIRST LAST
  ...

Lemon's %fallback means: when the parser cannot shift this token, retry it
as ID. 85 tokens are listed (more under %ifndef SQLITE_OMIT_WINDOWFUNC
etc.), which matches the 89 measured above once token aliases like
COLUMNKW/LIKE_KW are expanded.

Our tokenizer has no equivalent concept — ("KEY", Keyword::KEY)
(tokenizer.rs:523) is unconditional.

Two ways to fix it, and they are not equivalent

A. Fallback at the parser. Mirror Lemon: where a rule wants an identifier,
accept a fallback-eligible keyword token too. Faithful to SQLite, because it
keeps the word working as a keyword in keyword position — SELECT key FROM t ORDER BY key and PRIMARY KEY(a) both keep working in the same statement.
Touches every identifier-accepting production.

B. Fallback at the tokenizer. Emit Identifier instead of Keyword for
the 89. Much smaller, and wrong: PRIMARY KEY(a) would stop parsing, because
KEY there is a keyword.

So A, despite the size. The grammar convention applies —
.openspec/grammar/sqlite.ebnf needs the fallback set expressed, with
make check-grammar-drift clean afterwards.

Non-goals

  • Quoted identifiers ("key", [key], `key`) already work and are not
    affected.
  • The 57 genuinely-reserved keywords stay reserved. CREATE TABLE t(select)
    must keep failing, as it does in the oracle.
  • Making sqlite.ebnf model Lemon's conflict resolution generally. Only the
    fallback set.

Acceptance Criteria

  • All 89 words above work as column names, table names and aliases, with
    the same statements accepted as the pinned 3.53.4 oracle accepts
  • The 57 reserved words still fail, matching the oracle
  • SQE's actual table works end to end: CREATE TABLE p(namespace TEXT, key TEXT, value TEXT, PRIMARY KEY(namespace, key)),
    then INSERT, then SELECT key, value FROM p — byte-identical to the
    oracle
  • A keyword used as both in one statement still parses:
    SELECT key FROM t ORDER BY key and a table declared with
    PRIMARY KEY(key)
  • Corpus test enumerating the fallback set against the oracle, so the
    set cannot silently drift from parse.y
  • sqlite.ebnf expresses the fallback set with a [parse.y:272 fallback]
    origin annotation; make check-grammar-drift clean
  • Full suite green, make lint both clippy passes, make assurance with
    no dead links

Complexity

Estimate: medium
Reasoning: The set is not guesswork — parse.y:272 enumerates it and the
89 are measured, so there is no discovery left. The work is threading
fallback acceptance through the parser's identifier positions (option A), plus
the grammar annotation. The risk is regression in keyword position — a
fallback that is too eager turns PRIMARY KEY(a) or ORDER BY x DESC into a
parse of something else, which is what the both-in-one-statement criterion
exists to catch.

Found while writing tests for #695: a test using AS first failed, which
turned out not to be a bad test.

Refs: 002/Req-2, #678, #695

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions