8000 fix(writer): engine semantics -- resolved version/sign columns, CollapsingMergeTree cancel row, relocation on stored values, TRUNCATE at its binlog position, no dropped records by minguyen9988 · Pull Request #1476 · Altinity/clickhouse-sink-connector · GitHub
[go: up one dir, main page]

Skip to content

fix(writer): engine semantics -- resolved version/sign columns, CollapsingMergeTree cancel row, relocation on stored values, TRUNCATE at its binlog position, no dropped records - #1476

Merged
minguyen9988 merged 9 commits into
2.11.0from
fix-writer-engine-semantics
Sep 22, 2026
Merged

minguyen9988 merged 9 commits into
2.11.0from
fix-writer-engine-semantics

Conversation

@minguyen9988
Copy link
Copy Markdown
Collaborator

Base: 2.11.0 at 5780b62 (after 1470-1474). Part of the replication-correctness audit; spec-first, failing-then-passing test and mutation check per finding; Lean BatchOrder model added for the TRUNCATE ordering.

  • Sorting-key relocation is detected on stored values (getWithoutDefault), not on Connect-schema defaults: a key column moving NULL to its DEFAULT compared equal, so no tombstone was written and the old row was stranded. Spec 05.01.
  • CollapsingMergeTree UPDATE stages the cancel row (sign -1) with its own addBatch; previously the before image was bound and then overwritten by the after image, so deletes and updates never collapsed. Every UPDATE is grouped once instead of twice. Specs 04.04, 05.04, 04.01.
  • No record is dropped at the grouping stage; an empty batch is refused loudly. The missing-image case used to fail with an opaque NPE and retry forever.
  • Kafka sink task fails when its runnable is dead; only true tombstones are skipped.
  • A TRUNCATE that ClickHouse keeps refusing fails the batch instead of being reported as applied after ten retries.
  • A replicated TRUNCATE change event is applied at its binlog position, against the target database name; before, TRUNCATE events grouped with DML ran in HashMap order (rows after it could be lost, rows before it resurrected) and against the source database name.
  • Engine columns are bound by their resolved names: a ReplacingMergeTree whose version or is_deleted column is not literally _version or is_deleted, or a CollapsingMergeTree whose sign column is not _sign, previously never had the column bound (version 0 for every row, sign 0, deletes never collapsing). A ReplacingMergeTree that cannot be versioned or deleted from is refused.

Verification: sink-connector unit suite and lightweight unit suite green; Docker-backed ITs (TruncateTableIT, BinLogHistoryIT) rely on CI. Spec validator passes; lake build clean.

Merge note: see fix-writer-value-fidelity for the suggested order of the six audit PRs; forward-merge 2.11.0 after each merge to clear the shared registration files.

minguyen9988 and others added 9 commits September 20, 2026 21:35
…ect-schema defaults

updateRelocatesSortingKey compared before.get(C) with after.get(C).
Struct.get substitutes schema.defaultValue() for a stored null, and Debezium
fills that default from the MySQL column DEFAULT, so an UPDATE that moved a
sorting-key column from NULL to its DEFAULT value compared equal, was judged
an in-place update, and the pre-update row at the NULL key was never
tombstoned: MySQL held one row, ClickHouse two, forever. The bind path already
reads stored values with getWithoutDefault (spec 04.03 section 3.3); the
relocation decision now observes the same values the rows are written with.

Spec 05.01 section 3.1 step 3 now names getWithoutDefault and a new section
3.2 states the rule and the failure it prevents.
PreparedStatementExecutorSortingKeyTombstoneTest.testNullToSchemaDefaultInSortingKeyIsAChange
fails on the previous code (NULL -> 'x' under DEFAULT 'x' returned false) and
passes with the fix; mutation-checked by reverting the comparison.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…UPDATE once

Two coupled defects on the UPDATE path.

GroupInsertQueryWithBatchRecords appended an UPDATE record once for its
before image and once for its after image. Both images resolve to the same
INSERT template, so the same record landed twice in the same list and
PreparedStatementExecutor bound and wrote it twice: 2x write amplification on
ReplacingMergeTree, and on CollapsingMergeTree two +1 rows per update. The
record is now grouped once, under the after-image template; the executor
binds whichever images the engine needs from that one entry.

PreparedStatementExecutor bound the before image with sign = -1 for a
CollapsingMergeTree target but never called addBatch() before binding the
after image over the same parameters, so the -1 cancel row never reached
ClickHouse and nothing ever collapsed. The before image is now staged with
its own addBatch() before the after image is bound: one UPDATE stages
[-1 (before image), +1 (after image)].

Specs: 04.01 gains section 3.2 (one record, one entry); 04.04 section 3.1
states the once-only grouping and the CollapsingMergeTree sequence; 05.04
gains section 3.1 with the exact bind/addBatch order and the failure it
prevents.

Tests: GroupInsertQueryHistoryMultiRowTest.standardModeStillSplitsUpdateIntoBeforeAndAfter
pinned the defect (asserted 40 grouped records for 20 UPDATEs) and is
deliberately flipped to standardModeGroupsEachUpdateOnce (20), plus
standardModeGroupsOneUpdateUnderOneTemplateOnce (one template, list size 1).
New PreparedStatementExecutorCollapsingSignTest drives the real grouping and
executor against a recording JDBC proxy (RecordingJdbc, new test support) and
asserts the sign bound at each addBatch(): failed before with [1, 1], passes
with [-1, 1]; INSERT stages [1], DELETE stages [-1]. Mutation-checked by
reverting each half separately.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ty batch loudly

GroupInsertQueryWithBatchRecords.updateQueryToRecordsMap answered false when
a record could not be turned into a statement, and groupQueryWithRecords
reported only the LAST record's answer, so the contract allowed a per-record
skip with the batch's offset still advancing past it. In practice the
missing-image case (a DELETE with no before image, an INSERT/UPDATE with no
after image) never reached that return: the null template was dereferenced
first and the worker saw an opaque NullPointerException, retrying the batch
forever with no hint of which record or image was missing. An UPDATE lacking
its after image was grouped by its before image alone and written as a live
row of the pre-update values. A null column map was a NullPointerException
from inside the formatter.

Both methods now return void: a record is grouped or the batch fails with
IllegalStateException naming the record (operation, topic, partition,
offset), the table and the missing image / metadata. The unreachable
"RECORD DROPPED: INVALID CDC RECORD STATE" log line is an exception too, as
is the executor's "INVALID CDC RECORD STATE" branch, which would otherwise
have staged the statement with the previous row's parameters.

