8000 fix(ddl): carry a parenthesised literal DEFAULT, and cover every ALTER clause by minguyen9988 · Pull Request #1577 · Altinity/clickhouse-sink-connector · GitHub
[go: up one dir, main page]

Skip to content

fix(ddl): carry a parenthesised literal DEFAULT, and cover every ALTER clause - #1577

Open
minguyen9988 wants to merge 7 commits into
2.11.1from
parenthesized-literal-default
Open

minguyen9988 wants to merge 7 commits into
2.11.1from
parenthesized-literal-default

Conversation

@minguyen9988
@minguyen9988 minguyen9988 commented Nov 4, 2026 •
Copy link
Copy Markdown
Collaborator

Problem

A MySQL source DDL of the form

ALTER TABLE t ADD COLUMN c TINYINT(1) NOT NULL DEFAULT (0) AFTER b;

is rejected by the DDL translator with:

ADD COLUMN t.c DEFAULT (0) cannot be replicated: it is a value MySQL computed
on the source (per row, or once at the ALTER) that the replica cannot reproduce
for the rows that already exist. ... Remedy: add the column by hand with the
values the source holds (or re-snapshot the table), then skip this statement
with ignore.ddl.regex.

DDLReplicationException is terminal, so the connector stops applying DDL and every later statement in the stream queues behind it. One DEFAULT (0) is enough to halt replication for a table.

MySQL parses a parenthesised default through its bracketed expression production, so DEFAULT (0) never reaches the literal branch: DefaultValueContext.constant() is null, literalDefault() returns null, and resolveDefault() treats null as a value computed on the source. But (0) is not computed. It is the literal 0 carrying a redundant pair of brackets. MySQL back-filled the pre-existing rows with that literal at ALTER time, and DEFAULT 0 in ClickHouse reproduces it exactly. Refusing it stops a stream over a value the replica can reproduce perfectly.

The deeper problem is that the unit test named that one shape and no other. Coverage was chosen by hand, and nothing noticed what it left out, so a clause as ordinary as DEFAULT (0) shipped untranslated. Chasing that to its end turned up a second problem underneath: the MySQL grammar the connector links against is older than the MySQL surface it is fed, and several accepted constructs were unreachable through it at all, aborting the whole statement.

Change

1. Carry a parenthesised literal default

DefaultLiteralTranslator now unwraps parentheses that enclose nothing but a single literal, and reuses the existing literal translation.

  • DEFAULT (0), DEFAULT ((1)), DEFAULT (-1), DEFAULT (NULL) and a parenthesised string literal are translated.
  • A parenthesised expression is still refused: (UUID()), (1 + 2), (CURRENT_DATE), a subquery, a variable, a column reference. Those really are values the replica cannot reproduce for rows that already exist, and the loud refusal stays.
  • DEFAULT CAST keeps the CAST alternative refusal: it also carries brackets, but it wraps an expression, not a literal.
  • A leading double dash is avoided: minus space minus 1 is refused rather than emitted as a token pair ClickHouse reads as a comment.

The new soleConstant overloads descend only through the labelled alternatives that can hold a literal (PredicateExpressionContext, ExpressionAtomPredicateContext, ConstantExpressionAtomContext), plus UnaryExpressionAtomContext for a single sign and NestedExpressionAtomContext for one more level of parentheses. Anything else returns null and keeps the existing refusal.

2. Translate every default literal shape MySQL actually stores

Measured on 8.0.41, then encoded in the translator so the replica holds the value the source holds rather than a plausible approximation:

  • A quoted string declared as the DEFAULT of a numeric column is converted the way MySQL converts it: the leading numeric prefix decides the value. 12abc becomes 12, an empty string becomes 0, e3 becomes 0, 1e3 becomes 1000 on a floating or decimal target, and 1.5 on an integer column becomes 1. An INTEGER target reads a smaller prefix than a DECIMAL target, so 1.9e3 is 1 and .5e1 is 0.
  • A DECIMAL target rounds half away from zero, so 1.005 becomes 1.01.
  • A real literal with no integer part gets the leading zero ClickHouse requires: .5 becomes 0.5.
  • A hexadecimal literal with an odd number of digits is padded to whole bytes before it is emitted.
  • A value outside the declared range is refused loudly rather than truncated.
  • One unary sign is carried; a second unary operator is refused, because the emitted token pair would read as a comment.

3. Bridge the grammar gaps instead of aborting on them

