fix(arrow): error on required field absent from data file with no default#2797
Open
mbutrovich wants to merge 2 commits into
Open
fix(arrow): error on required field absent from data file with no default#2797mbutrovich wants to merge 2 commits into
mbutrovich wants to merge 2 commits into
Conversation
…ault RecordBatchTransformer resolved a projected field missing from a data file by always producing a null column when it had no initial-default. That is correct only for optional fields. Per the spec's Default values section, null is a valid default only for optional fields, so a required field that is absent with no initial-default has no valid value and must error rather than producing a null column that violates its non-nullable constraint. Mirror Iceberg-Java's Parquet readers (BaseParquetReaders / SparkParquetReaders): resolve to the file value, then initial-default, then null if optional, otherwise raise "Missing required field: <name>".
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
1 participant
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.
Which issue does this PR close?
What changes are included in this PR?
When a projected field is not present in a data file,
RecordBatchTransformerresolves it using the spec's Column Projection fallback rules. For a field with noinitial-defaultit produced a null column (rule #4) unconditionally. That is correct only for optional fields.Per the spec's Default values section, null is a valid default only for an optional field:
So a required field that is absent from a data file with no
initial-defaulthas no valid value, and producing a null column for it violates the field's required (non-nullable) constraint. Previously this surfaced later as a confusing Arrow error (Column '<name>' is declared as non-nullable but contains null values) instead of a clear one.This PR adds the required-field case to the "not present" fallback in
record_batch_transformer.rs: if a field is absent from the data file, has noinitial-default, and is required, returnMissing required field: <name>. The resolution order now mirrors Iceberg-Java's Parquet readers (BaseParquetReaders/SparkParquetReaders.defaultReader): file value, theninitial-default, then null if optional, otherwise error.Are these changes tested?
Yes, a unit test in
record_batch_transformer.rs. Together with the existingtest_all_four_spec_rulesandschema_evolution_adds_date_column_with_nulls, all combinations of a field absent from the data file are covered:initial-defaultreturns the default value (rule Discussion: The design of in memory model of iceberg spec. #3) - existingtest_all_four_spec_rules.test_all_four_spec_rulesandschema_evolution_adds_date_column_with_nulls.Missing required field: <name>- newschema_evolution_required_field_absent_without_default_errors.