PreparedStatementExecutor.addToPreparedStatementBatch refuses an empty query
map with IllegalStateException instead of returning false; false made the
worker keep the batch and retry it on every tick forever for a batch that
could never produce a statement.

Callers (ClickHouseBatchRunnable, ClickHouseBatchWriter) no longer read a
boolean from the grouper. NullValueColumnDropTest's
assertTrue("query generation should succeed", ok) is replaced by the
exception contract (an always-true boolean is not an assertion);
DbWriterTest / DbKafkaOffsetWriterTest drop the unused status variable.

Specs: 04.01 section 3.3 (no record is silently dropped; void contracts;
empty map refused), 10.04 section 3.5; doc/architecture_and_flow_specification.md
signature and the UPDATE grouping bullet (grouped once, previous commit).

Tests (red on the previous src/main, verified by stashing it):
GroupInsertQueryWithBatchRecordsTest.deleteWithoutBeforeImageFailsLoudly,
updateWithoutAfterImageFailsLoudly, missingColumnMapFailsLoudly (all three
threw NullPointerException before, IllegalStateException now) and
PreparedStatementExecutorNoSilentDropTest.emptyQueryMapIsRefusedNotRetried
(returned false before, throws now).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rue tombstones

Kafka Connect mode never inspected the ScheduledFuture of its
ClickHouseBatchRunnable. ScheduledThreadPoolExecutor cancels a periodic task
whose run throws and calls nobody, so after the runnable's FATAL rethrow
nothing drained the queue while put() kept enqueueing (until
sink.connector.max.queue.size blocked the Connect worker) and preCommit()
kept answering from the frozen durablyInsertedOffsets watermark: the task
consumed Kafka forever and replicated nothing, with no error after the first.

ClickHouseSinkTask.start now retains the future; put() and preCommit() call
failIfRunnableDied() first (in preCommit before the try that maps exceptions
to "commit nothing"). A done future throws ConnectException naming the task
and carrying the runnable's cause, so Kafka Connect fails the task and a
restart resumes from the last committed (durably inserted) offset. This is
the Kafka-mode counterpart of DebeziumChangeEventCapture.failIfWorkerDied.

put() dropped every record for which ClickHouseConverter.convert returned
null at DEBUG. Only a Kafka tombstone (value() == null, emitted by Debezium
after a DELETE for log compaction) carries no change event; it is now the one
record skipped. A non-null value that does not convert fails the task with
DataException naming topic, partition and offset instead of losing the change
while the offset advances.

ClickHouseBatchRunnable.logErrorToClickHouse read
firstRecord.getSourceRecord().value() unguarded; a record built from a
SinkRecord has no source record, so with error.logging.enable=true the
Kafka-mode runnable threw NullPointerException from the error logger,
replacing the ClickHouse error being reported. sourceRecordOrNull guards it,
and a runtime failure inside the (best-effort) logger is logged rather than
allowed to replace the exception whose classification must still run.

attachForTest wires the task's collaborators without a ClickHouse connection
or executor so put/preCommit can be unit-tested; start() builds the same
state.

Specs: 03.01 section 3.4 (Kafka-mode liveness contract, error-logger guard),
10.04 section 3.5 (tombstone-only skip), 10.05 consumer note.

Tests: ClickHouseSinkTaskTest.deadRunnableFailsPut / deadRunnableFailsPreCommit,
tombstoneIsDroppedQuietly, unconvertibleRecordIsLoud, liveRunnableAcceptsRecords;
ClickHouseBatchRunnableErrorLoggerTest.sourceRecordIsNullForKafkaModeRecord /
sourceRecordIsUnwrappedWhenPresent. The tests need the new test seams, so
"red before" was established by mutation against the fixed tree: removing the
two failIfRunnableDied() calls and restoring the silent null drop fails
exactly deadRunnableFailsPut, deadRunnableFailsPreCommit and
unconvertibleRecordIsLoud ("nothing was thrown"); removing the null guard
fails sourceRecordIsNullForKafkaModeRecord with the original
NullPointerException.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…tead of being reported as applied

DBMetadata.truncateTable retried a refused TRUNCATE TABLE ten times and then
fell out of its loop without throwing. PreparedStatementExecutor only rethrows
a SQLException it is handed, so the batch continued, answered true and was
acknowledged while the pre-truncate rows survived in ClickHouse: MySQL had
emptied the table, the replica had not, and the offset had moved past the
event. Spec 04.05 section 3 step 2 claimed the rethrow; the code did not do
it. The method now throws SQLException (cause: the last refusal) once every
attempt has failed, and closes the statement it prepares.

DBMetadata.getPreparedStatement returned null after its retries, which the
caller could only dereference into a NullPointerException far from the cause.
It now throws SQLException naming the statement, with the last refusal as
the cause.

The TRUNCATE this code issues is MySQL's own statement, already executed at
the source and delivered as a replicated binlog change event (op = t); it is
never issued on the connector's initiative. The seven sites counted below are
that one statement, its failure message, and test fixture / assertion text
driving a recording JDBC proxy that refuses the statement.

Specs: 04.05 section 3 step 2 now matches the code; 03.06 section 3.2 notes
the getPreparedStatement contract.

Tests (red on the previous src/main, verified by stashing it):
PreparedStatementExecutorTruncateTest.truncateRefusedByClickHouseFailsTheBatch
(no exception, result true before), DBMetadataStatementFailureTest.
truncateFailureIsRethrownAfterRetries (returned normally before),
preparedStatementFailureIsRethrownNotNull (returned null before).
RecordingJdbc (test support) is made public so the db package can use it.

Destructive-Op-Check: sites=7; result=pass
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nst the target database

A replicated MySQL TRUNCATE (change event op = t) grouped with DML in one
batch was keyed into the SAME HashMap as the INSERT templates and the
executor iterated that map. Whether the truncation ran before or after the
inserts of the same batch therefore depended on the hash of the table name:
for [INSERT r1, TRUNCATE, INSERT r2] one order resurrected r1, the other lost
r2. Two TRUNCATEs in one batch had equal keys and collapsed onto one entry.
And the statement named the SOURCE database carried by the record, which
under clickhouse.database.override.map is not the database of the table at all.

