fix: a column rename can NULL out rows still in flight - #1395
Merged
Conversation
…ptions add options to take non locking snapshots
`ALTER TABLE ... CHANGE COLUMN old new` renames the column in ClickHouse
while records read under the PRE-rename schema are still buffered. Those
records carry the OLD field name; the table now has the NEW column. The
writer finds no value for it, binds NULL, and overwrites the real one.
The row count is unaffected, so count-based checksum jobs report the table
clean. The corruption is invisible until someone reads the column.
Measured on MySQL 8.0.36 (ROW/FULL/GTID) -> ClickHouse 24.8.14, 80 inserts
issued across a rename with no pause:
row count mysql=80 ch=80 <- matches, so checksums pass
NULL in renamed mysql=0 ch=3 <- ids 38,39,40 silently blanked
and an UPDATE straddling the rename was lost outright:
mysql : 1=upd1, 2=upd2, 3=orig3
ch : 1=<NULL>, 2=upd2, 3=orig3
The connector logged `Column index missing for column b_renamed` five
times during the run, which is the writer reporting exactly this and
carrying on.
Root cause is the DDL path's flush, which does not flush. On receiving a
DDL it logs "Flush all existing records" and calls `executor.pause()`, but
pause() only stops NEW batches from starting: it does not drain the queue
and does not wait for a batch already inside a task body. The DDL was then
applied immediately, while a batch of pre-rename records was still being
written -- against the post-rename table.
The fix makes the drain real, in three ordered steps before the DDL is
applied: let the pool consume what is already queued (the pool is still
running at this point), then pause() so no new batch starts, then wait out
the batches still executing. Only then is every record read under the
pre-ALTER schema actually in ClickHouse.
Waiting out the running batches needs a signal the executor did not have,
so ClickHouseBatchExecutor now counts batches inside a task body
(incremented in beforeExecute, decremented in afterExecute so a batch that
throws cannot leak the counter) and exposes awaitQuiescent(timeout).
The drain is bounded at 60s. On timeout the DDL proceeds and a warning
names the condition: blocking replication indefinitely would be worse than
the corruption being avoided, and a warning is better than doing this
silently, which is how the bug behaved.
After the fix, same scenarios:
scenario mysql ch result
quiescent rename, no in-flight writes 0 0 PASS
rename with 80 writes in flight 0 0 PASS (was 3)
UPDATE straddling the rename upd1 upd1 PASS (was NULL)
"Column index missing" log lines - 0 PASS (was 5)
Note this is the second half of the pre-ALTER record problem. The ADD
COLUMN half was fixed earlier by omitting from the INSERT column list any
column absent from the record's schema, so ClickHouse applies the column
DEFAULT instead of NULL. That approach cannot address a rename: the value
is present in the record, just under the previous name, so the correct
outcome is to write it before the rename takes effect rather than to skip
the column.
Tests: sink-connector 190 run / 0 failures / 20 errors against an untouched
baseline of 186 / 0 / 20 on the same base commit; the 20 are pre-existing
testcontainers "Could not find a valid Docker environment" errors and the
delta is exactly the 4 new tests. Failing-first verified: with
awaitQuiescent stubbed to return immediately (the old no-drain behaviour)
testAwaitQuiescentWaitsForRunningBatch fails on the corruption window,
while the idle-path, failed-batch and pause-still-blocks guards keep
passing.
This was referenced Aug 24, 2026
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.
Reviewed by a 3-model cross-provider adversarial judge panel (Gemini 3.1 Pro / Grok 4.2 Reasoning / GPT-5.5): 3/3 APPROVE, round 1, zero blocking findings.
Full detail follows, as committed:
fix: a column rename can NULL out rows still in flight
ALTER TABLE ... CHANGE COLUMN old newrenames the column in ClickHousewhile records read under the PRE-rename schema are still buffered. Those
records carry the OLD field name; the table now has the NEW column. The
writer finds no value for it, binds NULL, and overwrites the real one.
The row count is unaffected, so count-based checksum jobs report the table
clean. The corruption is invisible until someone reads the column.
Measured on MySQL 8.0.36 (ROW/FULL/GTID) -> ClickHouse 24.8.14, 80 inserts
issued across a rename with no pause:
and an UPDATE straddling the rename was lost outright:
The connector logged
Column index missing for column b_renamedfivetimes during the run, which is the writer reporting exactly this and
carrying on.
Root cause is the DDL path's flush, which does not flush. On receiving a
DDL it logs "Flush all existing records" and calls
executor.pause(), butpause() only stops NEW batches from starting: it does not drain the queue
and does not wait for a batch already inside a task body. The DDL was then
applied immediately, while a batch of pre-rename records was still being
written -- against the post-rename table.
The fix makes the drain real, in three ordered steps before the DDL is
applied: let the pool consume what is already queued (the pool is still
running at this point), then pause() so no new batch starts, then wait out
the batches still executing. Only then is every record read under the
pre-ALTER schema actually in ClickHouse.
Waiting out the running batches needs a signal the executor did not have,
so ClickHouseBatchExecutor now counts batches inside a task body
(incremented in beforeExecute, decremented in afterExecute so a batch that
throws cannot leak the counter) and exposes awaitQuiescent(timeout).
The drain is bounded at 60s. On timeout the DDL proceeds and a warning
names the condition: blocking replication indefinitely would be worse than
the corruption being avoided, and a warning is better than doing this
silently, which is how the bug behaved.
After the fix, same scenarios:
Note this is the second half of the pre-ALTER record problem. The ADD
COLUMN half was fixed earlier by omitting from the INSERT column list any
column absent from the record's schema, so ClickHouse applies the column
DEFAULT instead of NULL. That approach cannot address a rename: the value
is present in the record, just under the previous name, so the correct
outcome is to write it before the rename takes effect rather than to skip
the column.
Tests: sink-connector 190 run / 0 failures / 20 errors against an untouched
baseline of 186 / 0 / 20 on the same base commit; the 20 are pre-existing
testcontainers "Could not find a valid Docker environment" errors and the
delta is exactly the 4 new tests. Failing-first verified: with
awaitQuiescent stubbed to return immediately (the old no-drain behaviour)
testAwaitQuiescentWaitsForRunningBatch fails on the corruption window,
while the idle-path, failed-batch and pause-still-blocks guards keep
passing.