Repository navigation
Project identity partition values into required columns missing from the data file - #4268
Open
slachiewicz wants to merge 2 commits into
Open
slachiewicz wants to merge 2 commits into
slachiewicz wants to merge 2 commits into
Conversation
…the data file Column projection filled a missing identity partition column only when the field was optional, so a required one raised ResolveError even though the manifest carries its value. Closes apache#4267.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Null partition handling remains incorrect for required fields with defaults and for required children masked by null optional parents.
2 open issues
What changed in this PR
This PR addresses #4267 by allowing PyIceberg scans to populate required columns omitted from data files using non-null identity partition values.
Changes:
- Extends Arrow projection eligibility to required fields with non-null partition values.
- Adds regression coverage for projected values, non-nullable output, row filtering, and null rejection without a default.
| File | Description |
|---|---|
| tests/io/test_pyarrow.py | Adds required identity partition column projection tests. |
| pyiceberg/io/pyarrow.py | Allows non-null partition values to populate missing required fields. |
Review effort: Balanced
…s parent is null A null projected value no longer reaches a required field through its initial-default, and it fills a required child of an optional struct when that struct is null in every row.
This branch has not been deployed
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.

Closes #4267
Rationale for this change
The spec's column projection rules fill a field that is missing from a data file from the identity partition value in the manifest.
ArrowProjectionVisitor.structapplied that rule only inside the branch for optional fields and fields with aninitial-default, so a required identity partition column raisedResolveErrorinstead. Tables whose writers keep partition columns out of the data files, such as Hive-style layouts and tables converted from Delta, hit this on every read.A required field now takes the projected value. A null partition value raises
ResolveErrorfor a required field, even when the field has aninitial-default, unless the field's parent struct is null in every row.Are these changes tested?
Yes.
test_identity_transform_column_projection_required_fieldcovers the projected value, the column's non-nullable type and row filters on it, with and without aninitial-default, plus the null partition value.test_identity_transform_column_projection_required_field_under_null_parentcovers a required child of an optional struct. Both fail without the change.Are there any user-facing changes?
Scans of such tables return the partition value instead of raising
ResolveError.