Cherry-pick upstream #1234: fix add-files partition key lookup for repeated transforms - #26
Draft
EDsCODE wants to merge 3 commits into
Draft
Cherry-pick upstream #1234: fix add-files partition key lookup for repeated transforms#26EDsCODE wants to merge 3 commits into
EDsCODE wants to merge 3 commits into
Conversation
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.
Problem
ducklake_add_data_files()writes everyducklake_file_partition_valuerow at the last transform'spartition_key_indexwhen a partition spec applies multiple transforms to one source column — e.g. the PostHog duckling backfill's events specyear(timestamp), month(timestamp), day(timestamp). The lookup map was keyed on the bare source field index, so the three spec entries collapsed to one.Symptoms downstream:
DuckLakeCompactor: Files have different hive partition path.PostHog worked around this in the Dagster backfill (PostHog/posthog#67168) with a post-register catalog fix-up, but that fix-up needs a direct catalog Postgres connection that CR-based (CNPG-shard) ducklings can't provide from Dagster, so backfills on prod-us/prod-eu currently run with the fix-up disabled (PostHog/charts#13004) and accumulate mis-stamped rows.
Changes
Cherry-picks upstream duckdb/ducklake#1234 (both commits,
-xprovenance preserved; zero conflicts):a5a6be4d— key the partition lookup on (field index, transform) instead of bare field index; regression test withyear/month/dayon one timestamp column inadd_file_partitioned.test.fda045ba— refactor: each parsedHivePartitionrecords itspartition_key_indexat map time and validation compares the full transform (handles repeated bucket transforms with different widths too); newadd_file_repeated_bucket.test.Plus a docs commit recording the cherry-pick in the "PostHog changes" list.
Testing
just build— clean (732/732 targets; local build needscroaringonCMAKE_PREFIX_PATH, e.g.brew install croaring)../build/release/test/unittest "test/sql/add_files/*"— 34/34 test cases, 1134 assertions, including both new regression tests.just test(full suite) — all pass: 404 test cases, 14724 assertions, 18 environment-gated skips (postgres_scanner/spatial/sqlite_scanner extensions and CI-only env vars).Release / follow-up
After merge: tag
v1.0-posthog.5(release binaries publish on tag push), bumpDUCKLAKE_EXTENSION_TAGin duckgresDockerfile+Dockerfile.worker, roll images. Then revert PostHog/charts#13004 and re-run registration on partitions registered while the fix-up was off (the fix-up's DELETE-then-INSERT is idempotent and convergent, so re-registration heals the rot).