feat(eap-items): add version column to eap_items_1_downsample_64_local - #8426
Open
onewland wants to merge 1 commit into
Open
feat(eap-items): add version column to eap_items_1_downsample_64_local#8426onewland wants to merge 1 commit into
onewland wants to merge 1 commit into
Conversation
|
This PR has a migration; here is the generated SQL for -- start migrations
-- forward migration events_analytics_platform : 0066_add_version_column_downsample_64
Local op: ALTER TABLE eap_items_1_downsample_64_local ON CLUSTER 'cluster_one_sh' ADD COLUMN IF NOT EXISTS version UInt64 DEFAULT 0 CODEC (ZSTD(1));
-- end forward migration events_analytics_platform : 0066_add_version_column_downsample_64
-- backward migration events_analytics_platform : 0066_add_version_column_downsample_64
Local op: ALTER TABLE eap_items_1_downsample_64_local ON CLUSTER 'cluster_one_sh' DROP COLUMN IF EXISTS version;
-- end backward migration events_analytics_platform : 0066_add_version_column_downsample_64 |
onewland
force-pushed
the
feat/eap-items-version-downsample-64
branch
from
September 3, 2026 14:00
3d089ce to
369eaff
Compare
phacops
approved these changes
Sep 3, 2026
Base automatically changed from
feat/eap-items-version-downsample-8
to
master
September 3, 2026 20:03
onewland
added a commit
that referenced
this pull request
Sep 3, 2026
…#8428) ### Stack 1. ~#8424 — `eap_items_1` (merged)~ 2. #8428 — `eap_items_1_downsample_8_local` 3. #8426 — `eap_items_1_downsample_64_local` 4. #8427 — `eap_items_1_downsample_512_local` 5. #8430 — downsample MVs carry `version` Merge in order; each targets the one above it. --- Adds the `version` column to the `downsample_8` tier. ```sql ALTER TABLE eap_items_1_downsample_8_local ADD COLUMN IF NOT EXISTS version UInt64 DEFAULT 0 CODEC (ZSTD(1)); ``` ### Why Part of getting the `eap_items` schemas aligned with the target `ReplacingMergeTree` (keyed on `version`) so that existing parts can be attached onto the new tables. This mirrors the column added to `eap_items_1` in migration 0064. Schema-only — no engine change, so no dedup behaviour changes yet. ### Why `DEFAULT 0` `ADD COLUMN` does not rewrite existing parts, so rows written before this migration have no `version` on disk and the default is evaluated at read/merge time. That makes a non-deterministic default actively dangerous here: under `ReplacingMergeTree(version)` a `now64()` default gives legacy rows a version of "whenever the merge ran", which always beats a genuine write. Verified on CH 25.3 — a real update was silently discarded in favour of the stale row against a `DEFAULT now64()` destination, and won as expected against a `DEFAULT 0` one. `0` is the sentinel for "written before versioning existed" and deterministically loses to every real write, matching the `received_at` precedent in `snuba/manual_jobs/create_eap_received_at_version_test.py`. ### Why local only Unlike 0064, this does not touch `eap_items_1_downsample_8_dist` (or `_dist_ro`): - We never query `version` — it is internal bookkeeping for the cutover and is absent from the storage YAML, so the query layer cannot generate SQL naming it. - The downsample tiers are fed by materialized views that write straight to `*_local` (`swap_downsample_materialized_views` uses `destination_table_name={prefix}_local`, `target=OperationTarget.LOCAL`), so the distributed table is never an insert target. A Distributed table declaring a subset of its local table's columns behaves normally — verified that reads and `SELECT *` through the narrower dist table work, and that an insert routed through it still lets the local default fire. Only an explicit `SELECT version` through the dist table errors, and nothing does that. ### Follow-up The downsample MVs do not yet carry `version` through from the source, so rows they insert will read `0` rather than the source row's version. Fixing that needs an MV rebuild and is deliberately out of scope here. ### Testing - `tests/migrations/test_runner.py::test_no_schema_differences` - `tests/migrations/test_runner.py::test_run_and_reverse_all`
Mirrors the version column added to eap_items_1 in 0064, aligning the downsample_64 tier's schema with its target ReplacingMergeTree ahead of attaching existing parts. Only the local table is touched. We never query `version`, and the downsample tiers are fed by materialized views that write straight to *_local, so the distributed table is neither a read nor an insert path for this column. Defaults to 0, the sentinel for rows written before versioning existed, which deterministically loses to every real write.
onewland
force-pushed
the
feat/eap-items-version-downsample-64
branch
from
September 3, 2026 20:03
369eaff to
6dd9519
Compare
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.
Stack
feat(eap-items): add version column to eap_items_1 #8424 —eap_items_1(merged)eap_items_1_downsample_8_localeap_items_1_downsample_64_localeap_items_1_downsample_512_localversionMerge in order; each targets the one above it.
Adds the
versioncolumn to thedownsample_64tier.Why
Part of getting the
eap_itemsschemas aligned with the targetReplacingMergeTree(keyed onversion) so that existing parts can be attached onto the new tables. This mirrors the column added toeap_items_1in migration 0064.Schema-only — no engine change, so no dedup behaviour changes yet.
Why
DEFAULT 0ADD COLUMNdoes not rewrite existing parts, so rows written before this migration have noversionon disk and the default is evaluated at read/merge time. That makes a non-deterministic default actively dangerous here: underReplacingMergeTree(version)anow64()default gives legacy rows a version of "whenever the merge ran", which always beats a genuine write. Verified on CH 25.3 — a real update was silently discarded in favour of the stale row against aDEFAULT now64()destination, and won as expected against aDEFAULT 0one.0is the sentinel for "written before versioning existed" and deterministically loses to every real write, matching thereceived_atprecedent insnuba/manual_jobs/create_eap_received_at_version_test.py.Why local only
Unlike 0064, this does not touch
eap_items_1_downsample_64_dist(or_dist_ro):version— it is internal bookkeeping for the cutover and is absent from the storage YAML, so the query layer cannot generate SQL naming it.*_local(swap_downsample_materialized_viewsusesdestination_table_name={prefix}_local,target=OperationTarget.LOCAL), so the distributed table is never an insert target.A Distributed table declaring a subset of its local table's columns behaves normally — verified that reads and
SELECT *through the narrower dist table work, and that an insert routed through it still lets the local default fire. Only an explicitSELECT versionthrough the dist table errors, and nothing does that.Follow-up
The downsample MVs do not yet carry
versionthrough from the source, so rows they insert will read0rather than the source row's version. Fixing that needs an MV rebuild and is deliberately out of scope here.Testing
tests/migrations/test_runner.py::test_no_schema_differencestests/migrations/test_runner.py::test_run_and_reverse_all