Skip to content

fix(arrow): load equality deletes on fields nested in structs - #3453

Draft
JeonDaehong wants to merge 2 commits into
apache:mainfrom
JeonDaehong:fix-nested-eq-delete
Draft

JeonDaehong wants to merge 2 commits into
apache:mainfrom
JeonDaehong:fix-nested-eq-delete

Conversation

@JeonDaehong

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

What changes are included in this PR?

Equality ids may name a field nested in a struct (the spec allows it and EqualityDeleteWriter writes it), but loading such a delete file failed with field not found:

  • Schema evolution of the delete file only maps top-level field ids, so the delete file is now read by the top-level fields that hold the equality ids.
  • The delete predicate used the leaf name (f), which fails to bind or binds to a top-level column with the same name. It now uses the full name (s.f). test_delete_file_loader_parse_equality_deletes expected the leaf name sa for a nested field, so its expected string changes to s.sa.

Applying the delete to data rows also needs predicates on nested leaves in the row filter, which #2906 adds. With #2906 merged locally, an end-to-end scan deletes the expected rows (including rows whose parent struct is null). I'll add that end-to-end test once #2906 lands.

cc @brgr-s, as discussed in #2961.

Are these changes tested?

Yes. test_nested_field_equality_delete_predicate covers loading and the full-name predicate (the table also has a top-level column with the same leaf name). cargo test -p iceberg --lib, clippy and fmt are clean.

AI Disclosure

I used Claude Code to find this, write the change and run the tests. I reviewed it before opening the PR.

Equality ids may name a field nested in a struct, which the spec allows and
EqualityDeleteWriter writes. Loading such a delete file failed because schema
evolution only maps top-level field ids, so the delete file is now read by the
top-level fields that hold the equality ids. The delete predicate also used the
leaf name (`f`), which binds to a top-level column of the same name or fails; it
now uses the full name (`s.f`).
@JeonDaehong

Copy link
Copy Markdown
Contributor Author

@mbutrovich could you take a look when you have time? This fixes loading equality deletes on nested fields (#3450); applying them on scan also needs #2906, which you've been reviewing.

@JeonDaehong

Copy link
Copy Markdown
Contributor Author

Converted to draft to stay within the new limit of 3 open PRs (#3465). The change is still ready for review; I will mark it ready again when one of my other PRs lands.

This branch has not been deployed

No deployments
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.

Scan fails when an equality delete uses a nested field

1 participant