GroupInsertQueryWithBatchRecords.groupQueryWithRecords now emits an ORDERED
list of segments (List<Map<template, records>>): a truncation closes the
current segment, occupies a segment of its own and a fresh segment starts
behind it. PreparedStatementExecutor.addToPreparedStatementBatch executes
the segments strictly in order and issues the truncation for such a segment
through DBMetadata.truncateTable against ITS database, the resolved target
database of the writer; the marker key text is never executed; a truncation
record inside an INSERT group is an IllegalStateException.
ClickHouseBatchRunnable and ClickHouseBatchWriter pass the segment list. The
statement is the one MySQL already executed at the source; the connector
never issues one on its own initiative. The eleven attested sites are the
log and error text at the one call site, one formal-model arm naming the
truncate constructor, and recording-proxy test fixtures and assertion text.

Lean: new module Replication.BatchOrder (registered in lakefile roots, the
Lean README, CONSTITUTION section 5 and spec 11.03): segments_match_source /
segmented_batch_converges (segments in order reproduce binlog order for every
batch), every_truncate_is_its_own_segment (two TRUNCATEs never collapse),
truncate_last_loses_rows and truncate_first_resurrects_rows (the pre-fix
hash-map order disagrees with the source either way). lake build succeeds,
zero sorry, axioms [propext, Quot.sound] only.

Specs: 04.05 section 3 rewritten (3.1 grouping into segments, 3.2 execution
against the target database, Lean citations); 04.01 section 3.1 (ordered
segments); 03.06 section 3.2; 03.04 gains section 3.2 (every statement names
D_final); doc/architecture_and_flow_specification.md updated.

Tests: PreparedStatementExecutorTruncateTest.truncateIsAppliedAtItsBinlogPositionForBothHashOrders
(chooses two table names whose pre-fix HashMap order differs and asserts
INSERT(r1), FLUSH, TRUNCATE, INSERT(r2), FLUSH for both),
truncateTargetsTheExecutorDatabaseNotTheSourceDatabase,
twoTruncatesInOneBatchAreBothApplied. Red before, verified by mutation
against the fixed tree because the tests need the new signature: keeping the
truncation in the current map fails the ordering test (it ran first for
orders_0), the two-truncation test (collapsed) and the refused-truncation
test (r2 written); using the source database of the record fails the
target-database test with db1 instead of db1_replica.

Destructive-Op-Check: sites=11; result=pass
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…placingMergeTree that cannot be versioned or deleted from

QueryFormatter.createColumns recognised engine columns only by the default
constants (_version, is_deleted, _sign) and the configured delete column.
A table declared ReplacingMergeTree(ver) or CollapsingMergeTree(sgn) has
its version / sign column under another name; that column is never in the
source record, so createColumns omitted it as a "pre-ALTER column" and
ClickHouse stored the type default in every row: ver = 0 (a redelivered
older row wins every merge), sgn = 0 (no +1/-1 pair ever collapses). The
binder then bound the column only "if present in the index map" and moved
on at DEBUG, so nothing ever said so.

DbWriter already resolves the real names from the engine clause; they now
reach query construction on every production path:
GroupInsertQueryWithBatchRecords(versionColumn, signColumn, deleteColumn)
is built by ClickHouseBatchRunnable and ClickHouseBatchWriter from the
writer, and passes them to a new getInsertQueryUsingInputFunction overload
whose isConnectorManagedColumn treats the resolved names exactly like the
constants (the shorter overloads and the no-arg grouper delegate with null
and keep their behaviour). PreparedStatementFieldMapper.requireEngineColumnPlaceholder
refuses with IllegalStateException a row whose version / sign / delete
column exists in the table but has no placeholder in the INSERT, except in
replication-history mode whose SCD Type 2 statement emits those columns as
SQL literals by design.

DbWriter.requireReplacingMergeTreeColumns (static, package-private) refuses
a ReplacingMergeTree target whose engine clause resolves to no version
column, or to one the table lacks, or whose delete column (resolved, else
configured) is not a table column unless ignore_delete=true; the
constructor rethrows the IllegalStateException so the batch fails with the
reason instead of writing rows that are versioned 0 or resurrected on
DELETE. Names match case-insensitively like the rest of the column map.

Specs: 04.02 section 3.1 step 2 (resolved names; bind-time backstop),
04.03 section 3.2.1, 05.04 section 3, 08.01 section 3.1 step 3 and new
section 3.2, 04.01 codebase mapping.

Tests: QueryFormatterNullValueDropTest.testConnectorManagedColumnsAlwaysRetained
gains the ver / sgn / removed case; GroupInsertQueryWithBatchRecordsTest.resolvedEngineColumnsAreBoundParameters;
PreparedStatementFieldMapperEngineColumnTest (setLong at ver's index,
version / delete / sign without placeholder refused, history-mode literal
columns accepted); DbWriterEngineColumnsTest (bare RMT, missing version
column, missing delete column refused; ignore_delete and case-insensitive
matching accepted). The formatter, grouper-constructor and DbWriter tests
need the new signatures, so "red before" is by mutation against the fixed
tree: dropping the resolved names from isConnectorManagedColumn fails the
formatter and grouper tests; removing the placeholder check fails the three
refusal tests; removing the DbWriter check fails its three refusal tests.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…emantics

# Conflicts:
#	specs/03-execution-engine/06-jdbc-batch-flushing.md
…pen; refuse only the rows that need the delete marker

DbWriter.requireReplacingMergeTreeColumns (a8fca87) refused every
ReplacingMergeTree target whose delete column is not a table column.
That is too strict: INSERTs and UPDATEs replicate correctly to such a
table, and old-style ReplacingMergeTree(ver) targets without a delete
column are common -- the connector's own fixtures (employees.employees
with delete column 'sign', DbWriterTest / DbKafkaOffsetWriterTest /
DBMetadataTest.getTestGetServerTimeZone in java-tests-kafka) and the
pre-created public.schema_drift_* tables of PostgresSchemaDriftIT in
java-tests-lightweight all failed on it. Only a row that needs the
delete marker is wrong on such a table.

The writer now logs a WARN for a missing delete column and keeps the
table (a missing or absent VERSION column is still refused: every row
written to that table would be wrong). The refusal moves to the row
that needs the marker: PreparedStatementFieldMapper.requireDeleteColumn
throws IllegalStateException for a DELETE record and for the sorting-key
relocation tombstone (insertTombstonePreparedStatement, which without
the column would land as a LIVE row at the old key) of a
ReplacingMergeTree / ReplicatedReplacingMergeTree target whose delete
column is not in the column map, unless ignore_delete=true. Replication-
history mode retires rows through its own SCD Type 2 statement and is
exempt, like requireEngineColumnPlaceholder. The message names the row,
the table, the missing column, the table's columns and the remediation.

