Repository navigation
fix(grammar): stop flagging valid SQLite migrations as partially parsed - #2957
Merged
Merged
Conversation
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
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]>
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.
What
Valid SQLite migrations were reported as partially parsed. The vendored SQL
grammar (DerekStride/tree-sitter-sql @
851e9cb257ba) rejects SQLite syntaxthat 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 AUTOINCREMENTCREATE TRIGGER ... BEGIN ... END;, also after anunparenthesized
WHENcondition, withINSERT OR IGNOREinsidekeyafter the first column) WITHOUT ROWID;CREATE VIRTUAL TABLE messages_fts USING fts5(...)IF NOT EXISTSon 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
keycase is lexical: after a comma inside a column list the grammar alsoaccepts MySQL's
KEY name (cols)index, sokeylexes as that keyword thereand
key TEXT NOT NULLbecame a malformed index.How
sqlbecomes a self-maintained fork of the pin we already vendored, on thesame model as
tools/tree-sitter-pascal/:tools/tree-sitter-sql/holds upstream'sgrammar.js+grammar/(blob-identical to the pin and to upstream main @
3c99273f9b32, where itscorpus lives), upstream's MIT
LICENSE, and one patch,patches/sqlite-dialect.patch, which touchesgrammar/only. Itsindexlines carry upstream's pre-image blob hashes.
AUTOINCREMENTis upstream's own change (c042cde1e2a6, 2026-11-08, afterour pin), taken verbatim, so that hunk disappears on the next refresh.
blocknode. Precedence keeps PostgresWHEN (...) EXECUTE FUNCTIONtrees unchanged.create_table, so it is extracted as aTable like any other.
(
pragma_statement,module_argument, four keywords;create_tablegains amodulefield).scanner.cis 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).keycan now start acolumn, the type keywords that take no arguments (
name,date,text,uuid, ...) are accepted as MySQL index names soKEY name (name)stays anindex. An unquoted MySQL index named after a type that takes arguments
(
KEY timestamp (ts)) is now an ERROR; backtick-quoted names, whichmysqldump writes, are unaffected.
Registered like pascal: MANIFEST.md fork rows and notes (ABI tally recounted,
unchanged), the provenance audit's
FORKSmap, THIRD_PARTY.md, and thevendored checksums.
Provenance
parser.cbyte-for-byte except line 1 (upstream's generator stamp carries noversion);
grammar.json,node-types.jsonandtree_sitter/*.harebyte-identical. The previously vendored files were upstream's minus the final
newline.
parser.cSHA-2563c64e5c415d00f7d935b7f9f0516f7be04801b241bc5da2e5e71ca31cf337887), 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.txtadds 11 cases; the 9 SQLite cases fail on theunpatched grammar and the 2 controls pass on both.
Verification
Real migration sets, each indexed from scratch with an isolated cache, main
6d34ca15vs this branch:58a980f8,migrations/sqlite(116 files)2535161e,*.sql(69 files)9b88efd6(control, no SQL)ALTER TABLE ... DROP INDEXdown 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.
region swallowed.
Tests in
tests/test_parse_coverage.c:sqlite_valid_migration_not_flagged(red before this change) and thesqlite_malformed_still_flaggedcontrol.keycolumns,WITHOUT ROWID, a WHEN trigger withINSERT OR IGNOREplus an fts5 table (asserted as a Table), PRAGMA forms.KEYindex still clean, andALTER TABLE ... DROP INDEXstill flagged.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 byc042cde1e2a6,which this fork already mirrors. Not opened yet.