clickhouse: fix date overflow for initial load#4534
Conversation
| // Timestamps are micros since epoch; dates are days since epoch. Both encoded as Avro varint. | ||
| avroVal := t.UnixMicro() | ||
| if isDate { | ||
| avroVal = t.Unix() / 86400 |
There was a problem hiding this comment.
had to move it here out of process[type] functions because we clamp t value for clickhouse
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
❌ Test FailureAnalysis: A deterministic assertion failure in Test_MySQL_DateTime_ClickHouse_Range (expected nil but got 1900-01-01) reproduces identically across all MySQL/MariaDB matrix variants, indicating the PR's own date-overflow fix is incomplete rather than a flaky failure. |
❌ Test FailureAnalysis: Real bug: the PR's own test Test_MySQL_DateTime_ClickHouse_Range deterministically fails across all 4 CH suites and both matrix jobs with a clear value mismatch (out-of-range nullable DATE replicated as 1900-01-01 instead of NULL), with no timeout/race/network signals. |
parseDateTime64BestEffortOrNull (used by the CDC normalize path) saturates out-of-range values to ClickHouse's [1900, 2299] boundary, clamping the date while preserving the time-of-day, and never nulls. Make the snapshot/Avro path do the exact same thing so snapshot and CDC stay consistent for the same source value, regardless of column nullability. Verified the Go clamp matches parseDateTime64BestEffortOrNull output exactly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
❌ Test FailureAnalysis: The PR's own new test Test_MySQL_DateTime_ClickHouse_Range fails deterministically with a logic assertion ("expected out-of-range nullable DATE to be NULL") across all 4 ClickHouse suite variants and all 3 matrix jobs, indicating the date-overflow fix is genuinely broken rather than flaky. |
❌ Test FailureAnalysis: Real bug: Test_MySQL_DateTime_ClickHouse_Range fails deterministically across every matrix leg with an identical out-of-range datetime overflow (expected 2299-12-31 but got wrapped-around 1715-06-12), indicating a MySQL→ClickHouse datetime range-boundary conversion defect, not a flaky/timeout/race failure. |
…ysql-datetime-date-clickhouse-range
The clickhouse-go driver decodes DateTime64 into a Go time.Time via int64 nanoseconds, which overflows near the 2299 boundary (2299-12-31 wraps to 1715-06-12). The clamped values are stored correctly; compare ClickHouse's own string rendering instead of scanning into time.Time. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
❌ Test FailureAnalysis: Test_MySQL_DateTime_ClickHouse_Range failed deterministically across all 4 ClickHouse suites with an identical assertion (expected "1900-01-01", got ""), a real data-correctness bug in the exact date-overflow feature this PR modifies — not a flaky/timeout/race/network failure. |
❌ Test FailureAnalysis: The PR's own feature test Test_MySQL_DateTime_ClickHouse_Range deterministically fails across all 8 MySQL/MariaDB×ClickHouse variants with expected "1900-01-01" but actual "", indicating a real bug in the date-overflow initial-load fix rather than a flaky failure. |
❌ Test FailureAnalysis: The PR's own new test Test_MySQL_DateTime_ClickHouse_Range fails deterministically across four suites (expected "1900-01-01", got ""), a real bug in the datetime/date ClickHouse-range feature this branch implements; a concurrent BinlogIncident handshake-EOF failure looks flaky but is secondary. |
❌ Test FailureAnalysis: Real bug: Test_MySQL_DateTime_ClickHouse_Range fails deterministically across all four suite variants with a data mismatch (expected "1900-01-01", got empty), directly tied to this PR's date-overflow fix for initial load. |
Aliasing toString(col) back to the column's own name makes older ClickHouse (LTS) resolve the expression against the alias and return empty; the test only reads columns positionally, so the aliases are unnecessary. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
❌ Test FailureAnalysis: Real bug: the PR's own test Test_MySQL_DateTime_ClickHouse_Range fails deterministically across all four ClickHouse+MySQL/MariaDB variants at the same assertion (clickhouse_mysql_test.go:1822) with a data mismatch (expected "1900-01-01"), indicating the date-overflow fix is incomplete rather than a flaky/timeout/race issue. |
🔄 Flaky Test DetectedAnalysis: A single matrix job failed at the "Install ClickHouse" setup step with a transient tarball download/decompress error (curl piped a non-gzip response into tar), which is a network flake unrelated to any test or code change. ✅ Automatically retrying the workflow |
🔄 Flaky Test DetectedAnalysis: The job failed in the "Install ClickHouse" setup step (not in any test) when the ClickHouse binary download returned a non-gzip response causing ✅ Automatically retrying the workflow |
🔄 Flaky Test DetectedAnalysis: The job failed in the "Install ClickHouse" setup step when a curl download of the ClickHouse tarball returned non-gzip content ("gzip: stdin: not in gzip format"), a transient network/CDN flake — no tests actually ran. ✅ Automatically retrying the workflow |
🔄 Flaky Test DetectedAnalysis: The failure is a transient network flake in the "Install ClickHouse" setup step — the ClickHouse release tarball download returned non-gzip content ("gzip: stdin: not in gzip format"), aborting setup before any tests ran, so it's infrastructure flakiness rather than a real code bug. ✅ Automatically retrying the workflow |
🔄 Flaky Test DetectedAnalysis: The job failed while downloading the ClickHouse tarball in the "Install ClickHouse" setup step ("gzip: stdin: not in gzip format" — a transient network/download error), not on any test, so no tests ran and the result is a CI infrastructure flake unrelated to the code change. ✅ Automatically retrying the workflow |
| run: | | ||
| if [ "${{ matrix.db-version.ch }}" = "lts" ]; then | ||
| echo "ch_version=v25.8.11.66-lts" >> $GITHUB_OUTPUT | ||
| echo "ch_version=v25.8.28.1-lts" >> $GITHUB_OUTPUT |
There was a problem hiding this comment.
it was consistently failing on v25.8.11.66-lts: https://github.com/PeerDB-io/peerdb/actions/runs/28783586496/job/85344784176
clickhouse_mysql_test.go:1822:
Error Trace: /home/runner/work/peerdb/peerdb/flow/e2e/clickhouse_mysql_test.go:1822
/home/runner/work/peerdb/peerdb/flow/e2e/clickhouse_mysql_test.go:1828
/opt/hostedtoolcache/go/1.26.4/x64/src/runtime/asm_amd64.s:1771
Error: Not equal:
expected: "1900-01-01"
actual : ""
and what is weird is that clickhouse holds correct value
but clickhouse-go driver seems to interpret it incorrectly...
v25.8.28.1 is the latest patch of the oldest supported release (clickhouse LTS support seems to be 1 year).
so, I think it's ok if we test on the patched version, right?
i can also investigate further why v25.8.11.66 wasn't working properly, but it seems out-of-scope as it works fine on patched version.
clickhouse date types we currently use have min/max constraints (1900-2299)
this PR adds rounding logic for the dates outside this range