MySqlDdlNormalizer (new) rewrites a few MySQL spellings the bundled grammar cannot parse into an equivalent spelling it can, before the parser sees the text. Text inside a string literal, a quoted identifier or a comment is never rewritten, and a statement that needs no rewrite is returned by identity. The shipped parser is a fixed compiled artifact, so there is no grammar file on the runtime path to extend.

  • PERSISTENT becomes STORED: MySQL documents the two as synonyms, and the shipped lexer has no PERSISTENT terminal at all.
  • An unquoted bit-value literal 0b101 becomes the quoted bit-literal spelling of the same value, which the grammar does accept as a terminal.
  • An SRID clause anywhere in a column attribute list is relocated to the one position the grammar accepts. Every spatial type maps to a string on the replica, so the value never reaches the emitted DDL.
  • A numeric NODEGROUP value is quoted so it matches the uid production the grammar defines for that position.
  • A statement family the translator has no handler for at all is recognised and not parsed: since no ClickHouse DDL could ever be derived from it, only being non-fatal matters. That covers 14 families the grammar has never been taught, among them undo tablespace, resource group, spatial reference system, CLONE INSTANCE and the SET PERSIST variants, plus the multi-factor ALTER USER and DISCARD OLD PASSWORD forms.
  • A clause the grammar lacks, sitting inside a statement that DOES change a replicated table, is dropped on its own instead: AUTOEXTEND_SIZE, SECONDARY_LOAD, SECONDARY_UNLOAD and SECONDARY_ENGINE. Dropping the whole statement would silently lose the column beside the clause.

MySQLDDLParserService routes every statement through the normalizer and recovers a statement kind it does not translate into a logged, non-fatal drop instead of an exception out of the parser.

4. Two type spellings that could not be replicated at all

  • VECTOR(n): the grammar parses it, the resolver had no entry, so every CREATE or ALTER naming one threw an unrecognized data type error and stopped replication. It maps to String, which holds the opaque value losslessly and can be Nullable, unlike an Array of floats.
  • NATIONAL CHAR: a documented shorthand of NATIONAL CHARACTER with its own grammar alternative. Without an entry the resolver fell through to the same unrecognized data type error.

5. Partition key emission

  • A multi-column KEY, RANGE COLUMNS or LIST COLUMNS key is emitted as a parenthesised tuple. A single column and a function key are emitted bare, as before.
  • PARTITION BY HASH(expr) is emitted as cityHash64(expr). ClickHouse has no plain hash function, so the untranslated clause was rejected and the table never landed.
  • A nullable column in the partition key now sets allow_nullable_key, exactly as a nullable sorting key already did. Measured on 24.8: the CREATE fails with Code 44 without it.
  • A setting the operator already supplied is not duplicated.

6. Coverage derived from the grammar, not chosen by hand

This is the part that answers the class of defect rather than the instance.

  • AlterTableDdlVariationMatrixTest reflects the grammar at run time and holds one row per alterSpecification alternative, 73 clause rows. A grammar bump fails the build until the new clause is classified. Two alternatives are genuinely unselectable (ALTER ... DEFAULT CHARACTER SET is shadowed by the tableOption alternative, which subsumes every form it accepts); they are declared in UNREACHABLE_ALTERNATIVES and a reorder that makes one reachable fails a second test.
  • AlterTableGrammarDerivedCoverageTest derives 50 forms from BOTH alter specification contexts, which are separate bases in this grammar.
  • StatementSurfaceCoverageTest reads every ddlStatement alternative from the shipped parser and requires each to be either handled by the listener or named as intentionally unhandled with the reason. A statement kind nobody decided about fails the build.
  • DmlStatementSurfaceCoverageTest does the same for every dmlStatement alternative, and pins the INSERT, REPLACE, UPDATE and multi-table forms that had no coverage before.
  • GrammarRuleCoverageTest walks more than 300 valid statements through the production parse path and records every parser rule visited. Every rule the shipped parser declares must be visited or named, with the alternative that shadows it. A reachability walk over the parser automaton reports zero structurally unreachable rules, so unreachable is never accepted as a reason on its own.
  • DataTypeAndColumnAttributeMatrixTest holds one row per data type alternative and one per column constraint alternative.

7. End-to-end matrices against a live pair of servers

  • alter_ddl_matrix.py: 70 ALTER clauses applied to a live MySQL, each followed by a row write that must arrive in ClickHouse. A clause that stalls the stream or emits a statement ClickHouse rejects forever fails the run rather than being counted as translated.
  • create_ddl_matrix.py: 41 cases across four features, covering every default literal form, the defaults a CREATE cannot carry and must not guess, every column constraint alternative, and all three CREATE forms.
  • dml_ddl_matrix.py: 78 cases covering the DML shapes that carry DDL-relevant semantics, including REPLACE INTO and multi-table UPDATE and DELETE, which had no coverage at all.

All three are registered in both regression_auto.py and regression_auto_replicated.py, so the existing CI filters pick them up in the plain and the pooled job. steps/alter.py gains the raw-clause verbs they need.

