Repository navigation
Default MODIFY/CHANGE COLUMN to Nullable when schema metadata is unavailable - #1339
Conversation
…ailable When a MODIFY COLUMN or CHANGE COLUMN DDL has no explicit NULL/NOT NULL constraint, the parser looks up the column's current nullability from ClickHouse. If that lookup fails (no DB connection or query error), the column previously defaulted to non-Nullable, which can produce stuck mutations in ClickHouse when existing rows contain NULL values. In MySQL, omitting the nullability constraint on MODIFY/CHANGE COLUMN makes the column nullable, so default to Nullable in the fallback path, consistent with the existing ADD COLUMN behavior. Explicit NULL/NOT NULL constraints and the ClickHouse metadata lookup still take precedence.
7b81ab1 to
ea98c1e
Compare
|
Hi @subkanthi, could you take a look at this PR when you get a chance? The CI workflows also need maintainer approval to run since this is my first contribution here. Thanks! |
- Add explanatory comment to the AlterByChangeColumnContext branch, matching the MODIFY COLUMN branch above. - Add testChangeColumnWithExplicitNull to complete the explicit constraint test matrix (MODIFY/CHANGE x NULL/NOT NULL). - Fix missing blank line before testAddConstraints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🤖 AI Review (Claude)
CorrectnessTraced the full
Trade-offOne narrow regression scenario exists in the fallback path: if the modified column is part of the ClickHouse sorting key and the metadata lookup is down at the same moment, the generated Tests
VerdictThe fix is correct, minimally scoped, consistent with existing behavior, and well tested. Earlier review nits (comment symmetry on the CHANGE COLUMN branch, the missing CHANGE + explicit NULL test, a formatting nit) were addressed in eb3019e. |
Summary
When a
MODIFY COLUMNorCHANGE COLUMNDDL has no explicitNULL/NOT NULLconstraint,MySqlDDLParserListenerImpldetermines nullability by looking up the column's current state in ClickHouse. If that lookup is unavailable (broken DB connection or query error), the column falls back to non-Nullable.In MySQL, omitting the nullability constraint on
MODIFY/CHANGE COLUMNmakes the column nullable. Replicating it as non-Nullable caused stuck mutations in our production environment: aMODIFY COLUMNDDL was translated to a non-Nullable ClickHouse mutation on a column that contained NULL values, and the mutation could never complete. We have been running this fix in production and verified it resolves the issue.Changes
isNullColumn = truein theAlterByModifyColumnContextandAlterByChangeColumnContextbranches, consistent with the existingAlterByAddColumnContextbehavior.NULL/NOT NULLconstraints and the ClickHouse metadata lookup still take precedence. Only the fallback path (lookup unavailable or failed) changes.Known trade-off
If the modified column is part of the ClickHouse table's ORDER BY / primary key and the schema lookup is unavailable at the same time, the generated
MODIFY COLUMN ... Nullable(...)DDL will be rejected by ClickHouse ("Sorting key cannot contain nullable columns"). The old fallback happened to be correct for that narrow case, but silently produced stuck mutations for ordinary nullable columns. The new fallback fails loudly at DDL time instead of corrupting replication silently. The normal path is unaffected: when the metadata lookup succeeds, sorting-key columns still resolve to non-Nullable.Tests
MySqlDDLParserListenerImplTestthat pinned the old non-Nullable fallback (unit tests run without a DB connection, so they exercise exactly this path).testModifyColumnWithNotNull,testModifyColumnWithExplicitNull,testChangeColumnWithNotNull,testChangeColumnWithExplicitNull.MySqlDDLParserListenerImplTest: 99 run, 0 failures.AlterTableModifyColumnIT(testcontainers, live MySQL + ClickHouse): passes unchanged, confirming the live-connection path is unaffected.Files Changed
sink-connector-lightweight/src/main/java/com/altinity/clickhouse/debezium/embedded/ddl/parser/MySqlDDLParserListenerImpl.java(2 branches)sink-connector-lightweight/src/test/java/com/altinity/clickhouse/debezium/embedded/ddl/parser/MySqlDDLParserListenerImplTest.java(6 updated expectations, 4 new tests)