Skip to content

fix(grammar): stop flagging valid SQLite migrations as partially parsed - #2957

Merged
DeusData merged 2 commits into
mainfrom
distill/2246-sqlite-grammar
Nov 12, 2026
Merged

DeusData merged 2 commits into
mainfrom
distill/2246-sqlite-grammar

Conversation

@DeusData

Copy link
Copy Markdown
Owner

What

Valid SQLite migrations were reported as partially parsed. The vendored SQL
grammar (DerekStride/tree-sitter-sql @ 851e9cb257ba) rejects SQLite syntax
that real migration files use, so each use became an ERROR node and the file
was flagged parse_partial:

  • PRAGMA foreign_keys = ON;
  • id INTEGER PRIMARY KEY AUTOINCREMENT
  • a trigger body: CREATE TRIGGER ... BEGIN ... END;, also after an
    unparenthesized WHEN condition, with INSERT OR IGNORE inside
  • a column named key after the first column
  • ) WITHOUT ROWID;
  • CREATE VIRTUAL TABLE messages_fts USING fts5(...)

IF NOT EXISTS on a trigger or an index was already accepted upstream; in the
#2246 reproduction the failing lines were the PRAGMA, the AUTOINCREMENT and the
trigger body.

The key case is lexical: after a comma inside a column list the grammar also
accepts MySQL's KEY name (cols) index, so key lexes as that keyword there
and key TEXT NOT NULL became a malformed index.

How

sql becomes a self-maintained fork of the pin we already vendored, on the
same model as tools/tree-sitter-pascal/:

  • tools/tree-sitter-sql/ holds upstream's grammar.js + grammar/
    (blob-identical to the pin and to upstream main @ 3c99273f9b32, where its
    corpus lives), upstream's MIT LICENSE, and one patch,
    patches/sqlite-dialect.patch, which touches grammar/ only. Its index
    lines carry upstream's pre-image blob hashes.
  • AUTOINCREMENT is upstream's own change (c042cde1e2a6, 2026-11-08, after
    our pin), taken verbatim, so that hunk disappears on the next refresh.
  • The trigger body is the existing block node. Precedence keeps Postgres
    WHEN (...) EXECUTE FUNCTION trees unchanged.
  • The virtual table is aliased to create_table, so it is extracted as a
    Table like any other.
  • Upstream node types keep their shape. Node types only gain entries
    (pragma_statement, module_argument, four keywords; create_table gains a
    module field).
  • The vendored scanner.c is unchanged. It carries our start_tag leak fix
    (0e61867), which is now listed in the MANIFEST's "Local source patches"
    table; upstream fixed the same leak after our pin (ee2f84f9b857).
  • One trade-off, documented in the MANIFEST: since key can now start a
    column, the type keywords that take no arguments (name, date, text,
    uuid, ...) are accepted as MySQL index names so KEY name (name) stays an
    index. An unquoted MySQL index named after a type that takes arguments
    (KEY timestamp (ts)) is now an ERROR; backtick-quoted names, which
    mysqldump writes, are unaffected.

Registered like pascal: MANIFEST.md fork rows and notes (ABI tally recounted,
unchanged), the provenance audit's FORKS map, THIRD_PARTY.md, and the
vendored checksums.