Also adapts PreparedStatementExecutorClearParametersTest (from the
merged 2.11.0 head) to the ordered segment list that
addToPreparedStatementBatch takes on this branch.

Specs: 08.01 section 3.2 rewritten (writer side warns, binder refuses
per row) and section 5; 05.02 section 3.1 (tombstone refusal) and
section 5; 04.02 section 5.

Tests: DbWriterEngineColumnsTest.rmtTargetWithoutDeleteColumnIsAcceptedAtOpen
replaces rmtTargetWithoutDeleteColumnIsRefused;
Prepa
8000
redStatementFieldMapperEngineColumnTest gains
testDeleteForTableWithoutDeleteColumnIsRefused,
testInsertForTableWithoutDeleteColumnIsAccepted,
testDeleteForTableWithoutDeleteColumnIsAcceptedWhenDeletesAreIgnored,
testRelocationTombstoneForTableWithoutDeleteColumnIsRefused. Mutation:
disabling requireDeleteColumn fails the DELETE and tombstone tests
("Expected IllegalStateException ... nothing was thrown"). Full
sink-connector unit suite: 539 run, 0 failures, 20 errors all
"Could not find a valid Docker environment" (Testcontainers classes,
no daemon here; identical to base). Spec validator PASSED (59 specs,
404 citations).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@minguyen9988
Copy link
Copy Markdown
Collaborator Author

Pushed 7f35e6f on top of a merge of the current 2.11.0 head (cb523f1, PR 1475), which resolves the conflict, and it fixes the two red CI jobs of run 35688106750. Root cause of both was the same: DbWriter.requireReplacingMergeTreeColumns (a8fca87) refused every ReplacingMergeTree target whose delete column is not a table column. That is too strict. INSERTs and UPDATEs replicate correctly to such a table, and old-style ReplacingMergeTree(ver) targets without a delete column are common: the fixtures behind DbWriterTest, DbKafkaOffsetWriterTest and DBMetadataTest.getTestGetServerTimeZone (java-tests-kafka, 3 errors, table employees.employees with delete column sign) and the pre-created public.schema_drift_* tables of PostgresSchemaDriftIT (java-tests-lightweight, 2 failures) all failed on it. Only a row that needs the delete marker is wrong on such a table. Change: the writer now logs a WARN for a missing delete column and keeps the table (a missing version column is still refused, since every row written would be wrong); PreparedStatementFieldMapper.requireDeleteColumn refuses, with IllegalStateException, a DELETE record and the sorting-key relocation tombstone for a ReplacingMergeTree target whose delete column is not in the column map (unless ignore_delete=true; replication-history mode is exempt, like requireEngineColumnPlaceholder). Also adapts PreparedStatementExecutorClearParametersTest from 2.11.0 to the ordered segment list this branch introduced. Specs 08.01 section 3.2, 05.02 section 3.1 and 04.02 section 5 updated; spec validator PASSED; 4 new mapper tests plus the rewritten writer test; mutation check: disabling requireDeleteColumn fails the DELETE and tombstone tests; full sink-connector unit suite 539 run, 0 failures, 20 Docker-environment errors identical to base.

@minguyen9988
minguyen9988 merged commit 40752a4 into 2.11.0 Sep 22, 2026
6 of 11 checks passed
@minguyen9988
minguyen9988 deleted the fix-writer-engine-semantics branch September 22, 2026 08:40
minguyen9988 added a commit that referenced this pull request Sep 22, 2026
…s; fix disable.drop.truncate swallowing snapshot DDL

