AddFiles: SchemaDelta classifies what a file schema needs from the table - #40062
AddFiles: SchemaDelta classifies what a file schema needs from the table#40062claudevdm wants to merge 10 commits into
Conversation
The classifier behind the options. Iceberg's unionByNameWith has no knobs: it adds, relaxes and promotes in one go, or throws. To honour ALLOW_FIELD_ADDITION / ALLOW_FIELD_RELAXATION / ALLOW_TYPE_PROMOTION separately, SchemaDelta.classify(table, fileSchema) applies the union on a throwaway UpdateSchema (apply(), never commit()), diffs the result against the current table schema, and labels every change: - FIELD_ADDITION: a field id present only after the union. - FIELD_RELAXATION: required before, optional after; or a required table column with no counterpart in the file at all (see below). - TYPE_PROMOTION: same id, wider primitive type after. - CONFLICT: anything else. The union throwing (ValidationException or IllegalArgumentException, e.g. int column vs string file column, a dotted or empty file column name), a field removed by the union (cannot happen with unionByName but is refused rather than trusted), a struct where a primitive was, a doc string or default changing, a promotion Iceberg would not allow (TypeUtil.isPromotionAllowed guard, so a bad union result is never staged as a "promotion"). The diff is keyed by field id and walks fields attribute by attribute (name, optionality, type kind, doc, defaults), so an attribute the union silently changes is reported rather than committed unnoticed. Changes are listed in a deterministic order (unquoted path) with quoted names in messages so a reviewer can find them in the schema. Absence rule: a required table column that the file lacks is a FIELD_RELAXATION, not a pass. Registering such a file would put nulls in a required column for every reader; the fix is to relax the column explicitly (the commit side stages makeColumnOptional for exactly the paths absentRequiredPaths() reports). The walk descends through structs whose parent is present, through list elements and map values (paths use "element" and "value", which makeColumnOptional accepts); an absent struct is itself the relaxation, its children are not listed separately; map keys are required by definition and skipped. Pins: Change.allowedBy(config) refuses a relaxation of a pinned path even when ALLOW_FIELD_RELAXATION is set, and disallowedReason(config) names it, so "id is pinned" shows up as the reason rather than a generic "relaxation not allowed".
| } | ||
|
|
||
| /** | ||
| * File column names no table can absorb, checked at every level including structs the table does |
There was a problem hiding this comment.
To clarify are these conflicts for Iceberg itself or just for out solution. I think if we make column names more strict than Iceberg, some customers might ran into issues (we ran into similar issues before with BigQuery).
There was a problem hiding this comment.
Iceberg accepts a literal a.b column through unionByNameWith. But icebergs addColumn rejects dots as "ambiguous name". So upstream laready treats creating them as user error that unionByNameWith can bypass.
Empty names are rejected by iceberg at the top level. Empty nested names are silently added but I think that is not a valid use case and missing validation in iceberg?
Case collisions are legal in iceberg file format, but it breaks reader implementations. Users can work around this with a column alias.
So I think these are protecting the user against configuring something that might blow up without their knowledge.
|
Assigning reviewers: R: @kennknowles for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
github-actions
Bot
added
iceberg
|
R: @chamikaramj |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
3 participants
The classifier behind the options. Iceberg's unionByNameWith has no knobs: it adds, relaxes and promotes in one go, or throws. To honour ALLOW_FIELD_ADDITION / ALLOW_FIELD_RELAXATION / ALLOW_TYPE_PROMOTION separately, SchemaDelta.classify(table, fileSchema) applies the union on a throwaway UpdateSchema (apply(), never commit()), diffs the result against the current table schema, and labels every change:
The diff is keyed by field id and walks fields attribute by attribute (name, optionality, type kind, doc, defaults), so an attribute the union silently changes is reported rather than committed unnoticed. Changes are listed in a deterministic order (unquoted path) with quoted names in messages so a reviewer can find them in the schema.
Absence rule: a required table column that the file lacks is a FIELD_RELAXATION, not a pass. Registering such a file would put nulls in a required column for every reader; the fix is to relax the column explicitly (the commit side stages makeColumnOptional for exactly the paths absentRequiredPaths() reports). The walk descends through structs whose parent is present, through list elements and map values (paths use "element" and "value", which makeColumnOptional accepts); an absent struct is itself the relaxation, its children are not listed separately; map keys are required by definition and skipped.
Pins: Change.allowedBy(config) refuses a relaxation of a pinned path even when ALLOW_FIELD_RELAXATION is set, and disallowedReason(config) names it, so "id is pinned" shows up as the reason rather than a generic "relaxation not allowed".
Please add a meaningful description for your change here
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.