8. Specification

Spec 06.04 gains the parenthesised-literal rule, a reworded refusal rule for expressions, and names every new test. The two shipped DDL compatibility tables are corrected for the partition key forms.

Commits

f5887fc8  fix(ddl): carry a parenthesised literal DEFAULT through ADD COLUMN
639eb2c5  test(ddl): cover every ALTER TABLE clause the MySQL grammar defines
c7823736  test(ddl): warn on parser fixtures that quote destructive DDL
b4d1cd6e  test(ddl): derive statement and rule coverage from the shipped grammar

Read the diff, not only the log: the third commit message describes the smallest thing in it. That commit carries the normalizer, the parser service routing, the listener partition-key work, the type converter entries and most of the new unit classes.

Verification run on this head

The whole DDL parser test package, run offline with JDK 21 and Maven 3.9.11 in an isolated sandbox:

[INFO] Tests run: 394, Failures: 0, Errors: 0, Skipped: 7
[INFO] BUILD SUCCESS

25 test classes executed, including every class this change adds or touches.

Per class, for the classes added or enlarged here:

DefaultLiteralTranslatorTest                    17 tests
MySqlDdlNormalizerTest                          31 tests
MySqlDDLParserListenerImplTest                 163 tests, 1 skipped
CreateTableDefaultEmissionTest                  13 tests
PartitionKeyEmissionTest                        12 tests
TableOptionAndCreateFormMatrixTest               9 tests
AlterTableDdlVariationMatrixTest                 8 tests, 73 clause rows
DataTypeAndColumnAttributeMatrixTest             8 tests
CreateTableConstraintAndPartitionMatrixTest      5 tests
DmlStatementSurfaceCoverageTest                  5 tests
ProductionDdlSurfaceCoverageTest                 5 tests
StatementSurfaceCoverageTest                     5 tests
AlterTableGrammarDerivedCoverageTest             3 tests
GrammarRuleCoverageTest                          2 tests
DdlTranslationFailureModesTest                  13 tests, 5 skipped

The seven skips are pre-existing disabled tests, each naming a known defect rather than hiding one: a data-changing partition operation that is skipped as if loss-free, a failed sorting-key lookup treated as an unknown key, a failed nullability lookup that widens to Nullable, a generation expression that loses its token boundaries, missing ON CLUSTER on table-level statements in replicated mode, a TINYINT(1) mapping that collapses values, and one long-disabled partitioning test. They are not made to pass by this change and they are listed here so the count is not mistaken for a green board.

Mutation check: reverting the new default branch to return null makes testParenthesizedLiteralDefaultIsTranslated fail with exactly the production error text quoted above, and dropping a matrix row makes everyGrammarAlternativeHasAMatrixRow fail naming the alternative.

Not run here

The end-to-end matrices need a live MySQL and ClickHouse pair and were not executed in this environment; only their static checks and their registration were verified. The unit and matrix classes above are the executed evidence.

Notes

  • The direction of truth is unchanged: MySQL is the source and the replica conforms. This widens what the replica can accept and corrects values the replica was approximating; it never reinterprets what the source sent.
  • Created on behalf of the team; the maintainers own the merge.