Resolves the four conflicts of merging the current 2.11.0 head into the branch, all kept-both: DbWriter.java both init catch clauses (ColumnTypeOverrideMismatchException rethrown first, then IllegalStateException from #1476 logged and rethrown); Lean README lists OffsetFifo + DdlTranslation + BatchOrder; spec 10.04 unions both verification lists; spec 07.03 keeps #1476's out-of-range 3.3 and renumbers this branch's enable.time.adjuster section to 3.4, with its code cross-references (DebeziumChangeEventCapture, TimeAdjusterDefaultTest) updated to 3.4.

CI fix (java-tests-lightweight, DateTimeWithUserProvidedTimeZoneSchemaOnlyIT): this branch (794f1f2) made disable.drop.truncate live for the first time. The IT sets disable.drop.truncate=true with enable.snapshot.ddl=true, so the snapshot-phase schema-bootstrap DDL (a table drop-then-recreate) was suppressed, leaving the pre-created target table (Minimum_Value DateTime, min 1970-01-01) in place instead of the DateTime64 the snapshot recreate would build (min 1900-01-01 = DATETIME_MIN); the assertion at line 152 saw 1970-01-01 and failed.

Fix: disable.drop.truncate suppresses STREAMING drops only; snapshot-phase DDL (isSnapshotDDL(sr)) is exempt, because a snapshot recreate is schema initialisation, not a source-initiated data drop during replication. Suppressing it would freeze a stale target schema and make ClickHouse diverge from MySQL, the opposite of the option's purpose.

Spec 06.08 section 3.5 documents the snapshot exemption; the verification reference is corrected to 3.5. New unit test DdlIgnoreRulesTest.disableDropTruncateExemptsSnapshotDdl: a snapshot drop reaches execution despite disable.drop.truncate=true, while a streaming drop is still frozen. Mutation: dropping the isSnapshotDDL guard makes the snapshot assertion red.

Spec validator PASSED (59 specs, 463 citations). DdlIgnoreRulesTest 7/7. Full sink-connector unit suite 541 run, 0 failures, 20 errors all Docker-environment (Testcontainers, no daemon here; identical to base).

All 21 destructive sites are statement text parsed/classified/logged or test-code literals (no execution against any live database); the code sites carry #1476's DESTRUCTIVE: markers, unchanged by this merge.

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

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
minguyen9988 added a commit that referenced this pull request Sep 22, 2026
…-commit-order

Resolves eight conflicts of merging the current 2.11.0 head (now containing #1475/#1476/#1477) into the version-floor branch. Additive/kept-both in the Lean and spec files: both BatchOrder (#1476) and VersionFloor (this branch) listed in lakefile.lean, formal_specs/lean/README.md, CONSTITUTION.md, and spec 11.03 (numbered proof list renumbered so BatchOrder is 13 and VersionFloor 14-16); spec 04.04 keeps #1476's collapsing/grouping bullets and this branch's composite-PK bullet; spec 01.06 keeps #1477's terminal classifier plus this branch's heartbeat-sequence-state test.

The one semantic conflict is the version-assignment hot loop of DebeziumChangeEventCapture. Both behaviours are kept: #1478's VersionAssignment machinery (nextVersionAssignment + coverAssignedVersion, control records excluded from the sequence, chStruct versioned from assignment.sequenceNumber/effectiveTs) AND #1477's loud-failure path (rejectUnrepresentableValue before the sequence; parse() wrapped so a parser throw becomes RecordReplicationException; a null-parsed ROW record is terminal, not a WARN skip; the RecordReplicationException/DDLReplicationException rethrows ahead of the catch-all). NullParsedRowRecordIsTerminalTest keeps its terminal assertion, adapted to the VersionAssignment reflective signature.

Verification: lightweight compile clean; NullParsedRowRecordIsTerminalTest + ControlRecordLogLevelTest + DebeziumChangeEventCaptureTest + UnparseableRowRecordIsTerminalTest 38/38; spec validator PASSED (59 specs, 519 citations); lake build completed successfully (BatchOrder + VersionFloor both built, 0 sorry); full sink-connector unit suite 547 run, 0 failures, 20 errors all Docker-environment (identical to base). config.yml.j2 rendered (minimal + full variants) and YAML-linted with duplicate-key detection: no unrendered markers, no duplicate keys.

All 42 destructive sites in this merge are #1475/#1476/#1477 code already carrying DESTRUCTIVE: markers or test-code literals (statement text parsed/classified/logged, no execution against any live database), unchanged by this merge.

Jinja-Render-Check: rendered=2; formats=yaml; result=pass

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

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
minguyen9988 added a commit that referenced this pull request Sep 22, 2026
…ypes; fix inherited DdlIgnoreRulesTest signature

Resolves nine conflicts of merging the current 2.11.0 head (now containing #1475/#1476/#1477/#1478) into the ddl-translation branch. MySqlDDLParserListenerImpl.java: both sides added independent fields (this branch's PK-change/ddlEventTimestamp fields and #1477's dropOrTruncateStatement field + isDropOrTruncateStatement method) - kept both. Lean lakefile/README, CONSTITUTION and spec 11.03 list all of BatchOrder (#1476), VersionFloor (#1478) and this branch's CreateTable. spec 07.01 keeps this branch's DDL-path unsigned rule (§3.2) and #1478's record-path untyped-Int64 report (renumbered §3.3). spec 07.02 keeps this branch's IEEE section plus #1475's value-comparison caveat. spec 06.05 keeps both verification sets including #1478's RecordSchemaVsDdlTypeAgreementTest bullet. spec 07.06 (spatial) is a genuine two-path merge: the record-schema path keeps POINT/POLYGON native (#1475), the DDL path types every spatial column String/WKB-hex (this branch, new §3.4); they agree on every non-polygon type and the RecordSchemaVsDdlTypeAgreementTest does not cover spatial, so the code coexists.

CI fix: 2.11.0 itself is internally inconsistent - #1478 changed processEveryChangeRecord's last parameter to VersionAssignment but DdlIgnoreRulesTest (added by #1477) still reflected long.class, so disableDropTruncateIsScopedAndLive and disableDropTruncateExemptsSnapshotDdl threw NoSuchMethodException. This is the java-tests-lightweight failure the branch inherits through the merge. Fixed the reflective lookup and call to use VersionAssignment (mirroring NullParsedRowRecordIsTerminalTest). The same one-line fix should be back-ported to 2.11.0 directly.

Verification: spec validator PASSED (59 specs, 534 citations); lightweight compiles; DdlIgnoreRulesTest + MySqlDDLParserListenerImplTest + DataTypeConverterTest + CreateTableNoKeySortKeyTest + CreateTableUniqueKeySortKeyTest + NullParsedRowRecordIsTerminalTest 175/175 (1 Docker skip); full lightweight unit suite 556 run, 0 failures, 62 errors all Testcontainers/Docker (no daemon here). config.yml.j2 rendered (minimal + full) and YAML-linted, no duplicate keys.

All destructive sites in this merge are #1475/#1476/#1477/#1478 code with DESTRUCTIVE: markers or test-code literals, unchanged by this merge.

Jinja-Render-Check: rendered=2; formats=yaml; result=pass

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

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
minguyen9988 added a commit that referenced this pull request Sep 22, 2026
Rebases the CollapsingMergeTree clear-parameters fix onto the current 2.11.0 head, which now contains #1475-#1480. #1476 was squash-merged, so its commits on this branch conflicted with the same content already in 2.11.0; every such conflict (Lean lakefile/README, CONSTITUTION, specs 04.04/10.04/11.03, DbWriter) is resolved by taking the 2.11.0 side, which carries the accumulated #1476-#1480 content that subsumes this branch's #1476 commits. The only net change this branch adds over 2.11.0 is the one-file CollapsingMergeTree fix in PreparedStatementExecutor.java, re-applied on top of 2.11.0's version.

The fix: the CollapsingMergeTree UPDATE branch stages the -1 cancel row with ps.addBatch() but did not call ps.clearParameters() afterward, unlike the ReplacingMergeTree relocation-tombstone branch. The V2 JDBC driver retains bound values across addBatch(), so a parameter the after image does not rebind could inherit the cancel row's sign=-1 and lose the UPDATE. Defensive-consistency fix (Gemini panel finding on #1476, demoted-unproven because the sign is always rebound in practice); matches the RMT tombstone branch and the general per-row path (Spec 07.07 s3.2.2, Spec 05.04 s3.1).

Verification: net PR diff vs 2.11.0 is exactly PreparedStatementExecutor.java; sink-connector compiles; PreparedStatementExecutorCollapsingSignTest + ClearParametersTest + SortingKeyTombstoneTest + TruncateTest 17/17; spec validator PASSED (59 specs, 534 citations); config.yml.j2 rendered (minimal + full) and YAML-linted, no duplicate keys.

All 25 destructive sites in this merge are #1475-#1480 code with DESTRUCTIVE: markers or test-code literals, unchanged by this merge.

Jinja-Render-Check: rendered=2; formats=yaml; result=pass

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

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
minguyen9988 added a commit that referenced this pull request Sep 22, 2026
* fix(writer): detect sorting-key relocation on stored values, not Connect-schema defaults

updateRelocatesSortingKey compared before.get(C) with after.get(C).
Struct.get substitutes schema.defaultValue() for a stored null, and Debezium
fills that default from the MySQL column DEFAULT, so an UPDATE that moved a
sorting-key column from NULL to its DEFAULT value compared equal, was judged
an in-place update, and the pre-update row at the NULL key was never
tombstoned: MySQL held one row, ClickHouse two, forever. The bind path already
reads stored values with getWithoutDefault (spec 04.03 section 3.3); the
relocation decision now observes the same values the rows are written with.

Spec 05.01 section 3.1 step 3 now names getWithoutDefault and a new section
3.2 states the rule and the failure it prevents.
PreparedStatementExecutorSortingKeyTombstoneTest.testNullToSchemaDefaultInSortingKeyIsAChange
fails on the previous code (NULL -> 'x' under DEFAULT 'x' returned false) and
passes with the fix; mutation-checked by reverting the comparison.

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

* fix(writer): stage the CollapsingMergeTree cancel row and group each UPDATE once

Two coupled defects on the UPDATE path.

GroupInsertQueryWithBatchRecords appended an UPDATE record once for its
before image and once for its after image. Both images resolve to the same
INSERT template, so the same record landed twice in the same list and
PreparedStatementExecutor bound and wrote it twice: 2x write amplification on
ReplacingMergeTree, and on CollapsingMergeTree two +1 rows per update. The
record is now grouped once, under the after-image template; the executor
binds whichever images the engine needs from that one entry.

PreparedStatementExecutor bound the before image with sign = -1 for a
CollapsingMergeTree target but never called addBatch() before binding the
after image over the same parameters, so the -1 cancel row never reached
ClickHouse and nothing ever collapsed. The before image is now staged with
its own addBatch() before the after image is bound: one UPDATE stages
[-1 (before image), +1 (after image)].

Specs: 04.01 gains section 3.2 (one record, one entry); 04.04 section 3.1
states the once-only grouping and the CollapsingMergeTree sequence; 05.04
gains section 3.1 with the exact bind/addBatch order and the failure it
prevents.

Tests: GroupInsertQueryHistoryMultiRowTest.standardModeStillSplitsUpdateIntoBeforeAndAfter
pinned the defect (asserted 40 grouped records for 20 UPDATEs) and is
deliberately flipped to standardModeGroupsEachUpdateOnce (20), plus
standardModeGroupsOneUpdateUnderOneTemplateOnce (one template, list size 1).
New PreparedStatementExecutorCollapsingSignTest drives the real grouping and
executor against a recording JDBC proxy (RecordingJdbc, new test support) and
asserts the sign bound at each addBatch(): failed before with [1, 1], passes
with [-1, 1]; INSERT stages [1], DELETE stages [-1]. Mutation-checked by
reverting each half separately.

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

* fix(writer): never drop a record at the grouping stage; refuse an empty batch loudly

GroupInsertQueryWithBatchRecords.updateQueryToRecordsMap answered false when
a record could not be turned into a statement, and groupQueryWithRecords
reported only the LAST record's answer, so the contract allowed a per-record
skip with the batch's offset still advancing past it. In practice the
missing-image case (a DELETE with no before image, an INSERT/UPDATE with no
after image) never reached that return: the null template was dereferenced
first and the worker saw an opaque NullPointerException, retrying the batch
forever with no hint of which record or image was missing. An UPDATE lacking
its after image was grouped by its before image alone and written as a live
row of the pre-update values. A null column map was a NullPointerException
from inside the formatter.

Both methods now return void: a record is grouped or the batch fails with
IllegalStateException naming the record (operation, topic, partition,
offset), the table and the missing image / metadata. The unreachable
"RECORD DROPPED: INVALID CDC RECORD STATE" log line is an exception too, as
is the executor's "INVALID CDC RECORD STATE" branch, which would otherwise
have staged the statement with the previous row's parameters.

PreparedStatementExecutor.addToPreparedStatementBatch refuses an empty query
map with IllegalStateException instead of returning false; false made the
worker keep the batch and retry it on every tick forever for a batch that
could never produce a statement.

Callers (ClickHouseBatchRunnable, ClickHouseBatchWriter) no longer read a
boolean from the grouper. NullValueColumnDropTest's
assertTrue("query generation should succeed", ok) is replaced by the
exception contract (an always-true boolean is not an assertion);
DbWriterTest / DbKafkaOffsetWriterTest drop the unused status variable.

Specs: 04.01 section 3.3 (no record is silently dropped; void contracts;
empty map refused), 10.04 section 3.5; doc/architecture_and_flow_specification.md
signature and the UPDATE grouping bullet (grouped once, previous commit).

Tests (red on the previous src/main, verified by stashing it):
GroupInsertQueryWithBatchRecordsTest.deleteWithoutBeforeImageFailsLoudly,
updateWithoutAfterImageFailsLoudly, missingColumnMapFailsLoudly (all three
threw NullPointerException before, IllegalStateException now) and
PreparedStatementExecutorNoSilentDropTest.emptyQueryMapIsRefusedNotRetried
(returned false before, throws now).

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

* fix(kafka): fail the sink task when its runnable is dead; skip only true tombstones

Kafka Connect mode never inspected the ScheduledFuture of its
ClickHouseBatchRunnable. ScheduledThreadPoolExecutor cancels a periodic task
whose run throws and calls nobody, so after the runnable's FATAL rethrow
nothing drained the queue while put() kept enqueueing (until
sink.connector.max.queue.size blocked the Connect worker) and preCommit()
kept answering from the frozen durablyInsertedOffsets watermark: the task
consumed Kafka forever and replicated nothing, with no error after the first.

ClickHouseSinkTask.start now retains the future; put() and preCommit() call
failIfRunnableDied() first (in preCommit before the try that maps exceptions
to "commit nothing"). A done future throws ConnectException naming the task
and carrying the runnable's cause, so Kafka Connect fails the task and a
restart resumes from the last committed (durably inserted) offset. This is
the Kafka-mode counterpart of DebeziumChangeEventCapture.failIfWorkerDied.

put() dropped every record for which ClickHouseConverter.convert returned
null at DEBUG. Only a Kafka tombstone (value() == null, emitted by Debezium
after a DELETE for log compaction) carries no change event; it is now the one
record skipped. A non-null value that does not convert fails the task with
DataException naming topic, partition and offset instead of losing the change
while the offset advances.

ClickHouseBatchRunnable.logErrorToClickHouse read
firstRecord.getSourceRecord().value() unguarded; a record built from a
SinkRecord has no source record, so with error.logging.enable=true the
Kafka-mode runnable threw NullPointerException from the error logger,
replacing the ClickHouse error being reported. sourceRecordOrNull guards it,
and a runtime failure inside the (best-effort) logger is logged rather than
allowed to replace the exception whose classification must still run.

attachForTest wires the task's collaborators without a ClickHouse connection
or executor so put/preCommit can be unit-tested; start() builds the same
state.

Specs: 03.01 section 3.4 (Kafka-mode liveness contract, error-logger guard),
10.04 section 3.5 (tombstone-only skip), 10.05 consumer note.

Tests: ClickHouseSinkTaskTest.deadRunnableFailsPut / deadRunnableFailsPreCommit,
tombstoneIsDroppedQuietly, unconvertibleRecordIsLoud, liveRunnableAcceptsRecords;
ClickHouseBatchRunnableErrorLoggerTest.sourceRecordIsNullForKafkaModeRecord /
sourceRecordIsUnwrappedWhenPresent. The tests need the new test seams, so
"red before" was established by mutation against the fixed tree: removing the
two failIfRunnableDied() calls and restoring the silent null drop fails
exactly deadRunnableFailsPut, deadRunnableFailsPreCommit and
unconvertibleRecordIsLoud ("nothing was thrown"); removing the null guard
fails sourceRecordIsNullForKafkaModeRecord with the original
NullPointerException.

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

* fix(writer): a TRUNCATE ClickHouse keeps refusing fails the batch instead of being reported as applied

DBMetadata.truncateTable retried a refused TRUNCATE TABLE ten times and then
fell out of its loop without throwing. PreparedStatementExecutor only rethrows
a SQLException it is handed, so the batch continued, answered true and was
acknowledged while the pre-truncate rows survived in ClickHouse: MySQL had
emptied the table, the replica had not, and the offset had moved past the
event. Spec 04.05 section 3 step 2 claimed the rethrow; the code did not do
it. The method now throws SQLException (cause: the last refusal) once every
attempt has failed, and closes the statement it prepares.

DBMetadata.getPreparedStatement returned null after its retries, which the
caller could only dereference into a NullPointerException far from the cause.
It now throws SQLException naming the statement, with the last refusal as
the cause.

The TRUNCATE this code issues is MySQL's own statement, already executed at
the source and delivered as a replicated binlog change event (op = t); it is
never issued on the connector's initiative. The seven sites counted below are
that one statement, its failure message, and test fixture / assertion text
driving a recording JDBC proxy that refuses the statement.

Specs: 04.05 section 3 step 2 now matches the code; 03.06 section 3.2 notes
the getPreparedStatement contract.

Tests (red on the previous src/main, verified by stashing it):
PreparedStatementExecutorTruncateTest.truncateRefusedByClickHouseFailsTheBatch
(no exception, result true before), DBMetadataStatementFailureTest.
truncateFailureIsRethrownAfterRetries (returned normally before),
preparedStatementFailureIsRethrownNotNull (returned null before).
RecordingJdbc (test support) is made public so the db package can use it.

Destructive-Op-Check: sites=7; result=pass
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(writer): apply a replicated TRUNCATE at its binlog position, against the target database

A replicated MySQL TRUNCATE (change event op = t) grouped with DML in one
batch was keyed into the SAME HashMap as the INSERT templates and the
executor iterated that map. Whether the truncation ran before or after the
inserts of the same batch therefore depended on the hash of the table name:
for [INSERT r1, TRUNCATE, INSERT r2] one order resurrected r1, the other lost
r2. Two TRUNCATEs in one batch had equal keys and collapsed onto one entry.
And the statement named the SOURCE database carried by the record, which
under clickhouse.database.override.map is not the database of the table at all.

GroupInsertQueryWithBatchRecords.groupQueryWithRecords now emits an ORDERED
list of segments (List<Map<template, records>>): a truncation closes the
current segment, occupies a segment of its own and a fresh segment starts
behind it. PreparedStatementExecutor.addToPreparedStatementBatch executes
the segments strictly in order and issues the truncation for such a segment
through DBMetadata.truncateTable against ITS database, the resolved target
database of the writer; the marker key text is never executed; a truncation
record inside an INSERT group is an IllegalStateException.
ClickHouseBatchRunnable and ClickHouseBatchWriter pass the segment list. The
statement is the one MySQL already executed at the source; the connector
never issues one on its own initiative. The eleven attested sites are the
log and error text at the one call site, one formal-model arm naming the
truncate constructor, and recording-proxy test fixtures and assertion text.

Lean: new module Replication.BatchOrder (registered in lakefile roots, the
Lean README, CONSTITUTION section 5 and spec 11.03): segments_match_source /
segmented_batch_converges (segments in order reproduce binlog order for every
batch), every_truncate_is_its_own_segment (two TRUNCATEs never collapse),
truncate_last_loses_rows and truncate_first_resurrects_rows (the pre-fix
hash-map order disagrees with the source either way). lake build succeeds,
zero sorry, axioms [propext, Quot.sound] only.

Specs: 04.05 section 3 rewritten (3.1 grouping into segments, 3.2 execution
against the target database, Lean citations); 04.01 section 3.1 (ordered
segments); 03.06 section 3.2; 03.04 gains section 3.2 (every statement names
D_final); doc/architecture_and_flow_specification.md updated.

Tests: PreparedStatementExecutorTruncateTest.truncateIsAppliedAtItsBinlogPositionForBothHashOrders
(chooses two table names whose pre-fix HashMap order differs and asserts
INSERT(r1), FLUSH, TRUNCATE, INSERT(r2), FLUSH for both),
truncateTargetsTheExecutorDatabaseNotTheSourceDatabase,
twoTruncatesInOneBatchAreBothApplied. Red before, verified by mutation
against the fixed tree because the tests need the new signature: keeping the
truncation in the current map fails the ordering test (it ran first for
orders_0), the two-truncation test (collapsed) and the refused-truncation
test (r2 written); using the source database of the record fails the
target-database test with db1 instead of db1_replica.

Destructive-Op-Check: sites=11; result=pass
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(writer): bind engine columns by their resolved names; refuse a ReplacingMergeTree that cannot be versioned or deleted from

QueryFormatter.createColumns recognised engine columns only by the default
constants (_version, is_deleted, _sign) and the configured delete column.
A table declared ReplacingMergeTree(ver) or CollapsingMergeTree(sgn) has
its version / sign column under another name; that column is never in the
source record, so createColumns omitted it as a "pre-ALTER column" and
ClickHouse stored the type default in every row: ver = 0 (a redelivered
older row wins every merge), sgn = 0 (no +1/-1 pair ever collapses). The
binder then bound the column only "if present in the index map" and moved
on at DEBUG, so nothing ever said so.

DbWriter already resolves the real names from the engine clause; they now
reach query construction on every production path:
GroupInsertQueryWithBatchRecords(versionColumn, signColumn, deleteColumn)
is built by ClickHouseBatchRunnable and ClickHouseBatchWriter from the
writer, and passes them to a new getInsertQueryUsingInputFunction overload
whose isConnectorManagedColumn treats the resolved names exactly like the
constants (the shorter overloads and the no-arg grouper delegate with null
and keep their behaviour). PreparedStatementFieldMapper.requireEngineColumnPlaceholder
refuses with IllegalStateException a row whose version / sign / delete
column exists in the table but has no placeholder in the INSERT, except in
replication-history mode whose SCD Type 2 statement emits those columns as
SQL literals by design.

DbWriter.requireReplacingMergeTreeColumns (static, package-private) refuses
a ReplacingMergeTree target whose engine clause resolves to no version
column, or to one the table lacks, or whose delete column (resolved, else
configured) is not a table column unless ignore_delete=true; the
constructor rethrows the IllegalStateException so the batch fails with the
reason instead of writing rows that are versioned 0 or resurrected on
DELETE. Names match case-insensitively like the rest of the column map.

Specs: 04.02 section 3.1 step 2 (resolved names; bind-time backstop),
04.03 section 3.2.1, 05.04 section 3, 08.01 section 3.1 step 3 and new
section 3.2, 04.01 codebase mapping.

Tests: QueryFormatterNullValueDropTest.testConnectorManagedColumnsAlwaysRetained
gains the ver / sgn / removed case; GroupInsertQueryWithBatchRecordsTest.resolvedEngineColumnsAreBoundParameters;
PreparedStatementFieldMapperEngineColumnTest (setLong at ver's index,
version / delete / sign without placeholder refused, history-mode literal
columns accepted); DbWriterEngineColumnsTest (bare RMT, missing version
column, missing delete column refused; ignore_delete and case-insensitive
matching accepted). The formatter, grouper-constructor and DbWriter tests
need the new signatures, so "red before" is by mutation against the fixed
tree: dropping the resolved names from isConnectorManagedColumn fails the
formatter and grouper tests; removing the placeholder check fails the three
refusal tests; removing the DbWriter check fails its three refusal tests.

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

* fix(writer): accept a ReplacingMergeTree without a delete column at open; refuse only the rows that need the delete marker

DbWriter.requireReplacingMergeTreeColumns (a8fca87) refused every
ReplacingMergeTree target whose delete column is not a table column.
That is too strict: INSERTs and UPDATEs replicate correctly to such a
table, and old-style ReplacingMergeTree(ver) targets without a delete
column are common -- the connector's own fixtures (employees.employees
with delete column 'sign', DbWriterTest / DbKafkaOffsetWriterTest /
DBMetadataTest.getTestGetServerTimeZone in java-tests-kafka) and the
pre-created public.schema_drift_* tables of PostgresSchemaDriftIT in
java-tests-lightweight all failed on it. Only a row that needs the
delete marker is wrong on such a table.

The writer now logs a WARN for a missing delete column and keeps the
table (a missing or absent VERSION column is still refused: every row
written to that table would be wrong). The refusal moves to the row
that needs the marker: PreparedStatementFieldMapper.requireDeleteColumn
throws IllegalStateException for a DELETE record and for the sorting-key
relocation tombstone (insertTombstonePreparedStatement, which without
the column would land as a LIVE row at the old key) of a
ReplacingMergeTree / ReplicatedReplacingMergeTree target whose delete
column is not in the column map, unless ignore_delete=true. Replication-
history mode retires rows through its own SCD Type 2 statement and is
exempt, like requireEngineColumnPlaceholder. The message names the row,
the table, the missing column, the table's columns and the remediation.

Also adapts PreparedStatementExecutorClearParametersTest (from the
merged 2.11.0 head) to the ordered segment list that
addToPreparedStatementBatch takes on this branch.

Specs: 08.01 section 3.2 rewritten (writer side warns, binder refuses
per row) and section 5; 05.02 section 3.1 (tombstone refusal) and
section 5; 04.02 section 5.

Tests: DbWriterEngineColumnsTest.rmtTargetWithoutDeleteColumnIsAcceptedAtOpen
replaces rmtTargetWithoutDeleteColumnIsRefused;
PreparedStatementFieldMapperEngineColumnTest gains
testDeleteForTableWithoutDeleteColumnIsRefused,
testInsertForTableWithoutDeleteColumnIsAccepted,
testDeleteForTableWithoutDeleteColumnIsAcceptedWhenDeletesAreIgnored,
testRelocationTombstoneForTableWithoutDeleteColumnIsRefused. Mutation:
disabling requireDeleteColumn fails the DELETE and tombstone tests
("Expected IllegalStateException ... nothing was thrown"). Full
sink-connector unit suite: 539 run, 0 failures, 20 errors all
"Could not find a valid Docker environment" (Testcontainers classes,
no daemon here; identical to base). Spec validator PASSED (59 specs,
404 citations).

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

* fix(writer): clear JDBC bind state after the CollapsingMergeTree cancel row

Judge-panel finding (Gemini, PR #1476 round 4): the CollapsingMergeTree UPDATE branch of PreparedStatementExecutor staged the -1 cancel row with ps.addBatch() but did not call ps.clearParameters() afterward, while the ReplacingMergeTree relocation-tombstone branch immediately below it does. The V2 ClickHouse JDBC driver retains bound values across addBatch(), so any parameter the after image does not rebind would silently inherit the cancel row's value -- including sign=-1, which would turn the live +1 row into a second cancel row and lose the UPDATE.

In practice the after image always rebinds the sign column, so no leak is observed today (the finding was demoted UNPROVEN); this is a defensive-consistency fix that makes the CMT branch match the RMT tombstone branch and the general per-row path, both of which clear the bind state after addBatch (Spec 07.07 section 3.2.2, Spec 05.04 section 3.1).

No behaviour change under the existing RecordingJdbc harness (which resets params on addBatch); PreparedStatementExecutorCollapsingSignTest + ClearParametersTest + SortingKeyTombstoneTest + TruncateTest 17/17 pass.

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

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already 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