feat: check schema capability when adding parquet files#2158
feat: check schema capability when adding parquet files#2158charlesdong1991 wants to merge 1 commit intoapache:mainfrom
Conversation
kevinjqliu
force-pushed
the
feat/schema-compat-check
branch
from
0141cdc to
354d9b5
Compare
jdockerty
left a comment
jdockerty
left a comment
There was a problem hiding this comment.
This looks quite reasonable to me 👍
| /// in the file schema (looked up by field ID). Follows the same semantics as | ||
| /// iceberg-python's `_check_schema_compatible`. |
There was a problem hiding this comment.
I'm not sure how likely it is that this method name would change, but it is perhaps worth linking to the specific commit that you've referenced this from?
That way the details are clear from looking at the reference implementation.
| /// Returns `true` if `self` can be promoted to `target` per the Iceberg spec's | ||
| /// valid type promotion rules. | ||
| /// | ||
| /// See <https://iceberg.apache.org/spec/#schema-evolution> |
| .map(|s| serde_json::from_str::<NameMapping>(s)) | ||
| .transpose() |
There was a problem hiding this comment.
Is this transpose usage required?
If I'm understanding this rightly, it is being used to flip the map output on Err (after from_str) for the map_err to bubble up afterwards.
Although it seems like some indirection when a match would be more explicit/easy to follow too.
This may just be a preference thing though, feel free to disregard 👍
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions. |
|
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time. |
2 participants
Which issue does this PR close?
What changes are included in this PR?
Adds schema compatibility validation when converting existing parquet files to data files
via
parquet_files_to_data_files.Are these changes tested?
Yes, tests are passing locally