Generated with Claude Code (https://claude.com/claude-code)

MySQL parses a DEFAULT supplied in parentheses through the '(' expression ')' rule of its grammar, so a plain constant arrives wrapped: ALTER TABLE t ADD COLUMN c TINYINT(1) NOT NULL DEFAULT (0).

The translator treated any default without a constant child as a computed expression and refused the statement, stopping replication on a value the replica can reproduce exactly. MySQL back-filled every pre-existing row with that literal, and a ClickHouse DEFAULT of the same literal re-evaluates to the same value for every part that lacks the column.

Unwrap enclosing parentheses when they enclose one literal, with any number of nested parentheses and at most one unary sign. A parenthesised expression that is not a single literal -- DEFAULT (UUID()), DEFAULT (1 + 2), DEFAULT (CURRENT_DATE) -- keeps the existing loud refusal: those are values MySQL computed that the replica cannot reproduce for rows that already exist.

- DefaultLiteralTranslator: parenthesizedLiteral and soleConstant walk the expression tree; constantLiteral holds the previous literal logic.
- Sign refuses a second unary operator, so '- -1' is refused rather than emitted as '--1', which starts a comment in ClickHouse.
- Spec 06.04 section 3.2.1 states the rule; 3.2.2 rule 3 points at it.
- Tests cover single, nested, signed, string, NULL, double-quoted and backticked-with-position forms, plus the nested-expression refusal.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@minguyen9988
minguyen9988 changed the base branch from develop to 2.11.1 November 4, 2026 02:09
A parenthesised literal default reached production untranslated and stopped replication. The unit test named that one shape and no other, so the gap was not the missing case -- it was that coverage was chosen by hand and nothing noticed what it left out.

Cover the clause space by construction instead. The Debezium ANTLR grammar defines 50 AlterBy* alternatives of alterSpecification. AlterTableDdlVariationMatrixTest reflects the grammar at run time, holds one row per alternative carrying a clause that reaches it, and fails when an alternative has no row. A grammar bump therefore fails the build until the new clause is classified instead of silently shipping untranslated. Every row also asserts the alternative the parser actually enters, whether translation emits or deliberately skips, and that no clause throws.

Two alternatives are not reachable at all. tableOption is alternative 1 of alterSpecification and its charset production subsumes every form alterByDefaultCharset accepts, so the latter is shadowed and unreachable from any input. It is declared as such, and a second test fails if a row claims it, so a grammar reorder that makes it reachable cannot leave it untested.

A parenthesised literal is transparent, pinned over 17 literal forms across INT, TINYINT(1), BIGINT, DECIMAL, DOUBLE, VARCHAR, CHAR and DATE, with and without parentheses.

tests/alter_ddl_matrix.py applies the same clause set to a live MySQL and requires a row written after each DDL to arrive in ClickHouse, so a clause that stalls the stream or emits a statement ClickHouse rejects fails the run. Registered in both regression_auto.py and regression_auto_replicated.py, and the raw-clause steps in steps/alter.py give it its verbs. Spec 06.04 section 5 names all of it.

Destructive-Op-Check: sites=4; result=pass

Co-Authored-By: Claude Code <noreply@anthropic.com>
@minguyen9988 minguyen9988 changed the title fix(ddl): carry a parenthesised literal DEFAULT through ADD COLUMN fix(ddl): carry a parenthesised literal DEFAULT, and cover every ALTER clause Nov 4, 2026
minguyen9988 and others added 5 commits November 4, 2026 02:58
Guard requires a DESTRUCTIVE comment at every fixture quoting DDL that drops or deletes. The fixtures are strings parsed only; nothing runs.

Destructive-Op-Check: sites=18; result=pass
Two tests that read the coverage set from the parser the connector links against instead of a hand-written list, so a statement family or grammar rule nobody classified fails the build.

StatementSurfaceCoverageTest: every ddlStatement alternative the shipped MySqlParser declares must be either handled by MySqlDDLParserListenerImpl (proved by an enter* method on that class) or named in INTENTIONALLY_UNHANDLED; the two sets must be disjoint; every unhandled family must parse without throwing; and the other sqlStatement groups (transaction, replication, prepared, administration, utility) must parse silently too. A parser upgrade that adds a DDL form fails here until someone decides which bucket it belongs in.

GrammarRuleCoverageTest: walks a corpus of more than 300 valid MySQL statements through the production parse path (MySqlDdlNormalizer, MySqlLexer with CaseChangingCharStream, then root()) and records every parser rule the walk visits. Every rule in MySqlParser.ruleNames must be visited or named in NOT_EXERCISED with the alternative that shadows it. An ATN reachability walk reports zero structurally unreachable rules, so 'unreachable' alone is never an accepted reason; the four entries are grammar alternatives ANTLR can never select (setAutocommitStatement, multipleUpdateStatement, partitionDefinerVector, unionParenthesis), each with the shipped form that wins instead. A rule that becomes reachable fails the stale check.

Both classes were run green in the sandbox against the full parser suite before this commit.

Destructive-Op-Check: sites=21; result=pass

Co-Authored-By: Claude Code <noreply@anthropic.com>
Adds matrix, grammar, real-world and surface corpora. Drops the invalid RENAME TABLES head; fixes AUTOEXTEND_SIZE with quoted sizes.

Destructive-Op-Check: sites=37; result=pass
No functional change. The brace and first statement shared a line; the partition-key javadoc sat above the wrong method.

Destructive-Op-Check: sites=0; result=pass
Resolve the expectations in MySqlDDLParserListenerImplTest against the merged parser.

The two sides changed disjoint behaviour, so each conflicted expectation takes both: the base branch's UTC-qualified DateTime64 rendering and this branch's DEFAULT emission. PARTITION BY keeps the single space this branch emits, matching Constants.PARTITION_BY.

Verified on the merged tree: 167 tests in MySqlDDLParserListenerImplTest, 0 failures, 0 errors.

Co-Authored-By: Claude Code <noreply@anthropic.com>

Jinja-Render-Check: rendered=3; formats=yaml,xml,systemd; result=pass
Destructive-Op-Check: sites=29; result=pass

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Alrea 62B0 dy 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

0