Provenance

  • Without the patch, tree-sitter-cli 0.25.10 regenerates upstream's shipped
    parser.c byte-for-byte except line 1 (upstream's generator stamp carries no
    version); grammar.json, node-types.json and tree_sitter/*.h are
    byte-identical. The previously vendored files were upstream's minus the final
    newline.
  • Patched: three generations are byte-identical (parser.c SHA-256
    3c64e5c415d00f7d935b7f9f0516f7be04801b241bc5da2e5e71ca31cf337887), ABI 15,
    states 30,622 -> 31,081, symbols 770 -> 782, 0 non-ASCII bytes, no string
    literal containing a space, one comment (the generator stamp), and the same
    identifiers in call position as before.
  • tree-sitter test: upstream's 511 corpus cases pass unchanged.
    test/corpus/sqlite.txt adds 11 cases; the 9 SQLite cases fail on the
    unpatched grammar and the 2 controls pass on both.

Verification

Real migration sets, each indexed from scratch with an isolated cache, main
6d34ca15 vs this branch:

corpus parse_partial graph
dani-garcia/vaultwarden @ 58a980f8, migrations/sqlite (116 files) 3 -> 0 identical (303 nodes, 446 edges)
atuinsh/atuin @ 2535161e, *.sql (69 files) 19 -> 7 +2 Table nodes, +2 DEFINES, +3 USAGE
helm @ 9b88efd6 (control, no SQL) 84 -> 84 identical (12,251 nodes, 39,561 edges)
  • All 15 valid SQLite files are clean.
  • The 7 atuin files still flagged are the invalid ALTER TABLE ... DROP INDEX
    down migration (SQLite has no such statement) and the 2 MySQL and 4 Postgres
    migrations, with the same ranges as before. No file is newly flagged.
  • The new Table nodes are the fts5 table and a table that the old trigger error
    region swallowed.

Tests in tests/test_parse_coverage.c:

  • The two SQLite tests from fix: clear false partial-parse warnings for SQLite and TSX #2246, carried over as written:
    sqlite_valid_migration_not_flagged (red before this change) and the
    sqlite_malformed_still_flagged control.
  • New cases: key columns, WITHOUT ROWID, a WHEN trigger with
    INSERT OR IGNORE plus an fts5 table (asserted as a Table), PRAGMA forms.
  • Two new controls: MySQL's KEY index still clean, and
    ALTER TABLE ... DROP INDEX still flagged.
  • On main, 5 of these tests fail and the 3 controls pass. With this change,
    all pass.

Narrow suites under ASan/UBSan: parse_coverage, grammar_regression,
extraction, language, lang_contract, registry, grammar_labels, complexity,
grammar_probe_g and pipeline all pass (1,569 tests), with no sanitizer report.

Credit

Distilled from #2246 by @kerfern (Co-authored-by on the commit); the original
could not merge because its regenerated parser had no reproducible grammar
source and its commit has no DCO sign-off. Kerf's report and tests defined the
problem precisely, and the malformed-input control is theirs.

Follow-up

Offer the remaining SQLite forms to DerekStride/tree-sitter-sql so the local
patch can shrink on a later refresh. Upstream's AUTOINCREMENT request
(DerekStride/tree-sitter-sql#299) has since been closed by c042cde1e2a6,
which this fork already mirrors. Not opened yet.

Valid SQLite migration files were reported parse_partial because the
vendored SQL grammar (DerekStride/tree-sitter-sql @ 851e9cb257ba) rejects
SQLite syntax they use: PRAGMA, AUTOINCREMENT, a trigger's BEGIN ... END
body, a column named `key` after the first column, WITHOUT ROWID, an
unparenthesized trigger WHEN, INSERT OR IGNORE, and CREATE VIRTUAL TABLE
... USING fts5(...). Each became an ERROR node; the trigger's error region
in atuin's ai_session schema also swallowed the CREATE TABLE after it.

The `key` case is lexical: after a comma inside a column list the grammar
also accepts MySQL's `KEY name (cols)` index, so `key` lexes as that
keyword there and `key TEXT NOT NULL` became a malformed index.

Fix: sql becomes a self-maintained fork of the pin we already vendored,
following the pascal model. tools/tree-sitter-sql/ holds upstream's
grammar.js + grammar/ (blob-identical to the pin and to upstream main @
3c99273f9b32, where its corpus lives) and one patch,
patches/sqlite-dialect.patch, touching grammar/ only:
- PRAGMA [schema.]name [= value | (value)] (new pragma_statement)
- AUTOINCREMENT as keyword_auto_increment, upstream's own change
  (c042cde1e2a6, 2026-11-08, after our pin)
- `key` accepted as a column name; the type keywords that take no
  arguments are accepted as a MySQL index name so `KEY name (name)` stays
  an index
- WITHOUT ROWID
- a trigger body as the existing block node, after an optional
  unparenthesized WHEN (precedence keeps Postgres `WHEN (...) EXECUTE
  FUNCTION` trees unchanged)
- INSERT OR {ABORT | FAIL | IGNORE | REPLACE | ROLLBACK}
- CREATE VIRTUAL TABLE [IF NOT EXISTS] name USING module(args), aliased
  to create_table (new module_argument node), so the table is extracted
  as a Table like any other
Upstream node types keep their shape: upstream's 511 corpus cases pass
unchanged; test/corpus/sqlite.txt adds 11 (9 fail on the unpatched
grammar, 2 controls pass on both). The vendored scanner.c is kept as is
(it carries our start_tag leak fix 0e61867, now listed under the
MANIFEST's "Local source patches"; upstream fixed the same leak after our
pin in ee2f84f9b857).

Provenance: without the patch, tree-sitter-cli 0.25.10 regenerates
upstream's shipped parser.c byte-for-byte except line 1 (upstream's
generator stamp has no version), and grammar.json, node-types.json and
tree_sitter/*.h byte-identical. Patched: three generations are
byte-identical (parser.c SHA-256 3c64e5c4...7887); ABI 15; states 30,622
-> 31,081, symbols 770 -> 782; 0 non-ASCII bytes, no string literal with
a space, one comment, the same call-position identifiers as before. Node
types only gain entries (pragma_statement, module_argument, four
keywords; create_table gains a `module` field).

Proof, indexed from scratch with isolated caches (main 6d34ca1 vs this
change): dani-garcia/vaultwarden @ 58a980f8 migrations/sqlite (116
files) parse_partial 3 -> 0, graph identical (303 nodes, 446 edges);
atuinsh/atuin @ 2535161e *.sql (69 files) 19 -> 7: all 12 valid SQLite
files clean; the 7 left are the invalid `ALTER TABLE ... DROP INDEX`
down migration and the 2 MySQL + 4 Postgres files, same ranges as
before; graph +2 Table nodes (the fts5 table and the table the trigger
error swallowed), +2 DEFINES, +3 USAGE lineage edges. No newly flagged
file. Control: helm @ 9b88efd6 indexes to an identical graph (12,251
nodes, 39,561 edges).

Registered like pascal: MANIFEST.md fork rows + notes (ABI tally
recounted, unchanged), the audit FORKS map, THIRD_PARTY.md, vendored
checksums. LICENSE stays upstream's MIT byte-identical.

Tests: parse_coverage carries the two SQLite tests from #2246 as written
(sqlite_valid_migration_not_flagged, red before this change, and the
sqlite_malformed_still_flagged control) and adds `key` columns, WITHOUT
ROWID, a WHEN trigger with INSERT OR IGNORE plus an fts5 table (asserted
as a Table), PRAGMA forms, and two controls: MySQL's KEY index still
clean, and ALTER TABLE ... DROP INDEX still flagged.

Distilled from #2246.

Co-authored-by: Kerf <[email protected]>
Signed-off-by: Martin Vogel <[email protected]>
@DeusData
DeusData merged commit fcbfed4 into main Nov 12, 2026
79 of 81 checks passed
DeusData added a commit that referenced this pull request Nov 12, 2026
Brings in #2957 (sql became a self-maintained fork), #2959 and #2962.
Both sides added a self-maintained grammar fork, so every conflict keeps
both additions:

- MANIFEST.md summary: 164 grammars = 139 vendored-from-upstream + 20
  self-maintained (sql moved over on main, vba added here) + 5
  registry-disagreement; the ABI tally stays 9/78/77 (sql stayed ABI 15).
  The notes and the forks table carry both the sql and the vba rows.
- THIRD_PARTY.md: thirteen further self-maintained forks including sql and
  vba; main's sql paragraph, then this branch's tree-sitter-vba section.
- tests/test_parse_coverage.c: both test blocks and both sets of
  RUN_TEST registrations.
- scripts/vendored-checksums.txt: regenerated with
  security-vendored.sh --update; against main only the MANIFEST.md line
  and the five vba files differ.

Signed-off-by: Martin Vogel <[email protected]>
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