fix(lore-0059): rollup timestamp-shadowing fix + full-chain version-propagation test#61
Merged
karczuRF merged 3 commits intoJun 26, 2026
Conversation
…n test The shipped rollups.sql and preroll.sql alias the bucket key `toStartOfInterval(timestamp, …) AS timestamp`, which shadows the source `timestamp` column. The bare `timestamp` inside argMin(open)/argMax(close)/ argMax(close_usd) then resolves to the constant bucket-start, so open/close/ close_usd tie-break to an arbitrary row instead of the true first/last by time (volumes, high, low were unaffected). Qualify the source column via FROM … AS t. Add tests/rollup_chain_it.rs: a full _1m → … → _1M integration test that drives the real refreshable-MV chain end-to-end and proves enrichment re-inserts propagate with no under/double-count and a winning version at every grain, plus a preroll.sql full-range pass. The fix is what makes the OHLC assertions green. current.sql was already correct (FROM … AS c + c.timestamp). Schema-overview §3.2 reference DDL updated to match.
Mark 0059's remaining ACs proven against the real 0051 DDL, document the argMin/argMax timestamp-shadowing finding (Emerged), and spawn 0071 to re-apply the corrected MV/preroll DDL to the live ch-prod-01 cluster (the buggy DDL was applied under 0051).
…nchor The full-chain rollup test ran against docker CH 25.6 while ch-prod-01 runs 26.3.10.60 (25.x -> 26.x gap), so "green locally" did not cover the engine the shipped DDL actually runs on. Pin docker-compose to the exact prod version 26.3.10.60; full prices-clickhouse suite re-verified green. Also make insert_bucket deterministic: it re-evaluated toStartOfInterval(now()) per INSERT, so a wall-clock 15-minute boundary crossing between the un-enriched and enriched batches anchored them to different buckets, breaking the ReplacingMergeTree dedup and the single-bucket assertions. bucket_anchor() now freezes the boundary once as an integer epoch (toUInt64(toUnixTimestamp(...)) -> toDateTime(<n>)), reused across every INSERT and robust across CH versions/timezones.
Collaborator
Author
✅ Full suite verified against the prod ClickHouse versionRe-ran the
Notes:
Caveat: this validates the test + its SQL on the prod engine version, not the shipped rollup DDL applied to the live |
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.
Task
lore-0059 — MV rollup-chain version propagation under enriched
_1mre-inserts. Unblocked once 0051 landed the production MV DDL.What this does
Closes 0059's remaining ACs by verifying the real shipped rollup chain against a live ClickHouse, and fixes a correctness bug the verification surfaced.
New: full-chain integration test
packages/prices-clickhouse/tests/rollup_chain_it.rs(#[ignore], runs against the pinned docker CH):rollups.sqlchain end-to-end_1m → _15m → _1h → _4h → _1d → _1w → _1MviaSYSTEM REFRESH VIEW+ poll.version+1,volume_quote_usdfilled) propagates to every grain with no under-count and no double-count (volume_basestays 30;volume_quote_usd0 → 300; version 1 → 2 wins at every grain).preroll.sqlfull-range pass asserting the same OHLC correctness.Bug fixed:
argMin/argMaxtimestamp shadowingThe as-shipped
rollups.sqlandpreroll.sqlalias the bucket keytoStartOfInterval(timestamp, …) AS timestamp, which shadows the sourcetimestampcolumn. The baretimestampinsideargMin(open)/argMax(close)/argMax(close_usd)then resolved to the constant bucket-start, so open/close/close_usd tie-broke to an arbitrary row instead of the true first/last by time (volumes, high, low were unaffected). Fixed by qualifying the source column viaFROM … AS t+t.timestamp.current.sqlwas already correct (AS c+c.timestamp).Review follow-ups (post-review)
Two issues raised in code review, both addressed in this branch:
clickhouse-server:25.6(25.6.13.41) whilech-prod-01runs26.3.10.60— a 25.x → 26.x major gap, so "green locally" did not cover the engine the shipped DDL actually runs on.docker-compose.ymlis now pinned toclickhouse/clickhouse-server:26.3.10.60; the fullprices-clickhousesuite (rollup_chain, current_mv, views, lib) was re-verified green against it. Policy going forward: local/CI matches the livech-prod-01version — re-checkSELECT version()on prod before bumping the pin. This complements, does not replace the live re-verify owed by 0071.insert_bucketre-evaluatedtoStartOfInterval(now(), …)per INSERT, so a wall-clock 15-minute boundary crossing between the un-enriched and enriched batches would anchor them to different buckets → brokenReplacingMergeTreededup and a flaky single-bucket assertion.bucket_anchor()now freezes the boundary once as an integer epoch (toUInt64(toUnixTimestamp(…))→toDateTime(<n>)), reused across every INSERT and robust across CH versions/timezones (avoidsformatDateTimespecifier portability).The buggy DDL is already applied on
ch-prod-01(under 0051, 2026-06-22). Spawned lore-0071 to re-create the six live MVs from the corrected DDL + recompute mis-rolled buckets. Not done here (prepare-not-deploy).Testing