Skip to content

[connectors]: read UC-Uniform/Iceberg column-mapped tables by field-id#6605

Merged
swanandx merged 1 commit into
mainfrom
issue6557
Jul 13, 2026
Merged

[connectors]: read UC-Uniform/Iceberg column-mapped tables by field-id#6605
swanandx merged 1 commit into
mainfrom
issue6557

Conversation

@swanandx

@swanandx swanandx commented Jul 9, 2026

Copy link
Copy Markdown
Member

Bump the delta-rs fork with fix which resolves columnMapping.mode=id columns by Parquet field-id rather than physical name. Without it, reading a UC-Uniform-over-Iceberg table fails with col-<id> is missing from the physical schema. Add a snapshot-read regression test (fixture + e2e).

Fix #6557

Describe Manual Test Plan

created custom runtime, and verified the fix works

Checklist

  • Unit tests added/updated
  • Integration tests added/updated
  • Documentation updated
  • Changelog updated

Breaking Changes?

should not be breaking change

@mythical-fred mythical-fred left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean scope: one-commit fork bump + a real regression test. The fork's v0.32.3-feldera-fix branch is rust-v0.32.3 + the two existing patches + the one new patch 297f2dd1 (comparison is 3-ahead / 0-behind of upstream, 1-ahead / 0-behind of the previously-pinned 5b35c098), so nothing is silently rebased or dropped. The Cargo.toml comment tracks all three patches, points at the new compare URL, and the lockfile update matches.

The regression test earns its keep: after is a six-field nested struct whose children are themselves column-mapped to diverging col-<id> names, so it exercises field-id resolution inside the struct rather than just at the top level — that is the "no field name overlap" struct-cast failure mode from #6557, not only the col-102 is missing case. op VARCHAR NOT NULL also covers the hard-error branch, and the fixture is built by pyarrow (via ensure_delta_spark_fixture), which is the only way to get logical-name Parquet on disk. FIXTURE_VERSION is present for cache invalidation. Good hygiene.

Nits, non-blocking:

  • Consider marking the test with the same @pytest.mark. used by other delta-input suites so it participates in the same selectors — I only see the bare def test_.... If the rest of test_delta_input_*.py similarly has no marker, ignore.
  • Comment nit: # (logical child name, field id) for the six-field after struct. — with AFTER_CHILDREN[0][1] == 104, the top-level ids are id=100/after=101/op=102 and children start at 104 (skipping 103). Not wrong, just worth a one-line note in the fixture header so a future reader doesn't assume 103 is missing by accident.
  • Changelog checkbox is unchecked in the PR body; a one-line "delta connector: read UC-Uniform-over-Iceberg column-mapped tables by Parquet field id (#6557)" under Unreleased would be nice for release notes but isn't strictly needed for a bug fix.

Approve. Solid, minimal, tests the actual failure surface, and the fork diff is auditable.

Comment thread Cargo.toml
# Feldera fork of delta-rs: upstream tag `rust-v0.32.3` + 2 patches, on branch
# `v0.32.3-feldera`. When bumping the pinned revision, preserve these patches:
# Feldera fork of delta-rs: upstream tag `rust-v0.32.3` + 3 patches, on branch
# `v0.32.3-feldera-fix`. When bumping the pinned revision, preserve these patches:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope we have submitted these to upstream and that someone is looking at them.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not yet, but will do when i get time

Comment thread python/tests/platform/fixtures/uniform_iceberg.py Outdated
@swanandx
swanandx force-pushed the issue6557 branch 2 times, most recently from 23acfe9 to 7b7ca2d Compare July 13, 2026 14:42
Bump the delta-rs fork to 9a58fbef, which resolves columnMapping.mode=id
columns by Parquet field-id rather than physical name. Without it, reading a
UC-Uniform-over-Iceberg table fails with `col-<id> is missing from the physical
schema`. Add a snapshot-read regression test (fixture + e2e).

Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
@swanandx

Copy link
Copy Markdown
Member Author

QA passed, merging this

@swanandx
swanandx added this pull request to the merge queue Jul 13, 2026

@mythical-fred mythical-fred left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at tip ee4f35aa.

Additive delta since my last APPROVE (23acfe97):

  1. Fork rev bumped 5b35c098 -> 297f2dd1 on branch v0.32.3-feldera-fix, adding a third patch 9a58fbef "delta-rs: resolve column-mapped Uniform/Iceberg columns by Parquet field-id". Fixes col-<id> is missing from the physical schema when reading UC-Uniform-over-Iceberg tables where columnMapping.mode=id. Cargo.toml header updated to list all three patches with the correct compare URL; matches Gerd's convention for the fork-pinning block.
  2. New pyarrow-driven fixture python/tests/platform/fixtures/uniform_iceberg.py (217 lines) that hand-writes the Parquet + _delta_log in the exact shape a native Iceberg writer + Unity Catalog Uniform produces: Parquet columns named by their logical names carrying PARQUET:field_id, Delta log using columnMapping.mode='id' with divergent col-<N> physical names — at both top level AND inside the after struct's six children. Delta Spark can't produce this shape, so the pyarrow route is correct. store_schema=False is right to keep the file Iceberg-shaped.
  3. New python/tests/platform/test_delta_input_uniform_iceberg.py (139 lines) exercising the customer's cdc_raw failure on two fronts:
    • Non-nullable top-level op (physical col-102) — pre-fix errors with "Non-nullable column 'col-102' is missing from the physical schema".
    • Nested after struct whose six children are each mapped to col-<id> — pre-fix fails the struct cast ("Cannot cast struct with 6 fields to 6 fields because there is no field name overlap"). The test projects a nested child of after, so field-id resolution is exercised at every level, not just the top.
      FIXTURE_VERSION = "v1" bump comment covers the caching gotcha.

QA per swanandx's comment above. Re-approving.

One small ask (non-blocking, per mihaibudiu's L115 comment on my last pass): please do submit the three patches upstream when you get cycles — the fork's diverging longer than we'd like.

Merged via the queue into main with commit 69579e5 Jul 13, 2026
1 check passed
@swanandx
swanandx deleted the issue6557 branch July 13, 2026 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[connectors] Delta snapshot read fails on UC-Uniform-over-Iceberg tables

3 participants