Skip to content

connectors: fix delta connector issues with unity catalog in follow mode - #6746

Merged
swanandx merged 8 commits into
mainfrom
uc-token-refresh
Jul 30, 2026
Merged

connectors: fix delta connector issues with unity catalog in follow mode#6746
swanandx merged 8 commits into
mainfrom
uc-token-refresh

Conversation

@swanandx

@swanandx swanandx commented Jul 28, 2026

Copy link
Copy Markdown
Member

Note: CDC-over-uc:// is not covered by this PR. The plain-file CDC read path (cdc_side_dataframe) still uses the URL-addressed ListingTable, which reads empty over a path-less uc:// location, the same way follow did before this fix.

Describe Manual Test Plan

tested out in staging

Checklist

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

Breaking Changes?

no

@swanandx swanandx changed the title connectors: fix delta connector issues with unity catalog connectors: fix delta connector issues with unity catalog in follow mode Jul 28, 2026
Comment thread python/tests/platform/fixtures/column_mapping_nested.py Outdated
Comment thread crates/adapters/src/integrated/delta_table/input.rs Outdated
Comment thread crates/adapters/src/integrated/delta_table/input.rs Outdated
Comment thread crates/adapters/src/integrated/delta_table/input.rs Outdated
Comment thread crates/adapters/src/integrated/delta_table/input.rs Outdated
Comment thread crates/adapters/src/integrated/delta_table/input.rs

@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.

LGTM. Commits are clean and the "why" is well captured; unit tests cover the field-id realign and nested relabel paths, and the Python fixture exercises the follow path against a real mode=name table with a nested struct. Deferring to @mihaibudiu's inline notes.

@feldera feldera deleted a comment from mihaibudiu Jul 29, 2026
@swanandx
swanandx requested a review from mythical-fred July 29, 2026 04:54

@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 of the seven new commits on top of my 2026-07-27 approval. Mihai's inline feedback is addressed cleanly: the doc for field_to_physical is trimmed and no longer describes sibling functions, relabel_array_data's doc no longer over-explains, and the deeper-nested tests he asked for are in — structs in structs, arrays in structs, structs in arrays, structs in arrays of structs, plus a struct-of-struct-of-array-of-struct end-to-end check.

The new work on top is substantive and warranted:

  • uc:// follow now routes through the object-store reader unconditionally (empty DV bitmap when there's no real deletion vector) instead of a ListingTable — a path-less UC URL couldn't resolve root_url() + Add.path, so the previous path read empty. table_root_base guarantees the trailing slash for other schemes so the same join stays well-formed.
  • Field-id-based column resolution (realign_array + project_to_logical in deletion_vector.rs) covers columnMapping.mode=id tables whose files name columns logically but stamp PARQUET:field_id, and where the read schema uses col-<id> names with delta.columnMapping.id. Struct children match by id first, position only as a fallback for unmapped, and missing children null-fill — matching the top-level behavior.
  • Container-type coercion (List vs LargeList) via cast for non-struct types is a real gap the earlier code would have hit as a hard error.

Tests are thorough: field-id resolution, missing-child null-fill, list-container coercion, and the nested-relabel round trip preserving buffers. Commit history is clean — no AI-attribution trailers.

Approving.

@ryzhyk
ryzhyk self-requested a review July 29, 2026 16:03
swanandx added 8 commits July 30, 2026 04:27
Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
…und)

root_url() for a uc:// table has no trailing slash, so joining Add.path
corrupted the object-store authority. Normalize the root before joining.

Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
The follow/CDC read builds its own Parquet ListingTable and renamed
column-mapped fields to their physical (col-<uuid>) names only at the top
level. Nested struct children kept their logical names, so reading a
mode=name table with a nested struct failed the Parquet struct cast
("Cannot cast struct ... no field name overlap") and dropped the rows,
while a snapshot read (which delta-rs resolves) worked.

Recurse the physical read schema into nested struct fields, and restore
their logical names per batch in execute_df_inner (top-level names are
still restored on the DataFrame by project_physical_to_logical). The
per-batch relabel is keyed on a nested-only map, so flat and
mapping-off tables are untouched.

Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
A uc:// location is path-less, so the URL-addressed ListingTable built from
root_url() + Add.path reads empty and silently drops the commit. Route uc://
follow reads (and DV files) through the delta-rs object store directly, the
reader snapshot and CDC-masked reads already use. Other schemes keep the
ListingTable path unchanged.

Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
columnMapping.mode=id tables (UniForm over Iceberg) write data files with
logical column names plus a Parquet field id, while the Delta log assigns
diverging col-<id> physical names. The follow reader matched columns by name,
so it could not find col-<id> in the file, null-filled, and dropped the data
(or failed on a non-nullable column).

Match file columns to the read schema by field id (PARQUET:field_id on the
file, delta.columnMapping.id on the schema), rebuilding nested struct/list/map
children and casting leaves. Falls back to name when a field id is absent, so
mode=name and unmapped tables are unaffected. Mirrors the snapshot path's
field-id resolution.

Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
realign_array recursed into list/largelist/map and downcast to the exact
container the read schema named, so a file whose list kind differed (Delta
List vs a file's LargeList) failed with an internal error. Rebuild only
structs by field id (where column-mapped names must line up) and route
everything else, including container coercion, through cast.

Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
realign_array fell back to the positional child when a target struct
child's field id had no match in the file. That could bind an unrelated
sibling column, or, when the position was out of range, fail with an
internal error. A column-mapped struct that gains a nested child through
schema evolution hits both.

Match nested children by field id only, and null-fill a child the file
lacks, mirroring how project_to_logical handles a missing top-level
column. Positional matching now applies solely to the unmapped case,
where neither side carries a field id.

Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
A failed conversion surfaced Arrow's bare cast error, which names neither
the column nor the file: "Casting from Utf8 to FixedSizeBinary(16) not
supported". Under column mapping the column is named col-<uuid> on disk,
so the user had no way to tell which column of which table failed.

Thread the file and a dotted column path through realign_array, so a
nested leaf reports 'after.col-2' rather than the top-level column, and
report the file's type alongside the type the Delta table declares.

Signed-off-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
@swanandx
swanandx enabled auto-merge July 29, 2026 22:58
@swanandx
swanandx added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit 9ba9b28 Jul 30, 2026
1 check passed
@swanandx
swanandx deleted the uc-token-refresh branch July 30, 2026 01:21
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.

4 participants