Repository navigation
db_compare: MySQL/ClickHouse checksum column-set asymmetry (approach under revision) - #1429
Merged
minguyen9988 merged 18 commits intoSep 2, 2026
Merged
Conversation
Added release notes for 2.10.1 release
…1096) (#1413) The /metrics handler wrote its body without ever setting a Content-Type header. Prometheus 3.x treats that as a non-compliant target and fails the scrape outright: non-compliant scrape target sending blank Content-Type and no fallback_scrape_protocol specified for target so the connector silently stopped being monitored after a Prometheus upgrade. Set the Prometheus text exposition content type, "text/plain; version=0.0.4; charset=utf-8", before sendResponseHeaders, which is the call that commits the headers. Also encode the body once instead of calling response.getBytes() twice. That is a real correctness bug, not tidying: the two calls each used the platform default charset, so the length declared in the headers and the bytes actually written could differ for non-ASCII metric labels under a non-UTF-8 default charset, leaving a scraper short of the promised Content-Length. Encoding once to UTF-8 and using that array length makes the declared length true and matches the charset now advertised. Regression test MetricsContentTypeTest starts the real metrics server on a free port and scrapes it over HTTP, since the header is only observable on the wire. Verified failing first: Content-Type was null ("expected: not <null>"); it is the expected value after the fix. The Content-Length assertion passed before the fix too -- the current scrape output is ASCII -- and is there to pin the invariant, not to reproduce a live failure.
#1414) convert() groups INT8 and INT32 into a single isFieldTypeInt branch whose terminal case was: ps.setInt(index, (Integer) value); An INT8 schema delivers a java.lang.Byte, never an Integer, so this threw ClassCastException: class java.lang.Byte cannot be cast to class java.lang.Integer as reported -- and because it aborts the batch, the connector could not replicate any table holding a MySQL TINYINT. The isWiderIntegerTarget hatch immediately above does not cover it: it only fires for Int32/UInt32/Int64/UInt64 ClickHouse targets, and an INT8 source column maps to ClickHouse Int8 (Int32_SCHEMA -> Int8 in dataTypesMap), or UInt8 when the column is unsigned. Read the value as a Number and bind intValue(), so the boxed type no longer has to be guessed. Byte, Short and Integer all widen correctly and keep their sign. For the INT32 case that works today this is a no-op: Integer.intValue() returns the same int the cast produced, so the same setInt call with the same value is issued. Regression test ClickHouseDataTypeMapperInt8Test pins TINYINT to Int8 and UInt8 targets and negative/boundary bytes (-1, MIN_VALUE, MAX_VALUE), with two controls: INT32 still binds identically via setInt, and the isWiderIntegerTarget path still routes to setObject. Verified failing first: the 3 INT8 tests errored with the ClassCastException above while both controls passed; all 5 pass after the fix.
…#749) (#1415) The ARRAY branch of ClickHouseDataTypeMapper.convert cast the incoming value to java.util.ArrayList: ps.setArray(index, ps.getConnection().createArrayOf( dt.name(), ((ArrayList) value).toArray())); But the Kafka Connect contract for an array field is java.util.List, not ArrayList. Debezium and the Connect converters may hand over any List implementation, and an empty array in particular arrives as Collections.emptyList(). The cast then throws ClassCastException: class java.util.Collections$EmptyList cannot be cast to class java.util.ArrayList which fails the entire batch -- exactly the frame in the reporter stack trace (ClickHouseDataTypeMapper.convert, from PreparedStatementExecutor.insertPreparedStatement). Bind through java.util.Collection instead. Every value that can reach this branch is a Connect array, i.e. a List, so Collection covers all of them; Collection.toArray() returns the same Object[] the ArrayList cast produced, so the case that worked before is unchanged. Regression test ClickHouseDataTypeMapperArrayTest pins the reporter Collections.emptyList() case plus List.of, Arrays.asList and LinkedList, and keeps an ArrayList control asserting identical bound elements. Verified failing first: 4 of 5 errored with the ClassCastException above (the ArrayList control passed); all 5 pass after the fix.
…rd (#1416) * fix: initial snapshot never finishes -- NPE on every unparseable record (#1379) Root cause ---------- processEveryChangeRecord dereferenced the parser's return value before testing it for null: chStruct = debeziumRecordParserService.parse(record, recordCommitter, lastRecordInBatch); chStruct.setSequenceNumber(sequenceNumber); try { if (chStruct != null) { Returning null is contract, not failure. SourceRecordParserService.parse returns null for a null SourceRecord or null value (line 57), for a value that is not a Struct (line 62), and for a value the converter cannot map (line 85). The `if (chStruct != null)` on the very next line is the method's own admission that null is expected. Every such record therefore raised java.lang.NullPointerException: Cannot invoke "...ClickHouseStruct.setSequenceNumber(long)" because "chStruct" is null which the method's outer `catch (Exception e)` swallowed. The record was dropped silently and the snapshot loop logged the same stack trace once per record -- the two symptoms reported in #1379: an initial snapshot that never completes, and NPE spam. Fix --- Move setSequenceNumber inside the existing null check and make the null case observable with a WARN naming the record, instead of a swallowed NPE. No other change; the surrounding logic is untouched. Proof ----- NullParsedRecordSkipTest drives processEveryChangeRecord with a parser stubbed to return null (the documented path) and captures what the class logs via a log4j2 appender. Before the fix, the test fails and quotes back the reporter's exception verbatim: [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 java.lang.AssertionError: an unconvertible record must not raise a NullPointerException; issue #1379 logged one per record for the whole snapshot, got: java.lang.NullPointerException: Cannot invoke "com.altinity.clickhouse.sink.connector.model.ClickHouseStruct.setSequenceNumber(long)" because "chStruct" is null After the fix: [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 No regression. Module suite (-pl sink-connector-lightweight -am test, -Dmaven.test.failure.ignore=true so the reactor reaches both modules; the 50 errors are *IT tests that need a Docker environment unavailable here, identical in both runs): baseline (stashed): Tests run: 288, Failures: 0, Errors: 50, Skipped: 11 with this change: Tests run: 289, Failures: 0, Errors: 50, Skipped: 11 A per-test-class diff of the two runs shows exactly one line of difference: the added NullParsedRecordSkipTest. Fixes #1379 * test: end-to-end regression coverage for #1379
…ion (#1417) * fix: MySQL DOUBLE auto-created as Float32, silently truncating precision ClickHouseDataTypeMapper.dataTypesMap mapped BOTH FLOAT32 and FLOAT64 to ClickHouseDataType.Float32. That map is what types the columns of auto-created tables, so every replicated MySQL DOUBLE / Kafka Connect FLOAT64 landed in a 32-bit ClickHouse column: ~15 significant decimal digits truncated to ~7. Nothing was logged and row counts still matched, so checksum jobs reported the table clean. The example in the issue, 123456789.123456789, is stored as 123456792. Blast radius of the map, every consumer traced: - ClickHouseTableOperationsBase.getColumnNameToCHDataTypeMapping:127 picks the column type for CREATE TABLE (via ClickHouseAutoCreateTable.createNewTable:68) and for the ADD COLUMN clause of an auto-ALTER (via ClickHouseAlterTable.alterTable:133). This is the defect site, and it only affects tables created from now on -- a wider column for new tables, no rewrite of existing ones. - DataTypeConverter:196 (lightweight DDL path) already returns Float64 for Types.DOUBLE at :189, before this lookup is reached, so that path was already correct and is unchanged. - ClickHouseDataTypeMapper.convert:526 uses it only for the ARRAY element type, where widening a Float64 array element is the same correction. Existing tables are unaffected: the write path does not consult this map. PreparedStatementFieldMapper:274 resolves the target type per column from live table metadata (DBMetadata.getColumnsDataTypesForTable, querying system.columns), so a column already declared Float32 keeps binding exactly as before. The bind path is deliberately left alone. convert():325 narrows with setFloat only for a genuine Float; a Double goes through ClickHouseDoubleValue.of(v).asBigDecimal(), which is lossless -- verified against clickhouse-data 0.9.8: 0.1234567890123456 binds unchanged, where (float) of the same value gives 0.12345679. The narrowing was purely in the declared column type. FLOAT32 stays Float32. Its narrowing already happened inside Debezium, and widening the column would only promise a precision the pipeline cannot deliver -- the same reasoning DataTypeConverter documents for Types.REAL. The defect was invisible to the existing auto-create tests because ClickHouseAutoCreateTableBase.getExpectedColumnToDataTypesMap hand-builds the column map with the CORRECT Float64 entries rather than deriving it from the mapper, so the wrong lookup was never exercised. The new tests drive the real mapper and the real auto-create consumer. Verified failing before the fix: float64MapsToFloat64:57 expected Float64 but was Float32 autoCreatedColumnForDoubleIsFloat64:86 expected Float64 but was Float32 and passing after (4/4). Suite for -pl sink-connector -am, baseline vs post: before Tests run: 226, Failures: 0, Errors: 20 after Tests run: 230, Failures: 0, Errors: 20 The 20 errors are the pre-existing docker-unavailable ones and are an identical set in both runs (diff of the error list is empty); the whole delta is the 4 new tests. Fixes #1262 * test: end-to-end regression coverage for #1262
* fix: support PostgreSQL infinity values in timestamptz (#1231) PostgreSQL timestamptz accepts the special values `infinity` and `-infinity`. Debezium delivers them verbatim as those literal strings (PostgresValueConverter#convertTimestampWithZone returns the constants POSITIVE_INFINITY / NEGATIVE_INFINITY, confirmed in the bytecode of debezium-connector-postgres 3.1.3.Final). ZonedTimestampConverter.convert() initialises `result` to "" and tries each of its 17 patterns in turn. None of them match `infinity`, so the loop 8000 falls through to the error log and returns the empty string -- the row is delivered to ClickHouse with an empty value rather than failing, which is exactly the symptom reported in #1231: ERROR - Error parsing zonedtimestamp -infinity ClickHouse DateTime64 cannot represent an actual infinity, so saturate to the bounds of the target type instead. These are the same bounds the method already clamps out-of-range timestamps to a few lines below (BinaryStreamUtils.DATETIME64_MAX / DATETIME64_MIN, i.e. 2299-12-31 23:59:59 and 1900-01-01 00:00:00), so no new sentinel is introduced and the PostgreSQL ordering semantics are preserved: `infinity` sorts after and `-infinity` before every other representable timestamp. The change is deliberately scoped to the two known literals. The wider silent-empty-string behaviour for genuinely unparseable input is left as-is: turning it into a throw would change the failure mode of every existing deployment from degraded to hard-stopped, which does not belong in a targeted bug fix. Test: testZonedTimestampConverterInfinity fails before the change with `expected:<[2299-12-31 23:59:59.000000]> but was:<[]>` and passes after. * test: end-to-end regression coverage for #1231
#1419) With auto.create.tables disabled and a table absent from ClickHouse, the single-threaded writer path drops the batch and every topic ordered after the failing one, with nothing but a log line to show for it. processRecordsByTopic returns false once the metadata retries are exhausted for a table it cannot read. persistRecords broke out of the per-topic loop on that and returned NORMALLY, and its outer catch swallowed anything thrown. The caller, DebeziumChangeEventCapture.appendToRecords, has no return value to inspect and no exception to catch, so handleBatch completed as if the batch had been written. The acknowledgement block is skipped, so THIS batch is never committed -- but the engine moves on to the next batch, whose markBatchFinished() commits a HIGHER offset. The unwritten records are then behind the committed offset and are never replayed: permanent loss, while the connector looks healthy. persistRecords now raises BatchPersistenceException instead of returning quietly, so the failure reaches the Debezium engine, which stops and restarts from the last committed offset and replays the records. The outer catch no longer swallows: the only exception it still absorbs is an interrupt, which means the task is shutting down rather than failing to write, and turning that into a persistence failure would make a clean stop trigger a restart loop. Scoped to ClickHouseBatchWriter. ClickHouseBatchRunnable carries the same break but survives it -- it clears currentBatch only when the result is true, so it re-polls and retries the same batch -- and is deliberately left alone. This writer is invoked once per batch and has no retry loop, so failing loudly is the only way for it not to lose data. Test: ClickHouseBatchWriterMissingTableTest drives wasTableMetaDataRetrieved() to false through the same state a missing table produces and asserts that no record is acknowledged AND that the failure reaches the caller. Verified failing first on the unmodified tree: AssertionError: persistRecords returned normally after failing to write the batch: the caller cannot tell the batch was dropped, so the offset advances past records that never reached ClickHouse (issue #1285) Module suite: 226 run / 0 failures / 20 errors before, 228 run / 0 failures / 20 errors after -- the 20 errors are testcontainers tests with no Docker environment on the build host, identical in both runs.
) On a MySQL source with GTID disabled, the _version column was filled with 18446744073709551615 and the JDBC driver then refused to read it back ("integer overflow: 18446744073709551615 cannot be presented as long"). Chain: - ClickHouseStruct.version is initialised to UNINITIALIZED_VALUE = -1 (ClickHouseStruct.java:48, :204). - calculateVersion() only assigned a version from gtid, sequenceNumber or lsn (:823-834) and fell through, leaving -1, when all three were absent. setGtid is only reached when source.gtid splits into exactly 2 segments (:477-482), which never happens without GTID; nothing in sink-connector/src/main ever calls setSequenceNumber (only the lightweight DebeziumChangeEventCapture does), so the Kafka Connect path has no sequence number either; and lsn is a Postgres-only field. - handleVersionColumn then executed ps.setLong(index, -1) (PreparedStatementFieldMapper.java:503) into the UInt64 _version column, which stores 18446744073709551615. The unreadable cell was only the visible symptom. UInt64 max is the largest representable version, so under ReplacingMergeTree that row wins every future deduplication permanently and all later updates and deletes for the key are silently discarded on merge -- unrecoverable data loss. Fix, in two parts: 1. calculateVersion() gains a final fallback for the case that previously fell through: SnowFlakeId.generate(ts_ms, kafkaOffset). This reuses the connector's existing, already shipping version formula, substituting the Kafka offset for the absent GTID. Both inputs are populated on exactly the affected path -- ts_ms from source.ts_ms (set in setAdditionalMetaData) and the offset from record.kafkaOffset() -- and both are monotonic per source, so versions order by source commit time first and by offset within the same millisecond, the same ordering the GTID form produces. 2. The two bind sites (handleVersionColumn and insertTombstonePreparedStatement) call rejectUnderivableVersion(), which throws IllegalStateException naming the record and the value that would have been stored. After (1) this is only reachable when the record carries no ordering key AND no source timestamp, i.e. a malformed event. Failing the batch is strictly better than writing a row that can never be superseded. No existing deployment changes: a source supplying a gtid, sequenceNumber or lsn still takes its existing branch untouched, so the 2.8.0-compatible version domain of the lightweight connector and the Postgres LSN domain are unaffected. The new fallback is reachable only from the branch that was previously corrupt, and it is applied regardless of the snowflake.id flag because without a GTID there is no raw transaction id for the non-snowflake branch to use. VersionFallbackWithoutGtidTest pins all of it. Its 5 defect tests fail against the previous logic -- verbatim, before the fix: bindMustNotWriteUint64Max:165 the bind wrote the -1 sentinel, stored as UInt64 18446744073709551615 ==> expected: not equal but was: <18446744073709551615> and its 4 regression tests (GTID snowflake, GTID plain, sequence number, LSN) pass both before and after, pinning that nothing else moved. Regression: sink-connector -am, baseline 226 tests / 0 failures / 20 errors, post-change 235 / 0 / 20. The 20 errors are the pre-existing Docker-dependent tests (DBMetadataTest, DbWriterTest, ClickHouseCreateDatabaseTest, DbKafkaOffsetWriterTest) and the sorted error sets are identical.
Added release notes for 2.10.2
Fix datatype integration tests
…ing (#1426) #1401 added a startup check that refuses to replicate a MySQL source holding a table with no PRIMARY KEY and no non-null UNIQUE key. The diagnosis behind it stands -- such a table has no row identity in the binlog, so its ClickHouse copy silently diverges -- but refusing is the wrong remedy, for two reasons. It is not the connector's decision. A keyless table is often migration bookkeeping nobody reads, or one whose key is already scheduled for a maintenance window. Whether an imperfect copy of it is acceptable in the meantime is a judgement about the data, which the operator has and the connector does not. And the blast radius is wrong. The refusal is per SOURCE, not per table: one keyless table stops the entire pipeline, including every correctly-keyed table on the same server. On a source with dozens of replicated schemas, a single alembic_version row -- three columns, written once per migration -- takes everything else down with it. The cost of refusing lands almost entirely on data that was replicating perfectly well. So the check now reports instead of blocking. Everything that made the refusal actionable is kept: the banner still names every offending table and carries the exact ALTER TABLE that fixes it, still says NOT retroactive so nobody expects sql_generate_invisible_primary_key to repair existing tables, and is still emitted at ERROR at startup, at table creation, and periodically during replication. Only the throw is gone, and the wording now states plainly that replication is continuing -- "REPLICATING ANYWAY -- THESE TABLES WILL DIVERGE" -- so nobody hunts for a startup failure that never happens. Both escape hatches keep working and now mean what their names say: table.exclude.list leaves a table out of replication entirely, and keyless.table.check.skip=true silences the reporting. Changes ------- * UnsupportedSourceException deleted; both throw sites log.error and return. * refusalPreExisting / refusalTooOld renamed to warningPreExisting / warningTooOld, text rewritten from "REFUSING TO REPLICATE" to "REPLICATING ANYWAY -- THESE TABLES WILL DIVERGE". * The rethrow in the catch chain removed, so no path escapes check(). * Class javadoc, the DebeziumChangeEventCapture call-site comment, the MySqlDDLParserListenerImpl note and two parser-test comments corrected: they described a refusal that no longer exists. * The IT config keeps keyless.table.check.skip=true, but its comment no longer claims the ITs need it to run -- they do not, now that startup is never blocked. It only suppresses a banner in a fixture that is keyless on purpose. Tests ----- 30 in KeylessTablePreflightTest (was 28), 160 across the affected suites, all green. Two of the new tests are structural rather than behavioural, and deliberately so. The behavioural test drives every entry path a unit test can reach -- the skip override, a non-MySQL source, an unreachable host, an incomplete config -- and asserts none throws. But the two sites that actually decided to refuse sit AFTER a successful connection and a completed table scan, which no unit test reaches without a live MySQL. Verified by measurement, not assumption: with the refusal reinstated, the behavioural test still PASSED and only the structural check failed. So check() is additionally scanned in source for any throw statement, and the class is reflected over to prove no refusal-era API survives. Failing-first: with the throws restored, testCheckContainsNoThrowStatement fails naming the exact reinstated line, and testNoRefusalApiRemains fails naming the exception type. With the fix in place, both pass. Co-authored-by: OmniWatcher <svc-dba-github@jumptrading.com> Co-authored-by: Claude <noreply@anthropic.com>
Added release notes for 2.10.3
The un-exclusion rule in get_table_checksum_query() was added so that is_deleted is not dropped when the connector also creates _is_deleted. It matched on the underscore prefix alone, so it fired for ANY excluded column whose _-prefixed name happened to exist on the table. A table with a business column 'version' and the connector's '_version' therefore had 'version' silently re-added to the ClickHouse checksum while mysql_table_checksum.py still honoured the exclusion. The two sides hashed different column sets, so the checksum could never match and no --exclude_columns/ignored_columns value could suppress it. Restrict the rule to columns the connector itself manages. is_deleted remains in the set, so the original is_deleted/_is_deleted behaviour is unchanged; a user-supplied exclusion is now always honoured. Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
Author
Change of direction — please do not review the current diffA senior DBA on our side pointed out that my framing was wrong, and he is right. Summarising here rather than silently force-pushing, so the history stays honest. What I got wrong. I treated the un-exclusion rule as a pure bug and proposed making ClickHouse honour the exclusion. But So the ClickHouse behaviour — keeping the real business column and dropping only the connector's shadow What the actual defect is. Not the rule itself, but that only one side applies it:
The two sides therefore hash different column sets whenever a table has both Note the two sides are also fed different exclude lists by One edge case worth flagging before anyone writes it: a naive symmetric rule keyed on both Our immediate situation is resolved without any tool change. The column was only in our ignore list because it was added while iterating to get a job green; the data was never divergent. Removing that one ignore line makes both sides hash How would you like to proceed? Happy to either (a) repoint this PR at the MySQL-side symmetric fix, or (b) close it and leave the report as an issue if you would rather own the fix shape yourselves. The asymmetry is real either way and silently produces an unfixable checksum mismatch, so it seemed worth reporting even though we no longer need it patched. Apologies for the churn. |
minguyen9988
deleted the
omniwatcher/fix-metadata-column-un-exclusion
branch
September 2, 2026 10:09
minguyen9988
pushed a commit
that referenced
this pull request
Sep 3, 2026
Base moved on two commits (#1432 DDL/DML race, #1429 db_compare). One file conflicted -- DebeziumChangeEventCapture -- in two hunks, and in both the correct resolution is neither side verbatim. 1. DDL cache invalidation ------------------------- Both sides fixed something real: base (#1432) an unresolvable DDL invalidated NOTHING while having changed the schema, so writers kept binding against a shape that no longer existed. Falls back to invalidateAll(). branch a PostgreSQL schema-change event may name no table in its tableChanges array, but the topic still identifies it; also invalidates the schema-drift detector cache. Taking base loses Postgres DDL invalidation entirely (Postgres events routinely resolve to an empty table list, so every Postgres DDL would fall through to a fleet-wide sweep, and the drift detector would keep serving a stale schema until its TTL). Taking ours loses the safety net for genuinely unresolvable DDL. Merged as a ladder: resolve from the event, then from the topic, and only if BOTH fail invalidate everything. The Postgres path keeps its targeted invalidation including the drift-detector cache; the base's fail-safe still catches what neither can name. 2. The null-record early return -- load-bearing for #1379 ---------------------------------------------------------- The branch had: chStruct = ...parse(...); if (chStruct == null) { log.debug("Skipping record with null ClickHouseStruct ..."); return null; } chStruct.setSequenceNumber(sequenceNumber); This predates both #1416 and #1428 and is INCOMPATIBLE with the fix the base now carries. Dropped in favour of the base deliberately, not as a conflict-resolution convenience: - a heartbeat parses to null by contract. Under #1428 the caller (handleChangeEventBatch) must still SEE that record so commitControlRecordOffset can acknowledge its offset -- on an idle source the post-snapshot state rides exclusively on heartbeats; - returning early means the record never reaches the control-record path, replica_source_info stays pinned at snapshot=INITIAL/snapshot_completed=false forever, and every restart re-runs the whole snapshot. That is precisely the reported bug. A comment now marks the spot so the early return is not reintroduced. Both #1416 and #1428 reach this branch through this merge and not before it: they are on develop, and 2.11.0 carries them as cherry-picks rather than as ancestors, so neither was an ancestor of the PR head. Before this merge the branch still had the pre-#1416 code at the parse site and none of the #1428 machinery (handleChangeEventBatch / commitControlRecordOffset / isPipelineQuiescent). Everything else auto-merged (55 files). Verified rather than assumed: - no conflict markers anywhere in the tree - the offset-table DDL is ORDER BY offset_key in all 27 shipped configs including the runtime default and all three Postgres ones (the other half of #1428; the single remaining "ORDER BY id" is an unrelated test fixture table in sql.py, not the offset store) - handleChangeEventBatch is wired into the engine's only ChangeConsumer, so the Postgres path reaches it identically to MySQL Build: both modules, mvn clean install -DskipTests, green under JDK 17.0.2 / Maven 3.9.11. Both destructive sites are pre-existing upstream content carried in by the merge, not introduced here: a documentation string describing the disable.drop.truncate option in the k8s configmap, and an already- annotated DROP COLUMN in StaleSchemaCacheIT scoped to a throwaway testcontainer table. Destructive-Op-Check: sites=2; result=pass
minguyen9988
added a commit
that referenced
this pull request
Sep 8, 2026
… DDL/DML race fix) (#1435) * add postgres * checksum for postgres * remove * clean up * clean up * update the dump script * build the python into a package * reinstate the lic * change back snapshot mode to initial * remove the unecessary files * sanitize the repo * Fix comments * fix the comments * centralize mapping * db name quoting issue * update the view to be compatible with postgres * fix the CI * fix the CI * fix kafka test * fix test * fix git work * address PR review comments from Kanthi (round 2) - Consolidate verbose 8 per-type 10BC8 test methods into parameterized tests using @MethodSource in ClickHouseDataTypeMapperDDLTest and PostgresSchemaReconcilerTest (review comments on test verbosity) - Remove MariaDB test constant and MariaDB-specific tests from DDLParserFactoryTest; remove duplicate MySQL test - Remove plans/postgres/ directory (internal development docs not suitable for the repository) - Add junit-jupiter-params dependency to sink-connector-lightweight * use ClickHouseDbConstants in PostgresSchemaReconciler (PR review round 2) * fix CI test failures * fix the loader * change the dump * fix the nullable type ingestion * upgrade the version to match both side * fix the failing test * totel fix to make it work * auto create the heartbeat table option in dumper to prepare for the sink-connector * dumper check the user condition * ignore the internal directory * ignore the internal directory * fix the tests * new naming stuff * implement column type override * fix the test * update the feature * fix(test): PostgresSchemaDriftIT — isolate tables and replication slots per test Root cause: both test methods shared a single table (schema_drift_test) and the default replication slot name. When the first test finished and the second started, the leftover slot caused 'Failed to start replication stream' after 6 retries, which meant no CDC events flowed and the drift detector never ran. Changes: - Split into schema_drift_single / schema_drift_multi tables so ALTER TABLE in one test does not pollute the other. - Assign unique slot.name per test (drift_slot_single / drift_slot_multi). - Increase slot.max.retries from 6→12 and slot.retry.delay.ms from 5000→10000 for slow CI runners. - Drop replication slot before and after each test to prevent leftovers. - Add null-guard in PostgresSchemaChangeDetector.fetchClickHouseSchema() so a null writer logs a warning instead of throwing NPE. * fix(test): INSERT before waiting for drift column — drift detection is event-driven The schema drift detector (checkAndReconcile) only fires when a DML SourceRecord arrives in processEveryChangeRecord(). The previous test waited for the column to appear in ClickHouse BEFORE inserting a row, creating a deadlock: no DML event → no drift detection → column never appears → test times out. Fix: INSERT immediately after ALTER TABLE so the CDC event triggers drift detection, then poll for both the column and the row data. * fix(test): replace fixed Thread.sleep with polling-based snapshot wait Test 1 (testColumnAddedToPostgresIsPropagatedToClickHouse) failed in CI at the initial snapshot verification step because the Debezium engine encountered a Producer failure (PSQLException: I/O error) and could not complete the snapshot within the fixed 60s Thread.sleep window. Root cause: In CI, Test 2 runs first and passes. By the time Test 1 starts, the shared PostgreSQL container may have stale connections or resource pressure, causing the snapshot to take longer than 60s. Fix: Replace the rigid Thread.sleep(60_000) with a polling loop that checks every 5 seconds for up to 120 seconds whether the initial row has appeared in ClickHouse. This is: - More robust: adapts to varying snapshot completion times - Faster on success: returns immediately when the row appears - Better diagnostics: logs each poll attempt for debugging - Consistent: applied to both Test 1 and Test 2 Test 2 (multi-column) already passed in CI run 22839813547, confirming the drift detection INSERT-before-wait fix works correctly. * fix(test): use unique offset storage tables per test to prevent cross-contamination Root cause: Both test methods in PostgresSchemaDriftIT share the same ClickHouse offset storage table (altinity_sink_connector.replica_source_info) and schema history table. When Test 2 runs first and completes, its committed offsets (snapshot=FALSE, LSN=0/1A25218) remain in the shared table. When Test 1 starts, the Debezium engine reads Test 2's offsets, sees snapshot=FALSE, and skips the initial snapshot entirely — going straight to streaming from an LSN that is invalid for Test 1's replication slot. This causes: 1. 'Last recorded offset is no longer available on the server' 2. PostgreSQL postmaster crash: 'FATAL: terminating connection due to unexpected postmaster exit' 3. No snapshot = no initial row replicated = assertion failure Fix: Give each test its own unique offset storage table name and schema history table name, keyed by the slot name (which is already unique per test): - Test 1: replica_source_info_drift_slot_single - Test 2: replica_source_info_drift_slot_multi This ensures each test starts with a clean offset state and always performs its own initial snapshot. * revert the images * fix(test): remove obsolete ClickHouse 22.3 tests incompatible with Testcontainers 1.21.4 ReplicatedRMTClickHouse22TIT and ReplicatedRMTDDLClickHouse22TIT both fail with IllegalStateException because clickhouse-server:22.3 is no longer recognized as a compatible substitute by the Testcontainers ClickHouse module (v1.21.4). ClickHouse 22.3 reached end-of-life. The same ReplicatedReplacingMergeTree auto-creation functionality is already covered by: - ReplicatedRMTIT (CH 24.8) - passes - ReplicatedRMTDDLIT (CH 23.8) - passes Removing these redundant tests eliminates the last 2 errors in the CI pipeline (Tests run: 288, Failures: 0, Errors: 0 expected after this). * feat: add clickhouse.table.schema.prefix config for schema-prefixed table naming Add config option clickhouse.table.schema.prefix (boolean, default false) that prefixes ClickHouse table names with __schema__ derived from the Debezium topic. When enabled, a topic like 'prefix.public.MyTable' creates table '__public__MyTable'. Modified files: - ClickHouseSinkConnectorConfigVariables: new enum constant - ClickHouseSinkConnectorConfig: config registration (Type.BOOLEAN) - Utils: overloaded getTableNameFromTopic(String, boolean) - DebeziumChangeEventCapture: schemaPrefixEnabled field, getTableName(), extractTableNameFromTopic(), DDL cache, schema drift detection - ClickHouseBatchRunnable: getTableFromTopic() reads config Validated on fpif-metrics-dev-ch01: 19 tables with __public__ prefix, 272,379 rows checksummed against PostgreSQL source - 100% match. * feat: add template-based schema naming for database suffix and table prefix Add two new config options for flexible schema-aware naming: - clickhouse.database.schema.suffix (boolean, default false): when enabled, appends the resolved common schema template to the ClickHouse database name - clickhouse.common.schema.template (string, default empty): shared template with {{ schema }} placeholder used by both table prefix and database suffix Example: with template '__{{ schema }}__' and PG schema 'public': - Database suffix enabled: 'app' -> 'app__public__' - Table prefix enabled: 'orders' -> '__public__orders' The common template is shared between clickhouse.table.schema.prefix (boolean) and clickhouse.database.schema.suffix (boolean), providing a single template definition for consistent naming across both features. New utility methods in Utils: - extractSchemaFromTopic(): extracts schema from Debezium topic - resolveSchemaTemplate(): replaces {{ schema }} placeholder - applyDatabaseSchemaSuffix(): applies suffix to database name - getTableNameFromTopic(String, boolean, String): 3-arg template-aware version * delete plan * move plan out * feat: add clickhouse.common.database.prefix config for multi-connector isolation Add clickhouse.common.database.prefix (string, default empty) that prepends a static prefix to the ClickHouse database name, enabling multiple sink-connectors to write to the same ClickHouse instance without conflicts when source databases share the same name. Only alphanumeric characters and underscores allowed ([a-zA-Z0-9_]*), validated at startup with IllegalArgumentException on failure. Application order: prefix + database + suffix Example: prefix='litellm_dev_' + db='app' + suffix='__public__' Result: 'litellm_dev_app__public__' New utility methods: - Utils.isValidDatabasePrefix(): validates allowed characters - Utils.applyDatabasePrefix(): prepends prefix to database name - Utils.applyDatabaseNaming(): combined prefix + suffix application * fix: apply database prefix/suffix in DML batch write path + schema-aware naming tests Production fix: - ClickHouseBatchRunnable.processRecordsByTopic(): Apply database prefix (clickhouse.common.database.prefix) and database schema suffix (clickhouse.database.schema.suffix + clickhouse.common.schema.template) to the raw database name from ClickHouseStruct.getDatabase(). Previously only the DDL/schema-drift path in DebeziumChangeEventCapture applied these transformations; the DML write path used the raw Debezium source.db value, causing data to be written to the wrong database. Unit tests (37 tests, all passing): - UtilsSchemaAwareTest.java: Tests extractSchemaFromTopic(), resolveSchemaTemplate(), getTableNameFromTopic(), isValidDatabasePrefix(), applyDatabasePrefix(), applyDatabaseSchemaSuffix(), applyDatabaseNaming() with various inputs including null/empty edge cases. E2E integration test (1 test, passing): - PostgresSchemaAwareNamingIT.java: Enables all 4 naming configs (table schema prefix, common schema template, database schema suffix, database prefix) and verifies PostgreSQL public.tm replicates to ClickHouse dev_public__public__.__public__tm with correct row count. - init_clickhouse_schema_naming.sql: Pre-creates the expected database. * improve postgres dumper * pausing logic on replica * fix vairous bugs * fix the view * Fix the bugs * fix the duplicate entry * Fix the failing tests * Fix testSourceDirectory and update DateTime64 test expectations - Fix testSourceDirectory in sink-connector/pom.xml: changed from src/test to src/test/java to prevent java.lang.SecurityException (prohibited package name java.*) - Update ClickHouseDataTypeMapperDDLTest: DateTime64 expectations now include UTC timezone - Update ClickHouseTableOperationsBaseTest: DateTime64 expectations now include UTC timezone * fix bugs * Refactor UtilsSchemaAwareTest to parameterized tests; add null guard for heartbeat events - Consolidate 30 individual @Test methods into 7 @ParameterizedTest methods with @MethodSource in UtilsSchemaAwareTest.java (addresses review comment #22) - Add null check for chStruct in DebeziumChangeEventCapture.java:1184 to skip Debezium internal heartbeat events that produce null ClickHouseStruct * refactor: address review feedback — extract utilities, use constants, delegate to DBMetadata * refactor: address review feedback round 2 — restore tests, extract PostgresConnectorConfig, move extractDebeziumSchema to ClickHouseConverter, reduce test timeouts - Restore ReplicatedRMTClickHouse22TIT and ReplicatedRMTDDLClickHouse22TIT tests from origin/2.9.1, upgrading ClickHouse Docker image from 22.3 to 24.8 - Move extractDebeziumSchema() and safeGet() from PostgresSchemaChangeDetector to ClickHouseConverter as public static methods; PostgresSchemaChangeDetector now delegates to ClickHouseConverter.extractDebeziumSchema() - Extract PostgresConnectorConfig class from DebeziumChangeEventCapture to encapsulate postgres-specific fields (schemaPrefixEnabled, databaseSchemaSuffix, commonSchemaTemplate, commonDatabasePrefix, postgresSchemaChangeDetector) and their initialization logic; removes TODO comment - Reduce AlterTableModifyColumnIT retry count from 40 to 10 (max wait from 200s to 50s) * Move sink-connector/python to separate branch (feature/ch-sink-tools-python) Restore sink-connector/python to match 2.9.1 so PR #1245 focuses on Postgres/Java connector work. Python toolkit changes live on feature/ch-sink-tools-python for a dedicated PR. Made-with: Cursor * fix: serialize PG text[] arrays as JSON strings when CH column is String type When PostgreSQL text[] columns are auto-created as Nullable(String) in ClickHouse, the ARRAY branch in PreparedStatementFieldMapper calls setArray() which triggers ClassCastException. Now checks the CH column type first — if it is not Array(), serializes the array value as a JSON string using setString(). Fixes ClassCastException on LiteLLM_VerificationToken table for columns: models, allowed_cache_controls, allowed_routes, policies, access_group_ids * fix: handle PostgreSQL db field name in getDatabaseName() to avoid DataException PostgreSQL connectors use "db" in the source struct, not "databaseName" (MySQL). Check schema.field() existence before calling Struct.get() to avoid DataException on every record during schema drift detection. * fix: repair two rebase merge errors that broke the build The rebase onto 2.10.0 merged the DDL cache-invalidation block in DebeziumChangeEventCapture twice. The final conflict resolution replaced the leading database-resolution statements but left the earlier merged loop behind, producing four compile errors: schemaPrefixEnabled -> moved to pgConfig.isSchemaPrefixEnabled() commonSchemaTemplate -> moved to pgConfig.getCommonSchemaTemplate() tableName -> redeclared in the same method invalidationDatabaseName -> declaration had been dropped Rewrite the block to the intended merge: the release branch's override-map-aware database resolution and multi-table invalidation, plus this branch's topic-derived fallback for schema-change events that name no table and its schema-drift detector invalidation, both reached through pgConfig. Also align the BinLogHistoryTest expectations with the producer. An earlier commit on this branch changes the _time column from DateTime64(0) to DateTime64(9); the rebase kept the release branch's DateTime64(0) expectation against it, so both assertions failed. The backticked database name and the db_time column in those expectations are correct and are left as they are. Verified with a real JDK 17 build: mvn -pl sink-connector-lightweight -am compile now reports BUILD SUCCESS, and BinLogHistoryTest passes 2/2. * test: the Postgres ITs assert a timestamp type the connector never emits Standing up the real stack for this branch -- debezium/postgres:15-alpine -> sink connector -> clickhouse-server:24.8 -- PostgresInitialDockerIT fails at the `created` column assertion. The connector emits: CREATE TABLE `public`.`tm`(... `set_date` Nullable(DateTime64(6, 'UTC')), `created` Nullable(DateTime64(6, 'UTC')), `updated` Nullable(DateTime64(6, 'UTC')), ...) while the test asserts Nullable(DateTime64(6)). The test has never matched this branch. The production code is right and is not touched here. Debezium timestamps are UTC by definition, and commit 78f2618d tagged the zone in ClickHouseDataTypeMapper.mapDebeziumSchemaToDDL -- the record-schema auto-create path, which is the path PostgreSQL uses. Dropping the zone would make an instant look like a local wall-clock time to every reader. Only the expectations were left behind. Scope is deliberately the record-schema path only. The MySQL DDL path resolves its type through DataTypeConverter, not this mapper, so CreateTableDataTypesIT and friends still legitimately expect a bare DateTime64 and are untouched. Five assertions, one per Postgres IT, all on the same `created` column: ClickHouseDebeziumEmbeddedPostgresDecoderBufsDockerIT, ClickHouseDebeziumEmbeddedPostgresPgoutputDockerIT, PostgresInitialDockerIT, PostgresInitialDockerWKeeperMapStorageIT, PostgresPgoutputMultipleSchemaIT. No production file changes. Verification: - PostgresInitialDockerIT on real containers via a rootless podman socket: 0/1 -> 1/1. The run reaches Snapshot completed and auto-creates the tables, so the assertion is measured against DDL the connector actually produced, not against a fixture. - Not merge fallout, proven by A/B: the same IT fails identically at the same line on the untouched pre-merge head 4fb183f5, in a separate worktree against the same podman socket. Commit 78f2618d is on this branch and in neither base. The unit-suite counterpart of this same defect -- three PostgresSchemaReconcilerTest timestamp cases expecting the bare form -- is left for the branch owner, as noted on the PR: it is the same question of which side is authoritative, and the answer there may reasonably be to align the reconciler instead. * db_compare: MySQL/ClickHouse checksum column-set asymmetry (approach under revision) (#1429) * Added release notes for 2.10.1 release * fix: /metrics served with no Content-Type, breaking Prometheus 3.x (#1096) (#1413) The /metrics handler wrote its body without ever setting a Content-Type header. Prometheus 3.x treats that as a non-compliant target and fails the scrape outright: non-compliant scrape target sending blank Content-Type and no fallback_scrape_protocol specified for target so the connector silently stopped being monitored after a Prometheus upgrade. Set the Prometheus text exposition content type, "text/plain; version=0.0.4; charset=utf-8", before sendResponseHeaders, which is the call that commits the headers. Also encode the body once instead of calling response.getBytes() twice. That is a real correctness bug, not tidying: the two calls each used the platform default charset, so the length declared in the headers and the bytes actually written could differ for non-ASCII metric labels under a non-UTF-8 default charset, leaving a scraper short of the promised Content-Length. Encoding once to UTF-8 and using that array length makes the declared length true and matches the charset now advertised. Regression test MetricsContentTypeTest starts the real metrics server on a free port and scrapes it over HTTP, since the header is only observable on the wire. Verified failing first: Content-Type was null ("expected: not <null>"); it is the expected value after the fix. The Content-Length assertion passed before the fix too -- the current scrape output is ASCII -- and is there to pin the invariant, not to reproduce a live failure. * fix: INT8 (Byte) hard-cast to Integer fails every TINYINT batch (#385) (#1414) convert() groups INT8 and INT32 into a single isFieldTypeInt branch whose terminal case was: ps.setInt(index, (Integer) value); An INT8 schema delivers a java.lang.Byte, never an Integer, so this threw ClassCastException: class java.lang.Byte cannot be cast to class java.lang.Integer as reported -- and because it aborts the batch, the connector could not replicate any table holding a MySQL TINYINT. The isWiderIntegerTarget hatch immediately above does not cover it: it only fires for Int32/UInt32/Int64/UInt64 ClickHouse targets, and an INT8 source column maps to ClickHouse Int8 (Int32_SCHEMA -> Int8 in dataTypesMap), or UInt8 when the column is unsigned. Read the value as a Number and bind intValue(), so the boxed type no longer has to be guessed. Byte, Short and Integer all widen correctly and keep their sign. For the INT32 case that works today this is a no-op: Integer.intValue() returns the same int the cast produced, so the same setInt call with the same value is issued. Regression test ClickHouseDataTypeMapperInt8Test pins TINYINT to Int8 and UInt8 targets and negative/boundary bytes (-1, MIN_VALUE, MAX_VALUE), with two controls: INT32 still binds identically via setInt, and the isWiderIntegerTarget path still routes to setObject. Verified failing first: the 3 INT8 tests errored with the ClassCastException above while both controls passed; all 5 pass after the fix. * fix: ARRAY binding hard-casts to ArrayList, breaking every other List (#749) (#1415) The ARRAY branch of ClickHouseDataTypeMapper.convert cast the incoming value to java.util.ArrayList: ps.setArray(index, ps.getConnection().createArrayOf( dt.name(), ((ArrayList) value).toArray())); But the Kafka Connect contract for an array field is java.util.List, not ArrayList. Debezium and the Connect converters may hand over any List implementation, and an empty array in particular arrives as Collections.emptyList(). The cast then throws ClassCastException: class java.util.Collections$EmptyList cannot be cast to class java.util.ArrayList which fails the entire batch -- exactly the frame in the reporter stack trace (ClickHouseDataTypeMapper.convert, from PreparedStatementExecutor.insertPreparedStatement). Bind through java.util.Collection instead. Every value that can reach this branch is a Connect array, i.e. a List, so Collection covers all of them; Collection.toArray() returns the same Object[] the ArrayList cast produced, so the case that worked before is unchanged. Regression test ClickHouseDataTypeMapperArrayTest pins the reporter Collections.emptyList() case plus List.of, Arrays.asList and LinkedList, and keeps an ArrayList control asserting identical bound elements. Verified failing first: 4 of 5 errored with the ClassCastException above (the ArrayList control passed); all 5 pass after the fix. * fix: initial snapshot never finishes -- NPE on every unparseable record (#1416) * fix: initial snapshot never finishes -- NPE on every unparseable record (#1379) Root cause ---------- processEveryChangeRecord dereferenced the parser's return value before testing it for null: chStruct = debeziumRecordParserService.parse(record, recordCommitter, lastRecordInBatch); chStruct.setSequenceNumber(sequenceNumber); try { if (chStruct != null) { Returning null is contract, not failure. SourceRecordParserService.parse returns null for a null SourceRecord or null value (line 57), for a value that is not a Struct (line 62), and for a value the converter cannot map (line 85). The `if (chStruct != null)` on the very next line is the method's own admission that null is expected. Every such record therefore raised java.lang.NullPointerException: Cannot invoke "...ClickHouseStruct.setSequenceNumber(long)" because "chStruct" is null which the method's outer `catch (Exception e)` swallowed. The record was dropped silently and the snapshot loop logged the same stack trace once per record -- the two symptoms reported in #1379: an initial snapshot that never completes, and NPE spam. Fix --- Move setSequenceNumber inside the existing null check and make the null case observable with a WARN naming the record, instead of a swallowed NPE. No other change; the surrounding logic is untouched. Proof ----- NullParsedRecordSkipTest drives processEveryChangeRecord with a parser stubbed to return null (the documented path) and captures what the class logs via a log4j2 appender. Before the fix, the test fails and quotes back the reporter's exception verbatim: [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 java.lang.AssertionError: an unconvertible record must not raise a NullPointerException; issue #1379 logged one per record for the whole snapshot, got: java.lang.NullPointerException: Cannot invoke "com.altinity.clickhouse.sink.connector.model.ClickHouseStruct.setSequenceNumber(long)" because "chStruct" is null After the fix: [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 No regression. Module suite (-pl sink-connector-lightweight -am test, -Dmaven.test.failure.ignore=true so the reactor reaches both modules; the 50 errors are *IT tests that need a Docker environment unavailable here, identical in both runs): baseline (stashed): Tests run: 288, Failures: 0, Errors: 50, Skipped: 11 with this change: Tests run: 289, Failures: 0, Errors: 50, Skipped: 11 A per-test-class diff of the two runs shows exactly one line of difference: the added NullParsedRecordSkipTest. Fixes #1379 * test: end-to-end regression coverage for #1379 * fix: MySQL DOUBLE auto-created as Float32, silently truncating precision (#1417) * fix: MySQL DOUBLE auto-created as Float32, silently truncating precision ClickHouseDataTypeMapper.dataTypesMap mapped BOTH FLOAT32 and FLOAT64 to ClickHouseDataType.Float32. That map is what types the columns of auto-created tables, so every replicated MySQL DOUBLE / Kafka Connect FLOAT64 landed in a 32-bit ClickHouse column: ~15 significant decimal digits truncated to ~7. Nothing was logged and row counts still matched, so checksum jobs reported the table clean. The example in the issue, 123456789.123456789, is stored as 123456792. Blast radius of the map, every consumer traced: - ClickHouseTableOperationsBase.getColumnNameToCHDataTypeMapping:127 picks the column type for CREATE TABLE (via ClickHouseAutoCreateTable.createNewTable:68) and for the ADD COLUMN clause of an auto-ALTER (via ClickHouseAlterTable.alterTable:133). This is the defect site, and it only affects tables created from now on -- a wider column for new tables, no rewrite of existing ones. - DataTypeConverter:196 (lightweight DDL path) already returns Float64 for Types.DOUBLE at :189, before this lookup is reached, so that path was already correct and is unchanged. - ClickHouseDataTypeMapper.convert:526 uses it only for the ARRAY element type, where widening a Float64 array element is the same correction. Existing tables are unaffected: the write path does not consult this map. PreparedStatementFieldMapper:274 resolves the target type per column from live table metadata (DBMetadata.getColumnsDataTypesForTable, querying system.columns), so a column already declared Float32 keeps binding exactly as before. The bind path is deliberately left alone. convert():325 narrows with setFloat only for a genuine Float; a Double goes through ClickHouseDoubleValue.of(v).asBigDecimal(), which is lossless -- verified against clickhouse-data 0.9.8: 0.1234567890123456 binds unchanged, where (float) of the same value gives 0.12345679. The narrowing was purely in the declared column type. FLOAT32 stays Float32. Its narrowing already happened inside Debezium, and widening the column would only promise a precision the pipeline cannot deliver -- the same reasoning DataTypeConverter documents for Types.REAL. The defect was invisible to the existing auto-create tests because ClickHouseAutoCreateTableBase.getExpectedColumnToDataTypesMap hand-builds the column map with the CORRECT Float64 entries rather than deriving it from the mapper, so the wrong lookup was never exercised. The new tests drive the real mapper and the real auto-create consumer. Verified failing before the fix: float64MapsToFloat64:57 expected Float64 but was Float32 autoCreatedColumnForDoubleIsFloat64:86 expected Float64 but was Float32 and passing after (4/4). Suite for -pl sink-connector -am, baseline vs post: before Tests run: 226, Failures: 0, Errors: 20 after Tests run: 230, Failures: 0, Errors: 20 The 20 errors are the pre-existing docker-unavailable ones and are an identical set in both runs (diff of the error list is empty); the whole delta is the 4 new tests. Fixes #1262 * test: end-to-end regression coverage for #1262 * fix: support PostgreSQL infinity values in timestamptz (#1418) * fix: support PostgreSQL infinity values in timestamptz (#1231) PostgreSQL timestamptz accepts the special values `infinity` and `-infinity`. Debezium delivers them verbatim as those literal strings (PostgresValueConverter#convertTimestampWithZone returns the constants POSITIVE_INFINITY / NEGATIVE_INFINITY, confirmed in the bytecode of debezium-connector-postgres 3.1.3.Final). ZonedTimestampConverter.convert() initialises `result` to "" and tries each of its 17 patterns in turn. None of them match `infinity`, so the loop falls through to the error log and returns the empty string -- the row is delivered to ClickHouse with an empty value rather than failing, which is exactly the symptom reported in #1231: ERROR - Error parsing zonedtimestamp -infinity ClickHouse DateTime64 cannot represent an actual infinity, so saturate to the bounds of the target type instead. These are the same bounds the method already clamps out-of-range timestamps to a few lines below (BinaryStreamUtils.DATETIME64_MAX / DATETIME64_MIN, i.e. 2299-12-31 23:59:59 and 1900-01-01 00:00:00), so no new sentinel is introduced and the PostgreSQL ordering semantics are preserved: `infinity` sorts after and `-infinity` before every other representable timestamp. The change is deliberately scoped to the two known literals. The wider silent-empty-string behaviour for genuinely unparseable input is left as-is: turning it into a throw would change the failure mode of every existing deployment from degraded to hard-stopped, which does not belong in a targeted bug fix. Test: testZonedTimestampConverterInfinity fails before the change with `expected:<[2299-12-31 23:59:59.000000]> but was:<[]>` and passes after. * test: end-to-end regression coverage for #1231 * fix: batch with a missing ClickHouse table is silently dropped (#1285) (#1419) With auto.create.tables disabled and a table absent from ClickHouse, the single-threaded writer path drops the batch and every topic ordered after the failing one, with nothing but a log line to show for it. processRecordsByTopic returns false once the metadata retries are exhausted for a table it cannot read. persistRecords broke out of the per-topic loop on that and returned NORMALLY, and its outer catch swallowed anything thrown. The caller, DebeziumChangeEventCapture.appendToRecords, has no return value to inspect and no exception to catch, so handleBatch completed as if the batch had been written. The acknowledgement block is skipped, so THIS batch is never committed -- but the engine moves on to the next batch, whose markBatchFinished() commits a HIGHER offset. The unwritten records are then behind the committed offset and are never replayed: permanent loss, while the connector looks healthy. persistRecords now raises BatchPersistenceException instead of returning quietly, so the failure reaches the Debezium engine, which stops and restarts from the last committed offset and replays the records. The outer catch no longer swallows: the only exception it still absorbs is an interrupt, which means the task is shutting down rather than failing to write, and turning that into a persistence failure would make a clean stop trigger a restart loop. Scoped to ClickHouseBatchWriter. ClickHouseBatchRunnable carries the same break but survives it -- it clears currentBatch only when the result is true, so it re-polls and retries the same batch -- and is deliberately left alone. This writer is invoked once per batch and has no retry loop, so failing loudly is the only way for it not to lose data. Test: ClickHouseBatchWriterMissingTableTest drives wasTableMetaDataRetrieved() to false through the same state a missing table produces and asserts that no record is acknowledged AND that the failure reaches the caller. Verified failing first on the unmodified tree: AssertionError: persistRecords returned normally after failing to write the batch: the caller cannot tell the batch was dropped, so the offset advances past records that never reached ClickHouse (issue #1285) Module suite: 226 run / 0 failures / 20 errors before, 228 run / 0 failures / 20 errors after -- the 20 errors are testcontainers tests with no Docker environment on the build host, identical in both runs. * Fix #1213: _version stores UInt64 max when the source has no GTID (#1420) On a MySQL source with GTID disabled, the _version column was filled with 18446744073709551615 and the JDBC driver then refused to read it back ("integer overflow: 18446744073709551615 cannot be presented as long"). Chain: - ClickHouseStruct.version is initialised to UNINITIALIZED_VALUE = -1 (ClickHouseStruct.java:48, :204). - calculateVersion() only assigned a version from gtid, sequenceNumber or lsn (:823-834) and fell through, leaving -1, when all three were absent. setGtid is only reached when source.gtid splits into exactly 2 segments (:477-482), which never happens without GTID; nothing in sink-connector/src/main ever calls setSequenceNumber (only the lightweight DebeziumChangeEventCapture does), so the Kafka Connect path has no sequence number either; and lsn is a Postgres-only field. - handleVersionColumn then executed ps.setLong(index, -1) (PreparedStatementFieldMapper.java:503) into the UInt64 _version column, which stores 18446744073709551615. The unreadable cell was only the visible symptom. UInt64 max is the largest representable version, so under ReplacingMergeTree that row wins every future deduplication permanently and all later updates and deletes for the key are silently discarded on merge -- unrecoverable data loss. Fix, in two parts: 1. calculateVersion() gains a final fallback for the case that previously fell through: SnowFlakeId.generate(ts_ms, kafkaOffset). This reuses the connector's existing, already shipping version formula, substituting the Kafka offset for the absent GTID. Both inputs are populated on exactly the affected path -- ts_ms from source.ts_ms (set in setAdditionalMetaData) and the offset from record.kafkaOffset() -- and both are monotonic per source, so versions order by source commit time first and by offset within the same millisecond, the same ordering the GTID form produces. 2. The two bind sites (handleVersionColumn and insertTombstonePreparedStatement) call rejectUnderivableVersion(), which throws IllegalStateException naming the record and the value that would have been stored. After (1) this is only reachable when the record carries no ordering key AND no source timestamp, i.e. a malformed event. Failing the batch is strictly better than writing a row that can never be superseded. No existing deployment changes: a source supplying a gtid, sequenceNumber or lsn still takes its existing branch untouched, so the 2.8.0-compatible version domain of the lightweight connector and the Postgres LSN domain are unaffected. The new fallback is reachable only from the branch that was previously corrupt, and it is applied regardless of the snowflake.id flag because without a GTID there is no raw transaction id for the non-snowflake branch to use. VersionFallbackWithoutGtidTest pins all of it. Its 5 defect tests fail against the previous logic -- verbatim, before the fix: bindMustNotWriteUint64Max:165 the bind wrote the -1 sentinel, stored as UInt64 18446744073709551615 ==> expected: not equal but was: <18446744073709551615> and its 4 regression tests (GTID snowflake, GTID plain, sequence number, LSN) pass both before and after, pinning that nothing else moved. Regression: sink-connector -am, baseline 226 tests / 0 failures / 20 errors, post-change 235 / 0 / 20. The 20 errors are the pre-existing Docker-dependent tests (DBMetadataTest, DbWriterTest, ClickHouseCreateDatabaseTest, DbKafkaOffsetWriterTest) and the sorted error sets are identical. * Added release notes for 2.10.2 * Fix datatype integration tests * fix: a keyless source table must not prevent the connector from starting (#1426) #1401 added a startup check that refuses to replicate a MySQL source holding a table with no PRIMARY KEY and no non-null UNIQUE key. The diagnosis behind it stands -- such a table has no row identity in the binlog, so its ClickHouse copy silently diverges -- but refusing is the wrong remedy, for two reasons. It is not the connector's decision. A keyless table is often migration bookkeeping nobody reads, or one whose key is already scheduled for a maintenance window. Whether an imperfect copy of it is acceptable in the meantime is a judgement about the data, which the operator has and the connector does not. And the blast radius is wrong. The refusal is per SOURCE, not per table: one keyless table stops the entire pipeline, including every correctly-keyed table on the same server. On a source with dozens of replicated schemas, a single alembic_version row -- three columns, written once per migration -- takes everything else down with it. The cost of refusing lands almost entirely on data that was replicating perfectly well. So the check now reports instead of blocking. Everything that made the refusal actionable is kept: the banner still names every offending table and carries the exact ALTER TABLE that fixes it, still says NOT retroactive so nobody expects sql_generate_invisible_primary_key to repair existing tables, and is still emitted at ERROR at startup, at table creation, and periodically during replication. Only the throw is gone, and the wording now states plainly that replication is continuing -- "REPLICATING ANYWAY -- THESE TABLES WILL DIVERGE" -- so nobody hunts for a startup failure that never happens. Both escape hatches keep working and now mean what their names say: table.exclude.list leaves a table out of replication entirely, and keyless.table.check.skip=true silences the reporting. Changes ------- * UnsupportedSourceException deleted; both throw sites log.error and return. * refusalPreExisting / refusalTooOld renamed to warningPreExisting / warningTooOld, text rewritten from "REFUSING TO REPLICATE" to "REPLICATING ANYWAY -- THESE TABLES WILL DIVERGE". * The rethrow in the catch chain removed, so no path escapes check(). * Class javadoc, the DebeziumChangeEventCapture call-site comment, the MySqlDDLParserListenerImpl note and two parser-test comments corrected: they described a refusal that no longer exists. * The IT config keeps keyless.table.check.skip=true, but its comment no longer claims the ITs need it to run -- they do not, now that startup is never blocked. It only suppresses a banner in a fixture that is keyless on purpose. Tests ----- 30 in KeylessTablePreflightTest (was 28), 160 across the affected suites, all green. Two of the new tests are structural rather than behavioural, and deliberately so. The behavioural test drives every entry path a unit test can reach -- the skip override, a non-MySQL source, an unreachable host, an incomplete config -- and asserts none throws. But the two sites that actually decided to refuse sit AFTER a successful connection and a completed table scan, which no unit test reaches without a live MySQL. Verified by measurement, not assumption: with the refusal reinstated, the behavioural test still PASSED and only the structural check failed. So check() is additionally scanned in source for any throw statement, and the class is reflected over to prove no refusal-era API survives. Failing-first: with the throws restored, testCheckContainsNoThrowStatement fails naming the exact reinstated line, and testNoRefusalApiRemains fails naming the exception type. With the fix in place, both pass. Co-authored-by: OmniWatcher <svc-dba-github@jumptrading.com> Co-authored-by: Claude <noreply@anthropic.com> * Added release notes for 2.10.3 * fix(db_compare): only un-exclude sink-connector metadata columns The un-exclusion rule in get_table_checksum_query() was added so that is_deleted is not dropped when the connector also creates _is_deleted. It matched on the underscore prefix alone, so it fired for ANY excluded column whose _-prefixed name happened to exist on the table. A table with a business column 'version' and the connector's '_version' therefore had 'version' silently re-added to the ClickHouse checksum while mysql_table_checksum.py still honoured the exclusion. The two sides hashed different column sets, so the checksum could never match and no --exclude_columns/ignored_columns value could suppress it. Restrict the rule to columns the connector itself manages. is_deleted remains in the set, so the original is_deleted/_is_deleted behaviour is unchanged; a user-supplied exclusion is now always honoured. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: kanthi subramanian <subkanthi@gmail.com> Co-authored-by: OmniWatcher <svc-dba-github@jumptrading.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: OmniWatcher <dbajobs@jumptrading.com> * fix: eliminate the DDL/DML race that silently drops column values (#1432) * Added release notes for 2.10.1 release * fix: /metrics served with no Content-Type, breaking Prometheus 3.x (#1096) (#1413) The /metrics handler wrote its body without ever setting a Content-Type header. Prometheus 3.x treats that as a non-compliant target and fails the scrape outright: non-compliant scrape target sending blank Content-Type and no fallback_scrape_protocol specified for target so the connector silently stopped being monitored after a Prometheus upgrade. Set the Prometheus text exposition content type, "text/plain; version=0.0.4; charset=utf-8", before sendResponseHeaders, which is the call that commits the headers. Also encode the body once instead of calling response.getBytes() twice. That is a real correctness bug, not tidying: the two calls each used the platform default charset, so the length declared in the headers and the bytes actually written could differ for non-ASCII metric labels under a non-UTF-8 default charset, leaving a scraper short of the promised Content-Length. Encoding once to UTF-8 and using that array length makes the declared length true and matches the charset now advertised. Regression test MetricsContentTypeTest starts the real metrics server on a free port and scrapes it over HTTP, since the header is only observable on the wire. Verified failing first: Content-Type was null ("expected: not <null>"); it is the expected value after the fix. The Content-Length assertion passed before the fix too -- the current scrape output is ASCII -- and is there to pin the invariant, not to reproduce a live failure. * fix: INT8 (Byte) hard-cast to Integer fails every TINYINT batch (#385) (#1414) convert() groups INT8 and INT32 into a single isFieldTypeInt branch whose terminal case was: ps.setInt(index, (Integer) value); An INT8 schema delivers a java.lang.Byte, never an Integer, so this threw ClassCastException: class java.lang.Byte cannot be cast to class java.lang.Integer as reported -- and because it aborts the batch, the connector could not replicate any table holding a MySQL TINYINT. The isWiderIntegerTarget hatch immediately above does not cover it: it only fires for Int32/UInt32/Int64/UInt64 ClickHouse targets, and an INT8 source column maps to ClickHouse Int8 (Int32_SCHEMA -> Int8 in dataTypesMap), or UInt8 when the column is unsigned. Read the value as a Number and bind intValue(), so the boxed type no longer has to be guessed. Byte, Short and Integer all widen correctly and keep their sign. For the INT32 case that works today this is a no-op: Integer.intValue() returns the same int the cast produced, so the same setInt call with the same value is issued. Regression test ClickHouseDataTypeMapperInt8Test pins TINYINT to Int8 and UInt8 targets and negative/boundary bytes (-1, MIN_VALUE, MAX_VALUE), with two controls: INT32 still binds identically via setInt, and the isWiderIntegerTarget path still routes to setObject. Verified failing first: the 3 INT8 tests errored with the ClassCastException above while both controls passed; all 5 pass after the fix. * fix: ARRAY binding hard-casts to ArrayList, breaking every other List (#749) (#1415) The ARRAY branch of ClickHouseDataTypeMapper.convert cast the incoming value to java.util.ArrayList: ps.setArray(index, ps.getConnection().createArrayOf( dt.name(), ((ArrayList) value).toArray())); But the Kafka Connect contract for an array field is java.util.List, not ArrayList. Debezium and the Connect converters may hand over any List implementation, and an empty array in particular arrives as Collections.emptyList(). The cast then throws ClassCastException: class java.util.Collections$EmptyList cannot be cast to class java.util.ArrayList which fails the entire batch -- exactly the frame in the reporter stack trace (ClickHouseDataTypeMapper.convert, from PreparedStatementExecutor.insertPreparedStatement). Bind through java.util.Collection instead. Every value that can reach this branch is a Connect array, i.e. a List, so Collection covers all of them; Collection.toArray() returns the same Object[] the ArrayList cast produced, so the case that worked before is unchanged. Regression test ClickHouseDataTypeMapperArrayTest pins the reporter Collections.emptyList() case plus List.of, Arrays.asList and LinkedList, and keeps an ArrayList control asserting identical bound elements. Verified failing first: 4 of 5 errored with the ClassCastException above (the ArrayList control passed); all 5 pass after the fix. * fix: initial snapshot never finishes -- NPE on every unparseable record (#1416) * fix: initial snapshot never finishes -- NPE on every unparseable record (#1379) Root cause ---------- processEveryChangeRecord dereferenced the parser's return value before testing it for null: chStruct = debeziumRecordParserService.parse(record, recordCommitter, lastRecordInBatch); chStruct.setSequenceNumber(sequenceNumber); try { if (chStruct != null) { Returning null is contract, not failure. SourceRecordParserService.parse returns null for a null SourceRecord or null value (line 57), for a value that is not a Struct (line 62), and for a value the converter cannot map (line 85). The `if (chStruct != null)` on the very next line is the method's own admission that null is expected. Every such record therefore raised java.lang.NullPointerException: Cannot invoke "...ClickHouseStruct.setSequenceNumber(long)" because "chStruct" is null which the method's outer `catch (Exception e)` swallowed. The record was dropped silently and the snapshot loop logged the same stack trace once per record -- the two symptoms reported in #1379: an initial snapshot that never completes, and NPE spam. Fix --- Move setSequenceNumber inside the existing null check and make the null case observable with a WARN naming the record, instead of a swallowed NPE. No other change; the surrounding logic is untouched. Proof ----- NullParsedRecordSkipTest drives processEveryChangeRecord with a parser stubbed to return null (the documented path) and captures what the class logs via a log4j2 appender. Before the fix, the test fails and quotes back the reporter's exception verbatim: [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0 java.lang.AssertionError: an unconvertible record must not raise a NullPointerException; issue #1379 logged one per record for the whole snapshot, got: java.lang.NullPointerException: Cannot invoke "com.altinity.clickhouse.sink.connector.model.ClickHouseStruct.setSequenceNumber(long)" because "chStruct" is null After the fix: [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 No regression. Module suite (-pl sink-connector-lightweight -am test, -Dmaven.test.failure.ignore=true so the reactor reaches both modules; the 50 errors are *IT tests that need a Docker environment unavailable here, identical in both runs): baseline (stashed): Tests run: 288, Failures: 0, Errors: 50, Skipped: 11 with this change: Tests run: 289, Failures: 0, Errors: 50, Skipped: 11 A per-test-class diff of the two runs shows exactly one line of difference: the added NullParsedRecordSkipTest. Fixes #1379 * test: end-to-end regression coverage for #1379 * fix: MySQL DOUBLE auto-created as Float32, silently truncating precision (#1417) * fix: MySQL DOUBLE auto-created as Float32, silently truncating precision ClickHouseDataTypeMapper.dataTypesMap mapped BOTH FLOAT32 and FLOAT64 to ClickHouseDataType.Float32. That map is what types the columns of auto-created tables, so every replicated MySQL DOUBLE / Kafka Connect FLOAT64 landed in a 32-bit ClickHouse column: ~15 significant decimal digits truncated to ~7. Nothing was logged and row counts still matched, so checksum jobs reported the table clean. The example in the issue, 123456789.123456789, is stored as 123456792. Blast radius of the map, every consumer traced: - ClickHouseTableOperationsBase.getColumnNameToCHDataTypeMapping:127 picks the column type for CREATE TABLE (via ClickHouseAutoCreateTable.createNewTable:68) and for the ADD COLUMN clause of an auto-ALTER (via ClickHouseAlterTable.alterTable:133). This is the defect site, and it only affects tables created from now on -- a wider column for new tables, no rewrite of existing ones. - DataTypeConverter:196 (lightweight DDL path) already returns Float64 for Types.DOUBLE at :189, before this lookup is reached, so that path was already correct and is unchanged. - ClickHouseDataTypeMapper.convert:526 uses it only for the ARRAY element type, where widening a Float64 array element is the same correction. Existing tables are unaffected: the write path does not consult this map. PreparedStatementFieldMapper:274 resolves the target type per column from live table metadata (DBMetadata.getColumnsDataTypesForTable, querying system.columns), so a column already declared Float32 keeps binding exactly as before. The bind path is deliberately left alone. convert():325 narrows with setFloat only for a genuine Float; a Double goes through ClickHouseDoubleValue.of(v).asBigDecimal(), which is lossless -- verified against clickhouse-data 0.9.8: 0.1234567890123456 binds unchanged, where (float) of the same value gives 0.12345679. The narrowing was purely in the declared column type. FLOAT32 stays Float32. Its narrowing already happened inside Debezium, and widening the column would only promise a precision the pipeline cannot deliver -- the same reasoning DataTypeConverter documents for Types.REAL. The defect was invisible to the existing auto-create tests because ClickHouseAutoCreateTableBase.getExpectedColumnToDataTypesMap hand-builds the column map with the CORRECT Float64 entries rather than deriving it from the mapper, so the wrong lookup was never exercised. The new tests drive the real mapper and the real auto-create consumer. Verified failing before the fix: float64MapsToFloat64:57 expected Float64 but was Float32 autoCreatedColumnForDoubleIsFloat64:86 expected Float64 but was Float32 and passing after (4/4). Suite for -pl sink-connector -am, baseline vs post: before Tests run: 226, Failures: 0, Errors: 20 after Tests run: 230, Failures: 0, Errors: 20 The 20 errors are the pre-existing docker-unavailable ones and are an identical set in both runs (diff of the error list is empty); the whole delta is the 4 new tests. Fixes #1262 * test: end-to-end regression coverage for #1262 * fix: support PostgreSQL infinity values in timestamptz (#1418) * fix: support PostgreSQL infinity values in timestamptz (#1231) PostgreSQL timestamptz accepts the special values `infinity` and `-infinity`. Debezium delivers them verbatim as those literal strings (PostgresValueConverter#convertTimestampWithZone returns the constants POSITIVE_INFINITY / NEGATIVE_INFINITY, confirmed in the bytecode of debezium-connector-postgres 3.1.3.Final). ZonedTimestampConverter.convert() initialises `result` to "" and tries each of its 17 patterns in turn. None of them match `infinity`, so the loop falls through to the error log and returns the empty string -- the row is delivered to ClickHouse with an empty value rather than failing, which is exactly the symptom reported in #1231: ERROR - Error parsing zonedtimestamp -infinity ClickHouse DateTime64 cannot represent an actual infinity, so saturate to the bounds of the target type instead. These are the same bounds the method already clamps out-of-range timestamps to a few lines below (BinaryStreamUtils.DATETIME64_MAX / DATETIME64_MIN, i.e. 2299-12-31 23:59:59 and 1900-01-01 00:00:00), so no new sentinel is introduced and the PostgreSQL ordering semantics are preserved: `infinity` sorts after and `-infinity` before every other representable timestamp. The change is deliberately scoped to the two known literals. The wider silent-empty-string behaviour for genuinely unparseable input is left as-is: turning it into a throw would change the failure mode of every existing deployment from degraded to hard-stopped, which does not belong in a targeted bug fix. Test: testZonedTimestampConverterInfinity fails before the change with `expected:<[2299-12-31 23:59:59.000000]> but was:<[]>` and passes after. * test: end-to-end regression coverage for #1231 * fix: batch with a missing ClickHouse table is silently dropped (#1285) (#1419) With auto.create.tables disabled and a table absent from ClickHouse, the single-threaded writer path drops the batch and every topic ordered after the failing one, with nothing but a log line to show for it. processRecordsByTopic returns false once the metadata retries are exhausted for a table it cannot read. persistRecords broke out of the per-topic loop on that and returned NORMALLY, and its outer catch swallowed anything thrown. The caller, DebeziumChangeEventCapture.appendToRecords, has no return value to inspect and no exception to catch, so handleBatch completed as if the batch had been written. The acknowledgement block is skipped, so THIS batch is never committed -- but the engine moves on to the next batch, whose markBatchFinished() commits a HIGHER offset. The unwritten records are then behind the committed offset and are never replayed: permanent loss, while the connector looks healthy. persistRecords now raises BatchPersistenceException instead of returning quietly, so the failure reaches the Debezium engine, which stops and restarts from the last committed offset and replays the records. The outer catch no longer swallows: the only exception it still absorbs is an interrupt, which means the task is shutting down rather than failing to write, and turning that into a persistence failure would make a clean stop trigger a restart loop. Scoped to ClickHouseBatchWriter. ClickHouseBatchRunnable carries the same break but survives it -- it clears currentBatch only when the result is true, so it re-polls and retries the same batch -- and is deliberately left alone. This writer is invoked once per batch and has no retry loop, so failing loudly is the only way for it not to lose data. Test: ClickHouseBatchWriterMissingTableTest drives wasTableMetaDataRetrieved() to false through the same state a missing table produces and asserts that no record is acknowledged AND that the failure reaches the caller. Verified failing first on the unmodified tree: AssertionError: persistRecords returned normally after failing to write the batch: the caller cannot tell the batch was dropped, so the offset advances past records that never reached ClickHouse (issue #1285) Module suite: 226 run / 0 failures / 20 errors before, 228 run / 0 failures / 20 errors after -- the 20 errors are testcontainers tests with no Docker environment on the build host, identical in both runs. * Fix #1213: _version stores UInt64 max when the source has no GTID (#1420) On a MySQL source with GTID disabled, the _version column was filled with 18446744073709551615 and the JDBC driver then refused to read it back ("integer overflow: 18446744073709551615 cannot be presented as long"). Chain: - ClickHouseStruct.version is initialised to UNINITIALIZED_VALUE = -1 (ClickHouseStruct.java:48, :204). - calculateVersion() only assigned a version from gtid, sequenceNumber or lsn (:823-834) and fell through, leaving -1, when all three were absent. setGtid is only reached when source.gtid splits into exactly 2 segments (:477-482), which never happens without GTID; nothing in sink-connector/src/main ever calls setSequenceNumber (only the lightweight DebeziumChangeEventCapture does), so the Kafka Connect path has no sequence number either; and lsn is a Postgres-only field. - handleVersionColumn then executed ps.setLong(index, -1) (PreparedStatementFieldMapper.java:503) into the UInt64 _version column, which stores 18446744073709551615. The unreadable cell was only the visible symptom. UInt64 max is the largest representable version, so under ReplacingMergeTree that row wins every future deduplication permanently and all later updates and deletes for the key are silently discarded on merge -- unrecoverable data loss. Fix, in two parts: 1. calculateVersion() gains a final fallback for the case that previously fell through: SnowFlakeId.generate(ts_ms, kafkaOffset). This reuses the connector's existing, already shipping version formula, substituting the Kafka offset for the absent GTID. Both inputs are populated on exactly the affected path -- ts_ms from source.ts_ms (set in setAdditionalMetaData) and the offset from record.kafkaOffset() -- and both are monotonic per source, so versions order by source commit time first and by offset within the same millisecond, the same ordering the GTID form produces. 2. The two bind sites (handleVersionColumn and insertTombstonePreparedStatement) call rejectUnderivableVersion(), which throws IllegalStateException naming the record and the value that would have been stored. After (1) this is only reachable when the record carries no ordering key AND no source timestamp, i.e. a malformed event. Failing the batch is strictly better than writing a row that can never be superseded. No existing deployment changes: a source supplying a gtid, sequenceNumber or lsn still takes its existing branch untouched, so the 2.8.0-compatible version domain of the lightweight connector and the Postgres LSN domain are unaffected. The new fallback is reachable only from the branch that was previously corrupt, and it is applied regardless of the snowflake.id flag because without a GTID there is no raw transaction id for the non-snowflake branch to use. VersionFallbackWithoutGtidTest pins all of it. Its 5 defect tests fail against the previous logic -- verbatim, before the fix: bindMustNotWriteUint64Max:165 the bind wrote the -1 sentinel, stored as UInt64 18446744073709551615 ==> expected: not equal but was: <18446744073709551615> and its 4 regression tests (GTID snowflake, GTID plain, sequence number, LSN) pass both before and after, pinning that nothing else moved. Regression: sink-connector -am, baseline 226 tests / 0 failures / 20 errors, post-change 235 / 0 / 20. The 20 errors are the pre-existing Docker-dependent tests (DBMetadataTest, DbWriterTest, ClickHouseCreateDatabaseTest, DbKafkaOffsetWriterTest) and the sorted error sets are identical. * Added release notes for 2.10.2 * Fix datatype integration tests * fix: a keyless source table must not prevent the connector from starting (#1426) #1401 added a startup check that refuses to replicate a MySQL source holding a table with no PRIMARY KEY and no non-null UNIQUE key. The diagnosis behind it stands -- such a table has no row identity in the binlog, so its ClickHouse copy silently diverges -- but refusing is the wrong remedy, for two reasons. It is not the connector's decision. A keyless table is often migration bookkeeping nobody reads, or one whose key is already scheduled for a maintenance window. Whether an imperfect copy of it is acceptable in the meantime is a judgement about the data, which the operator has and the connector does not. And the blast radius is wrong. The refusal is per SOURCE, not per table: one keyless table stops the entire pipeline, including every correctly-keyed table on the same server. On a source with dozens of replicated schemas, a single alembic_version row -- three columns, written once per migration -- takes everything else down with it. The cost of refusing lands almost entirely on data that was replicating perfectly well. So the check now reports instead of blocking. Everything that made the refusal actionable is kept: the banner still names every offending table and carries the exact ALTER TABLE that fixes it, still says NOT retroactive so nobody expects sql_generate_invisible_primary_key to repair existing tables, and is still emitted at ERROR at startup, at table creation, and periodically during replication. Only the throw is gone, and the wording now states plainly that replication is continuing -- "REPLICATING ANYWAY -- THESE TABLES WILL DIVERGE" -- so nobody hunts for a startup failure that never happens. Both escape hatches keep working and now mean what their names say: table.exclude.list leaves a table out of replication entirely, and keyless.table.check.skip=true silences the reporting. Changes ------- * UnsupportedSourceException deleted; both throw sites log.error and return. * refusalPreExisting / refusalTooOld renamed to warningPreExisting / warningTooOld, text rewritten from "REFUSING TO REPLICATE" to "REPLICATING ANYWAY -- THESE TABLES WILL DIVERGE". * The rethrow in the catch chain removed, so no path escapes check(). * Class javadoc, the DebeziumChangeEventCapture call-site comment, the MySqlDDLParserListenerImpl note and two parser-test comments corrected: they described a refusal that no longer exists. * The IT config keeps keyless.table.check.skip=true, but its comment no longer claims the ITs need it to run -- they do not, now that startup is never blocked. It only suppresses a banner in a fixture that is keyless on purpose. Tests ----- 30 in KeylessTablePreflightTest (was 28), 160 across the affected suites, all green. Two of the new tests are structural rather than behavioural, and deliberately so. The behavioural test drives every entry path a unit test can reach -- the skip override, a non-MySQL source, an unreachable host, an incomplete config -- and asserts none throws. But the two sites that actually decided to refuse sit AFTER a successful connection and a completed table scan, which no unit test reaches without a live MySQL. Verified by measurement, not assumption: with the refusal reinstated, the behavioural test still PASSED and only the structural check failed. So check() is additionally scanned in source for any throw statement, and the class is reflected over to prove no refusal-era API survives. Failing-first: with the throws restored, testCheckContainsNoThrowStatement fails naming the exact reinstated line, and testNoRefusalApiRemains fails naming the exception type. With the fix in place, both pass. Co-authored-by: OmniWatcher <svc-dba-github@jumptrading.com> Co-authored-by: Claude <noreply@anthropic.com> * Added release notes for 2.10.3 * fix: post-snapshot heartbeat offset is never committed, so the snapshot never finishes (#1379) (#1428) * fix: post-snapshot heartbeat offset is never committed, so the snapshot never finishes (#1379) A Debezium batch carries more than row changes. A heartbeat has a Struct value and no `op` field, so SourceRecordParserService.parse returns null for it -- that null is contract, not failure. Such a record was then dropped outright: never markProcessed, never markBatchFinished. Only records that became a row were ever acknowledged. That silently strands the end-of-snapshot state. Debezium marks the snapshot complete AFTER the last snapshot row is emitted (RelationalSnapshotChangeEventSource#createDataEvents calls preSnapshotCompletion/postSnapshotCompletion once the row loop is done), so every snapshot ROW still carries snapshot=INITIAL, snapshot_completed=false. The completed state rides only on records emitted after the snapshot -- and on an idle source those are exclusively heartbeats. Dropping them leaves replica_source_info pinned at snapshot=INITIAL/snapshot_completed=false forever, which is both halves of the report: - the status view never shows the snapshot finishing, even though the log says "Snapshot completed" and the rows are all in ClickHouse; - on restart InitialSnapshotter#shouldSnapshotData reads snapshotInProgress=true and re-runs the entire snapshot. The reporter's own 2.10.2 log shows the mechanism directly: the dropped heartbeat carries sourceOffset={lsn=74130877960, txId=3322, ...} with no snapshot keys at all (PostgresOffsetContext#getOffset omits them once postSnapshotCompletion has cleared the snapshot type), while the stored offset still has snapshot=INITIAL. Committing that heartbeat is exactly what lets a restart skip the snapshot. This is NOT the defect fixed in #1416. That fix stopped a NullPointerException and turned the drop into a clean, logged skip; the record is still dropped and its offset still never committed, which is why the reporter confirmed the symptom persists on 2.10.2. Ordering safety. A heartbeat carries the connector's CURRENT position, at or after every row already read, so acknowledging it while a row is still unwritten would advance the offset past that row and lose it on a crash -- the failure mode of #1285. The offset is therefore committed only when nothing is in flight: no row …
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.
Summary
get_table_checksum_query()inclickhouse_table_checksum.pycan silently re-add a column the user explicitly excluded, making the ClickHouse checksum hash a different column set than the MySQL side. When that happens the checksum for the affected table can never match, and no--exclude_columns/ignored_columnsvalue can suppress it.Root cause
The un-exclusion rule was introduced for the connector's
is_deleted/_is_deletedpair:The condition matches on the underscore prefix alone, so it fires for any excluded column whose
_-prefixed name happens to exist on the table — not just connector metadata.A table with a business column
versionand the connector's_versionhits this:versionis un-excluded on the ClickHouse side, whilemysql_table_checksum.pystill honours the exclusion. The two sides hash different column sets.The asymmetry is invisible in the logs — the ClickHouse side logs
Not excluding column version as _version is also excludedas an INFO line, while the top-level job reports only a checksum mismatch, which reads as a data divergence.Reproduction
A table with both
versionand_version, excludingversion:Observed on a table of 1,407,663 rows where every column value is byte-identical between MySQL and ClickHouse. Re-computing the tool's own hash arithmetic over both sides confirms the data agrees and only the column set differs:
Both sides agree under either consistent column set. The mismatch exists only in the asymmetric set the tool actually builds.
Natural control on the same schema: a sibling table with
_versionbut noversioncolumn passes cleanly.Fix
Restrict the un-exclusion rule to columns the connector itself manages:
is_deletedremains in the set, so the originalis_deleted/_is_deletedbehaviour the rule was written for is unchanged.versionis not connector metadata, so a user-supplied exclusion is now honoured.The set matches the metadata columns the tooling already treats as connector-owned —
clickhouse_table_checksum.pydefaults--exclude_columnsto_sign,_version,is_deleted,_is_deleted, andtop_level_table_checksum.pyalways passes_version,is_deleted,_is_deleted,__is_deleted. A user column such asversionnever enters the excluded list except via explicit user configuration, which is precisely the intent that must not be overridden.Verification
Simulating the filter over the real affected column list (
id, risk, bunit, date, _version, is_deleted, version, clientupdatetime, action, state, user) excluding_version, is_deleted, _is_deleted, __is_deleted, version, clientupdatetime, action, state, user, risk:Regression check on the pair the rule exists for (
id, val, is_deleted, _is_deleted):python -m py_compilepasses on the modified file.Scope
One file, two hunks, no behaviour change for any table without a business column colliding with a connector metadata name. Tables affected by the bug today are only those carrying a column whose
_-prefixed twin also exists.Suggested follow-up
A regression test covering a table with both
versionand_version, assertingversionis absent from the generated select. This is the case that reads as correct under any single-column test, which is why it survived.Filed by OmniWatcher, an AI agent, on behalf of the Jump Trading DBA team, at the request of a senior DBA. Marked WIP / DO NOT MERGE because we do not own this repository — please treat it as a proposal for maintainer review rather than a ready-to-merge change, and tell us if you would prefer a different shape for the fix (for example, deriving the metadata set from existing constants). Happy to add the regression test in this PR if you want it here rather than as a follow-up.
🤖 Generated with Claude Code