Hard fork the TiDB parser into pkg/parser (MySQL-only) - #1126
Conversation
Copies github.com/block/tidb/pkg/parser @ e528fd979fc8 (upstream
pingcap/tidb master as of 2026-05-04 plus our spatial type/index
support) into pkg/parser, rewrites all imports to
github.com/block/spirit/pkg/parser, and removes the go.mod
require+replace of the external module.
goyacc (needed only to regenerate parser.go from parser.y) becomes a
nested Go module so its modernc.org dependencies stay out of spirit's
dependency graph; its one import of the parser's format package is
replaced by a local copy of the 3-line Formatter interface.
Two mechanical fixes for Go 1.26 vet (which go test enforces):
Errorf(rangeErrMsg) -> Errorf("%s", ...) in parser.y/parser.go, and an
unused Sprintf result in digester_test.go.
Content is otherwise verbatim from the fork so that follow-up commits
that strip unused functionality have reviewable diffs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cases Grammar: remove TiDB-only statements (ADMIN, BRIE, IMPORT INTO, placement policies, resource groups, bindings, stats ops, FLASHBACK, SPLIT REGION, sequences, traffic, batch DML, procedures, HELP...) and in-statement features (TiFlash replicas, AUTO_RANDOM, SHARD_ROW_ID_BITS, TTL, clustered/global index, AS OF, TABLESAMPLE, vector/columnar/HYPO indexes, partial indexes), plus MariaDB-isms (PAGE_CHECKSUM, PAGE_COMPRESSED, TRANSACTIONAL, SEQUENCE= table option, IETF_QUOTES). MySQL surface is kept in full, including ALTER TABLE ... ANALYZE PARTITION, account management, FLUSH, KILL, EXPLAIN [ANALYZE|FOR CONNECTION], LOAD DATA, LOCK TABLES, prepared statements, BINLOG, and SECONDARY_LOAD/UNLOAD. Keywords: drop 277 keyword tokens that no surviving grammar rule references (TiDB/MariaDB statement vocabulary plus builtin-function names that reach the grammar through btFuncTokenMap); these words now lex as plain identifiers, which is strictly more MySQL-compatible. Keyword-list %prec annotations (DATE/TIME/TIMESTAMP vs string literals, PASSWORD/REUSE vs eq) are preserved. keywords.go regenerated. Lexer: drop the /*T![feature] */ TiDB special-comment machinery (now a plain comment, as MySQL treats it), the AS OF / TO TSO / TO TIMESTAMP multi-token lookahead, the &^ operator, and the CREATE BINDING hint special case. MEMBER OF handling is kept. Parse table shrinks from 3,797,221 to 1,722,509 entries (-55%); parser.y from 17.7k to 12k lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Fold test_driver into ast: concrete ValueExpr/ParamMarkerExpr/Datum/MyDecimal replace the hook-var driver registration; drop blank imports across spirit. - Remove TiDBKeyword grammar section, BEGIN OPTIMISTIC/PESSIMISTIC, CAUSAL CONSISTENCY, BINLOG MONITOR (MariaDB), TOKUDB row formats, SetMariaDB mode, and the /*T![feature_id] special-comment machinery. - Trim terror to the error-class registry the parser needs; drop pingcap/log + zap from our code (remaining go.mod entries are transitive via go-mysql-org/go-mysql). - Delete auth crypto (keep UserIdentity/RoleIdentity), digester, ast/util.go. - Prune tests of removed features; keyword consistency tests updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ports from pingcap/tidb pkg/parser commits since our fork base: - e2b6ce7333: bound parentheses nesting (10000) and AST depth to prevent parser DoS via deeply nested expressions (also covers the optimizer-hint scanner) - 551d10a652: INSERT ... VALUES/SET row aliases (MySQL 8.0.19+), e.g. INSERT INTO t VALUES (1,2) AS new(m,n) ON DUPLICATE KEY UPDATE; rejected for REPLACE like MySQL - b254c43931: dual-password syntax (MySQL 8.0.14+): ALTER USER ... RETAIN CURRENT PASSWORD / DISCARD OLD PASSWORD and SET PASSWORD ... RETAIN CURRENT PASSWORD, with grammar-level enforcement that RETAIN requires a cleartext (BY-form) password and CREATE USER accepts neither clause - 9bbc86e96e: SET_VAR optimizer hint accepts decimal/float values, e.g. /*+ SET_VAR(optimizer_prune_level=0.3) */; other hints still reject non-integer numerics Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…itted goyacc binary - untrack pkg/parser/goyacc/goyacc (4.6MB compiled binary committed by mistake in the fork commit); ignore it plus y.output - delete .editorconfig, test.sh (TiDB CI leftovers) and the duration package (nothing imports it since TTL grammar removal) - remove the plan-cache HashEquals machinery (util/hash64.go, Hash64/ Equals methods on FieldType, CIStr, SelectLockInfo) - remove masking-policy AST nodes and grammar scaffolding (block-TiDB extension; grammar rules were already gone) - orphan-token sweep: drop ATTRIBUTES, CLUSTER, COLUMNAR, LABELS, TTL, VECTOR, TIDB_CURRENT_TSO tokens and the vector type/index plumbing (TypeTiDBVectorFloat32, ETVectorFloat32, IndexTypeVector/HNSW/Hypo/ Inverted) - remove sequence functions (NEXTVAL/LASTVAL/SETVAL, NEXT VALUE FOR): MariaDB/TiDB feature, MySQL has no sequences - KILL: drop the KILL TIDB extension and TiDBExtension AST field - ShowStmt: drop 38 Show* types whose grammar is gone (stats, bindings, placement, import, region, sequence, procedure-status etc.) plus dead fields and the unused NeedLimitRSRow helper - drop the TiDB warning for CREATE/ALTER USER WITH <resource-options> (valid MySQL syntax, parse it silently) - remove TiDBStrictIntegerDisplayWidth global: display widths now always round-trip, matching the flag's default behavior - trim goleak ignores for dependencies we no longer have Parse table: 1,611,644 -> 1,566,523 entries; keywords 507 -> 496. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…code Second brutal-pass batch over the forked parser: - mysql/const.go: drop the TiDB version/server-build machinery (removes the coreos/go-semver direct dependency), client/server protocol capability flags, Com* command bytes, cursor types, and other wire-protocol constants the parser never touches. - mysql/type.go, util.go, error.go, charset.go: drop TypeInt24 bounds, IsAuthPluginClearText, ErrBadConn/ErrMalformPacket, and a dead collation alias. - mysql/locale_format.go: delete (locale-aware number formatting, unused). - ast: remove the expression-flag system (flag.go, SetFlag/GetFlag on ExprNode, FlagHas* consts). Only flag.go itself ever read these flags; removing it also saves a full-AST visitor walk per parse. - main_test.go: goleak ignores for long-gone dependencies removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
These are fresh errors with no wrapping or stack-trace semantics, so the stdlib is equivalent. Removes spirit-proper's last direct use of pingcap/errors outside the forked parser. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ire token - hintparser.y: keep only MySQL 8.0 hint names (plus the SEMIJOIN strategies). TiDB hints (INL_JOIN, MEMORY_QUOTA, READ_FROM_STORAGE, LEADING, USE_TOJA, QUERY_TYPE, ...) now degrade to the generic "unsupported hint" warning or a hint syntax error; the statement itself still parses either way. Hint parse table shrinks 28,934 -> 6,235 entries. The TiDB partition qualifier in hint tables and the MB/GB/TRUE/FALSE/TIKV/TIFLASH/OLAP/OLTP helper tokens are gone too. - ast: remove LeadingList/FlattenLeadingList/HintTimeRange and the Restore cases for removed hints; HintTable loses PartitionList. - errname: hint warning no longer says "by TiDB"; drop the MEMORY_QUOTA overflow error (8063). - keywords.go/generate_keyword: deleted. parser.Keywords was only read by its own tests; nothing in spirit consumes it. - parser.y/misc.go: rename the REQUIRE token from require to requireKwd so internal tests can import testify's require package without aliasing (consistent_test.go, lexer_test.go, reserved_words_test.go de-aliased). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The terror package (an error-class registry from TiDB) and the
github.com/pingcap/errors dependency are gone:
- mysql.ParseError is the one typed error now: a sentinel template
carrying a MySQL error code, instantiated with GenByArgs (standard
template) or GenByFormat (custom message). Instances match their
sentinel through errors.Is, replacing terror.ErrorEqual and
*Error.Equal. Rendering is unchanged ("[parser:1064]..."), so
messages and tests are stable.
- GenWithStackByArgs/FastGenByArgs -> GenByArgs, GenWithStack ->
GenByFormat. No caller inspected stacks; the names now say what
the methods do.
- errors.Annotate/Annotatef -> fmt.Errorf("...: %w", err);
errors.Trace(err) -> err; errors.Errorf -> fmt.Errorf;
errors.New -> stdlib.
- errname.go drops the ErrMessage/redaction indirection: MySQLErrName
is a plain map[uint16]string (no entry ever set RedactArgPos).
- mysql.SQLError/NewErr/NewErrf and mysql/state.go had no callers
left and are deleted.
- goyacc (nested module) converted too; its go.mod no longer needs
pingcap/errors.
github.com/pingcap/* and go.uber.org/* now appear in spirit's go.mod
only as // indirect via go-mysql-org/go-mysql.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The charset name/collation catalog is retained in full: any MySQL DDL can still reference gbk, gb18030, big5, etc. and parse correctly. What is removed is the byte-level transcoding support that only mattered when the *client* connection charset was gbk/gb18030 (a TiDB-oriented feature; spirit always connects with utf8mb4): - delete encoding_gbk.go, encoding_gb18030.go, encoding_gb18030_data.go (~76KB of tables) and the x/text encoding lookup in encoding_table.go - FindEncoding now falls back to the pass-through binary encoding for any charset without a transcoder, the same behavior big5/latin2/etc. always had - simplify encodingBase.Foreach: the GB18030-specific transformer hack is gone - drop AddCharset/RemoveCharset/AddCollation/AddSupportedCollation: dynamic charset registration is a TiDB experimental feature with no MySQL equivalent; the collation registry is now built once in init() - remove GBK-dependent tests and the dead gbkEncodingChecker, subqueryChecker and windowFrameBoundChecker test helpers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PARTITION BY SYSTEM_TIME (plus HISTORY/CURRENT partition definitions) is MariaDB system-versioning syntax, and RANGE ... INTERVAL (...) FIRST/LAST PARTITION LESS THAN is TiDB's interval partitioning; neither exists in MySQL. Removes the grammar rules, the SYSTEM_TIME token, PartitionInterval/PartitionIntervalExpr/PartitionDefinitionClauseHistory AST nodes, the PartitionMethod Unit/Limit/Interval fields, the now-unused MariaDB error-code block, and the corresponding pkg/statement handling. Parse table shrinks from 1,566,523 to 1,542,591 entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…stem golangci-lint (uncapped) reported 518 issues across pkg/parser. This fixes all of them; `golangci-lint run` is now clean repo-wide with no warnings. The biggest change: ExprNode.Format(io.Writer) and its 33 implementations are deleted. It was TiDB's legacy pretty-printer, fully parallel to Restore (which spirit uses), had no consumers, and accounted for ~70 of the unchecked-error findings on its own. Op.Format and format_test.go go with it; the fulltext tests now assert on Restore output. Everything else, by linter: - staticcheck/ST1005 (309): lowercase the upstream "An error occurred" message family; the sql_mode message keeps MySQL's capitalization with a nolint - errcheck: propagate Restore errors in ddl.go/dml.go; explicitly ignore builder writes in format.go; require.NoError in tests - exhaustive: //nolint:exhaustive on render-if-set switches, matching the existing repo convention - unused (21): delete isAllPlacementOptions, the __DEPRECATED_* enum tombstones, showTpCount, exprCleaner, lazyBuf, setKeepHint, eof, and dead test helpers - errorlint: errors.Is/errors.As for strconv.ErrRange and parserDepthLimitError (drops the pingcap-era Cause() method) - gocritic/QF*: if-else chains to switches (labeled where break targets the loop), embedded-field selectors simplified, assignOp/valSwap/ newDeref cleanups - testifylint/modernize: mechanical assertion and range-over-int fixes - remove stale '//nolint: all_revive' directives (unknown-linter warning) - gofmt: import ordering in pkg/fmt, pkg/lint, pkg/migration left over from the fork's import rewrite Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deletes everything the deadcode tool and a TiDB-reference audit turned up: - Dead statement nodes never constructed by the grammar: TraceStmt, SetSessionStatesStmt, StringOrUserVar, ShowSlow(+Type/Kind consts), TableNameExpr, StatisticsSpec/StatsType*, StatementScope. - TiDB-only grammar: FLUSH CLIENT_ERRORS_SUMMARY, FLUSH TIDB PLUGINS, and the ILIKE operator (PatternLikeOrIlikeExpr renamed back to PatternLikeExpr, IsLike discriminator dropped). Parse table shrinks 1,542,591 -> 1,522,413 entries. - Dead API: ast.NewDatum/NewBytesDatum/NewStringDatum/MakeDatums, GetStmtLabel, TrimComment(+regexps), CharsetClient, ColumnChoice, SensitiveStmtNode/SecureText, FieldType.PartialEqual, charset GetSupportedCharsets/GetSupportedCollations/GetDefaultCharsetAndCollate/ GetCharsetInfoByID/GetCollationByID(+backing maps), mysql CharsetNameToID/CharsetIDs/Collations/CollationNames/RangeGraph, Del/SetSQLMode, FormatSQLModeStr(+CombinationSQLMode), Str2Priority, seven unused type-flag helpers, system-table name consts. - TiDB builtin-function name consts (tidb_*, vitess_hash, format_nano_time, current_resource_group); DATE/TIME/TIMESTAMP literal markers rebranded 'tidb` -> 'spirit`. - LoadDataStmt loses the TiDB FORMAT field; FileLocServerOrRemote renamed FileLocServer. - utf8mb4_zh_pinyin_tidb_as_cs collation and TiFlashSupportedCharsets removed; TiDB-flavored comments reworded throughout. - reserved_words_test.go: TiDBKeyword marker removed, stale exceptions dropped, MYSQL_DSN override added; now passes against MySQL 8.0.45. deadcode -test ./... now reports only the four interface marker methods; golangci-lint clean; all parser + dependent tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pkg/parser/README.md now documents the hard fork: credit to PingCAP for creating and maintaining the parser, the fork base, what was stripped and why, usage, goyacc regeneration, and the reserved-words test that compares the grammar against a live MySQL server. Root README, AGENTS.md, and pkg/statement/README.md now point at pkg/parser instead of the external TiDB parser. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes the govulncheck CI failure: GO-2026-6090 (crypto/tls) and GO-2026-5972 (encoding/asn1) are both stdlib vulnerabilities fixed in go1.26.6. Bumps the go.mod directive, the govulncheck and linter workflow pins, and the Dockerfile base images. The release workflow already follows go.mod via go-version-file. govulncheck ./... now reports 0 vulnerabilities affecting our code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
b45712f to
eaeeffa
Compare
MySQL LIST partitioning has no DEFAULT partition; that is a MariaDB
feature TiDB adopted. Removes the bare 'PARTITION p DEFAULT' clause,
DEFAULT inside VALUES IN (...) (DefaultOrExpression{,List} rules), the
matching Restore/Validate special cases, and verifies MySQL LIST
COLUMNS row-constructor syntax still parses. Parse table shrinks to
1,519,313 entries.
Also deletes orphaned comments left by earlier removals (IF NOT EXISTS
on Constraint/CreateIndexStmt, 'MariaDB specific options' label) and
points the INTERSECT/EXCEPT docs at MySQL 8.0.31+ set-operations
documentation instead of the MariaDB knowledge base.
The only remaining MariaDB mention in code is the lexer comment
explaining that /*M! comments are skipped, which is MySQL-compatible
behavior.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
56891c8 to
7f639fd
Compare
… 2b285ed389e0) The separator literal in OptGConcatSeparator was constructed with an empty charset/collation instead of the connection's. Port the upstream fix: thread parser.charset/parser.collation through, and restore the separator with RestoreStringWithoutCharset since the grammar only accepts a plain string literal after SEPARATOR (a charset introducer there would not re-parse). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
MySQL 8.0.13+ distinguishes literal defaults (DEFAULT '{}') from
expression defaults (DEFAULT ('{}')); BLOB/TEXT/JSON/GEOMETRY columns
only accept the parenthesized form. The grammar discarded the
parentheses ('(' SignedLiteral ')' -> $2), so the --statement path
restored ADD COLUMN j JSON DEFAULT ('{}') as DEFAULT '{}' and MySQL
rejected it with Error 1101.
Parser:
- DefaultValueExpr keeps the parentheses as an ast.ParenthesesExpr, for
both CREATE/ADD COLUMN defaults and ALTER COLUMN SET DEFAULT (expr).
- AlterTableSpec.Restore no longer re-wraps an expression that already
carries its own parentheses (single set, not two).
- Upstream still has this bug (pingcap/tidb#57768); noted in README.
Statement layer:
- isExpressionDefault treats ParenthesesExpr as an expression default,
so DefaultIsExpr now distinguishes DEFAULT ('x') from DEFAULT 'x'.
- Extraction unwraps the parentheses before reading the value (emission
re-adds them from DefaultIsExpr), and emission quotes string-valued
expression defaults: DEFAULT ('{}').
- lint_zero_date unwraps parentheses so DEFAULT ('0000-00-00') is still
caught.
Validated against MySQL 8.0.45: the issue's exact statement now
round-trips and executes, and a table created via the restored DDL
extracts identically from SHOW CREATE TABLE (which renders these as
DEFAULT (_utf8mb4'{}')), so declarative diffs converge. Note MySQL
folds ALTER COLUMN ... SET DEFAULT (literal) into a plain literal
default server-side; spirit only emits MODIFY COLUMN, so this does not
create diff loops.
Fixes block#542.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test documented the old TiDB-parser limitation: ALTER ... ADD COLUMN
c BLOB DEFAULT ('abc') via --statement was expected to fail with Error
1101 because the restore dropped the parentheses. The parser fork
preserves them (block#542), so that path now succeeds — assert success and
keep the --table/--alter variant as a separate column. CREATE TRIGGER
remains unparsable and still asserts an error.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Functions whose names lex as keyword tokens rather than plain
identifiers were unparsable in two overlapping ways:
1. DEFAULT expressions only accepted raw-identifier function names
(upstream special-cased just REPLACE), so DEFAULT (point(0,0)) —
the MySQL manual's own expression-default example — failed while
SELECT point(0,0) parsed fine.
2. The spatial constructors other than POINT (LINESTRING, POLYGON,
MULTIPOINT, MULTILINESTRING, MULTIPOLYGON, GEOMETRYCOLLECTION)
became keyword tokens with the GIS fork but were never added to
FunctionNameConflict, so they failed as function calls in *every*
expression context: SELECT lists, generated columns, and CHECK
constraints, not just defaults.
MySQL accepts all of these forms and SHOW CREATE TABLE emits them, so
an affected table was invisible to spirit everywhere — table-info
loading and declarative diffs, not just --statement rewriting.
Fix: split FunctionNameConflict into FunctionNameConflictNonNow +
builtinNow, add the six spatial constructors to it, and let
BuiltinFunction (the DEFAULT-expression grammar) accept
FunctionNameConflictNonNow '(' ExpressionListOpt ')' in place of the
old REPLACE special case. NOW stays excluded from the DEFAULT path so
DEFAULT (now()) keeps folding to CURRENT_TIMESTAMP. Grammar regen is
conflict-free (parse table 1,519,313 -> 1,532,595 entries, +0.9%).
Validated against MySQL 8.0.45: every fixed form executes, spirit's
restored output executes and converges with SHOW CREATE TABLE of the
original, and MySQL's own SHOW CREATE output (charset introducers
included) parses and round-trips stably.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
aparajon
left a comment
There was a problem hiding this comment.
🤖 Adversarial correctness and security review at 210b33ce, looking specifically for anything that could take a process down or make Spirit execute DDL the author did not write. Verdict: no blocking findings, and nothing I found is a regression introduced by this PR. Every issue below reproduces identically on pre-fork main, so this reads as a faithful port; they belong in follow-up issues rather than in this diff.
A 59k-line fork cannot be reviewed line by line, so I reviewed it by behavior instead: the 50-file integration delta in full, then differential testing of the new parser against pre-fork main and against a live MySQL 8.0.45. Details of what that covered are in the collapsed section at the bottom.
1. A numeric literal longer than 81 digits panics the process. pkg/parser/ast/mydecimal.go carries upstream's five panic(panicInfo) branches in MyDecimal.FromString and fixWordCntError. They are reachable from both public entry points with ordinary DDL — a column default, a table option, or a partition bound:
statement.ParseCreateTable("CREATE TABLE t (a DECIMAL(65,0) DEFAULT " + strings.Repeat("9", 82) + ")")
statement.New("ALTER TABLE t1 AUTO_INCREMENT=" + strings.Repeat("9", 100))
statement.ParseCreateTable("CREATE TABLE t (id INT PRIMARY KEY) PARTITION BY RANGE (id) (PARTITION p0 VALUES LESS THAN (" + strings.Repeat("9", 100) + "))")Each panics with This branch is not implemented...; 81 digits parses fine, 82 panics. This is not new — I ran the same three inputs against pre-fork main and they panic identically, because the upstream test_driver MyDecimal has the same stubs and Spirit has always imported it. What changes is ownership: the code is now ours, and any long-running process that parses a schema file out of a pull request can be crashed by a one-line change to that file. TiDB's real types.MyDecimal returns an overflow/truncation error on these branches instead of panicking; porting that, or simply returning an error, closes it. This is the one I would file today.
2. Nothing in CI verifies the generated parser matches the grammar. pkg/parser/README.md says "CI verifies they are in sync with the grammar", but no workflow references pkg/parser/Makefile. I regenerated at this commit and it is clean — make parser reproduces parser.go and hintparser.go byte for byte with no conflicts — so the tree is correct today. The gap is forward-looking: an edit to parser.y that skips regeneration ships a grammar file that no longer describes the parser, and for the component that decides which DDL Spirit will execute, that divergence is worth catching mechanically. A make parser && git diff --exit-code step would do it; the README sentence should either become true or be softened.
3. Bare and parenthesized literal defaults are now a real diff — expect one-time churn on the first plan. This is the intended consequence of #542, but it is worth stating in operational terms. For a live column created as DEFAULT 'x' against a schema file that writes DEFAULT ('x'), pre-fork main produced no statements; this branch produces:
ALTER TABLE `t1` MODIFY COLUMN `v` varchar(10) NULL DEFAULT ('x')
That is correct — MySQL genuinely stores the two forms differently — and it converges in one shot (I verified round-trip idempotency against a live server). But it means a previously-converged table whose schema file wraps a literal default in parentheses on a non-BLOB/TEXT/JSON column will surface an unexpected MODIFY COLUMN on the first plan after this lands. Callers rolling the new parser out may want to sweep for DEFAULT (' in managed schema roots ahead of time rather than discover it one table at a time.
4. Two constructs MySQL accepts still fail to parse. INT DEFAULT ((1+2)) and JSON DEFAULT (CAST('{}' AS JSON)) are both accepted by MySQL 8.0.45 and both rejected here with a syntax error. Identical on pre-fork main, so no regression — but since the parser is now in-repo, these are fixable here rather than upstream, which is the stated point of the fork. Failing closed on unparseable DDL is the safe behavior, so this is a compatibility gap, not a safety one.
5. DEFAULT (ABS(-1)) never converges. MySQL stores it as (abs(-1)) and the differ re-emits MODIFY COLUMN ... DEFAULT (abs(-1)) on every run. Pre-existing and unchanged by this PR — I mention it only because the same round-trip harness that verified the new paren behavior surfaced it, and a permanently-diffing column is the kind of thing that erodes trust in a declarative plan.
6. _gbk'…' and _gb18030'…' introducers are now a parse error. Consistent with the documented charset reduction: the lexer returns Unsupported character introducer: 'gbk' where the TiDB parser would have transcoded. This fails closed, which is the right direction, but a table whose schema declares a gbk column with a non-ASCII literal default will stop parsing rather than silently mis-decode. Note that CHARACTER SET gbk on a column still parses — only the literal introducer is rejected — so the failure mode is narrow and loud.
Action items (all follow-ups; nothing here should hold this PR):
- File an issue for the
MyDecimalpanic in (1) and replace thepanic(panicInfo)branches with returned errors. - Add a CI step that runs
make parserand fails on a diff, and correct or soften the README claim in (2). - Optionally file (4) as a parser compatibility gap and (5) as a differ convergence bug.
Verified
Read the full 50-file non-parser integration delta (the import rewrite is mechanical and complete — no pingcap imports remain outside two comment references). Confirmed the dependency change is a genuine reduction and not a loosening: the block/tidb replace directive is gone, pingcap/errors and pingcap/tidb/pkg/parser are now indirect-only through go-mysql, pingcap/failpoint is dropped, golang.org/x/text is promoted to direct, and nothing was downgraded. Apache-2.0 LICENSE and per-file copyright headers are present with attribution in the README. Exercised the DoS surface with hostile input: 100k nested parentheses, 500k chained NOT, 200k-term OR, 300k unary minus, 50k nested function calls, deep CASE/CHECK/generated-column nesting and nested hint comments all terminate with a clean depth error (parentheses nesting depth exceeds maximum 10000 / AST nesting depth exceeds maximum 10064) rather than a stack overflow; invalid UTF-8 in identifiers and literals, NUL bytes, unterminated strings/comments/identifiers, unknown charset introducers, 100k-character identifiers, and raw binary garbage all either parse or error cleanly, and every AST that parsed restored to text that re-parses. Verified the new default handling against a live MySQL 8.0.45 across 22 column forms (JSON/BLOB/TEXT/VARCHAR/INT/TIMESTAMP/DATETIME parenthesized and bare, NOW(), UUID(), JSON_OBJECT(), hex and _binary literals, and defaults containing quotes, backslashes, newlines and % sequences): create in MySQL, read back SHOW CREATE TABLE, parse both sides, diff — all idempotent except the two pre-existing cases above, which fail identically pre-fork. Mutation-checked the lint_zero_date.go change by deleting the unwrap loop: DEFAULT ('0000-00-00') then escapes the zero-date linter entirely, so that hunk is closing a real safety-gate blind spot the paren preservation would otherwise have opened, and it is the only such consumer — ColumnOptionDefaultValue has exactly two readers outside the parser and both are handled. Confirmed the reserved-word set matches a live MySQL 8.0.45 (-tags reserved_words_test), parsed the repo's MySQL DDL corpus with no new failures, and ran the parser, statement, lint and fmt suites green (the FK and transaction-compression failures I saw are local server configuration and reproduce on pre-fork main). Checked for unsafe usage, mutable package-level state and concurrency hazards in the new package: all inherited from upstream, read-only maps built at init. CI is green including govulncheck and lint; Copilot left no inline comments.
This review was generated by Claude Code (claude-opus-5).
# Conflicts: # pkg/migration/singleversion_test.go
Review feedback on block#1126: the parser README claimed CI verifies parser.go/hintparser.go are in sync with the grammar, but no workflow did. Add a parser-regen job that deletes the generated files, rebuilds them with make, and fails on any diff, and update the README to describe it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The %r verb rework (block#1037) landed on main using errors.Errorf; this branch removed the pingcap/errors import from sqlescape as part of the fork, so the merge compiled against a missing identifier. Use fmt.Errorf like the rest of the file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🤖 Addressing the review feedback: (1) MyDecimal panic — fixed in the stacked PR #1142 ( (2) CI regen verification — added in this PR (e17f8a2): a (4)/(5) — both are on the known-gaps list from the fork audit: (3)/(6) — agreed these are rollout notes rather than code changes: the one-time Also while here: merged |
Hard forks the TiDB parser into
pkg/parserand removes thego.modrequire+replace ofgithub.com/pingcap/tidb/pkg/parser. Spirit only targets MySQL, so the fork strips everything TiDB- or MariaDB-specific and keeps a parser we can extend quickly for our narrower use case. See the new pkg/parser/README.md for the full story.Fork base:
block/tidb@e528fd979fc8(upstreampingcap/tidbmaster as of 2026-05-04, plus our spatial type/index support). The first commit is a verbatim copy with imports rewritten, so reviewing commit-by-commit gives readable diffs for everything that was actually changed.What was stripped
ADMIN,BRIE,TRACE,SET SESSION_STATES, placement/attributes/resource-groups,FLUSH TIDB PLUGINS,FLUSH CLIENT_ERRORS_SUMMARY), MariaDBSYSTEM_TIMEpartitioning, TiDBINTERVALpartitioning,ILIKE, TiDB system functions (tidb_*,vitess_hash, ...), TiDB-only optimizer hints, GLOBAL TEMPORARY tables, sequences, etc. The goyacc parse table shrinks from ~1.57M to ~1.52M entries.driverindirection andDatum/expression glue TiDB layered on top of the AST.terror+github.com/pingcap/errors+zap: replaced with stdlib wrapped errors (errors.Is/Aswork) and no logging dependency.pingcap/*andgo.uber.org/zapare no longer in spirit's direct dependency graph.Format(io.Writer)pretty-printer (parallel toRestore, zero consumers).utf8mb4_zh_pinyin_tidb_as_cscollation. Charset names are still recognized in DDL.deadcode -test ./...reports only interface-conformance marker methods.MySQL fidelity
reserved_words_test.go(build-tagged, requires a live server) now passes against MySQL 8.0.45: the grammar's reserved-word set matches MySQL's exactly, modulo two documented exceptions (CURRENT_ROLE,ARRAY).INSERT ... AS row_alias(551d10a652), dual-password syntax (b254c43931),SET_VARdecimal hints (9bbc86e96e), andGROUP_CONCATseparator charset handling (2b285ed389). A review of all upstreampkg/parsercommits through 2026-08-13 found nothing else MySQL-relevant to pick up.DEFAULT ('{}')(expression default, required on BLOB/TEXT/JSON/GEOMETRY) andDEFAULT '{}'(literal default) as different DDL; the parser used to discard the parens, so--statementmode rewrote valid ALTERs into DDL MySQL rejects with Error 1101. The upstream parser still has this bug (Parser cannot pass default value with an expression pingcap/tidb#57768). The statement layer now also distinguishes the two forms in declarative diffs, and extraction was validated to converge with MySQL'sSHOW CREATE TABLErendering (DEFAULT (_utf8mb4'{}')).DEFAULT (point(0,0))— the MySQL manual's own expression-default example — now parses (upstream special-cases onlyREPLACE), and the spatial constructors (linestring(),polygon(),multipoint(), ...) now parse as function calls in every expression context (SELECT lists, generated columns, CHECK constraints). As keyword tokens they had been broken everywhere since the GIS fork, making any table that uses one invisible to spirit. Validated against MySQL 8.0.45 includingSHOW CREATE TABLEconvergence.Validation
golangci-lint runreports 0 issues repo-wide (the fork previously surfaced ~518).make parserregeneratesparser.go/hintparser.goreproducibly with zero grammar conflicts; goyacc is a nested module so its dependencies stay out of the main graph.🤖 Generated with Claude Code