Read partitioned tables with source field missing from schema #2367
Read partitioned tables with source field missing from schema #2367Fokko merged 2 commits intoapache:mainfrom
Conversation
| source_field = schema.find_field(field.source_id) | ||
| result_type = field.transform.result_type(source_field.field_type) | ||
| nested_fields.append(NestedField(field.field_id, field.name, result_type, required=source_field.required)) | ||
| else: |
There was a problem hiding this comment.
Wanted to get some opinions for just allowing this for VoidTransforms fields, as as of now we can drop columns without dropping the partition first
There was a problem hiding this comment.
Unfortunally, dropping is not an option. The V1 tables do not have field-IDs encoded in the struct, and is purely positional based. See the spec for details. Dropping a field, would change the position, potentially resulting in data integrity issues.
There was a problem hiding this comment.
@Fokko I was referring more to that if the field's transform is not VoidTransform then we should potentially fail here. Java does not allow dropping a schema column if there is an non-void partition referencing that field, but here it is not guaranteed that if there is no source-field for a partition then the partition will be void.
There was a problem hiding this comment.
@Fokko gentle pin as this might gone off radar :)
There was a problem hiding this comment.
I think this is fine, if I understand the concern correctly. The worry is that we're allowing unknown type for non void transforms when the source field is missing.
On the Java side, when we're reading partition specs from the metadata, we are using the allow missing fields equal to true, which skips the validation if the source is missing(here).
This validation is only hit when we're constructing new partition specs. When we're reading, it's fine.
There was a problem hiding this comment.
Sorry, I think I misunderstood the question, but nothing is being dropped here, so that all looks good.
gabeiglio
force-pushed
the
read-dropped-fields
branch
from
a7b118b to
d93868c
Compare
gabeiglio
force-pushed
the
read-dropped-fields
branch
from
d93868c to
65b14f9
Compare
Fokko
left a comment
Fokko
left a comment
There was a problem hiding this comment.
Thanks @gabeiglio for fixing this, and thanks @geruh for the review 🙌
3 participants
Rationale for this change
Following with the Java solution implementation on how to read partition specs when a source field was dropped.
Are these changes tested?
Yes, added one integration tests, and one unit test
Are there any user-facing changes?
No