feat: support v3 column default values in UpdateSchema (3/4) - #793
Conversation
| bool is_defaulted_add = false; | ||
| // A column added in this update with a default value can be made required: rows | ||
| // written before the change read the initial-default instead of null. | ||
| bool is_defaulted_add = added_name_to_id_.contains(CaseSensitivityAwareName(name)) && |
There was a problem hiding this comment.
For map/list nested adds, AddColumnInternal stores the new field in added_name_to_id_ using the canonical internal path:
added_name_to_id_[CaseSensitivityAwareName(full_name)] = new_id;
where full_name becomes something like:
locations.value.alt
points.element.z
But RequireColumn receives and checks the user-facing shorthand path:
added_name_to_id_.contains(CaseSensitivityAwareName(name))
where name is typically:
locations.alt
points.z
So the lookup misses, even though it refers to the same added column
There was a problem hiding this comment.
Thanks for catching this -- yes it stored only the canonical full_name but callers use the short name. Fixed by also indexing the short name (matching how the schema name index handles map value / list element paths), with a regression test for requiring a defaulted nested column by short name in the same transaction.
It seems that same bug exists in Java; working on a fix apache/iceberg#17034.
f51fb4a to
d87a2b8
Compare
d87a2b8 to
1eb9681
Compare
4ff0974 to
171e233
Compare
AddColumn / AddRequiredColumn now accept an optional default value, used as both the initial-default and write-default of the new column; a non-null default also lets a required column be added (or an added column be made required) without AllowIncompatibleChanges(). UpdateColumnDefault sets or clears the write-default. Defaults are cast to the column type (rejecting uncastable or out-of-range values) and preserved across rename / doc / type updates and nested field-id reassignment. Part 4 of the v3 column-default-values work (POC apache#731), built on apache#746.
Extract the decimal same-scale precision-widening logic into a shared CastDefaultToType helper and use it in UpdateColumn, UpdateColumnDefault, and AddColumnInternal. Previously only UpdateColumn handled it, so setting or adding a decimal default whose precision differs from the column (e.g. Decimal(_,9,2) into a decimal(18,2) column) was wrongly rejected.
Guard the decimal default fast path in CastDefaultToType with a same-scale check. Decimal stores only the unscaled value, so rebuilding a decimal(9,3) default as decimal(18,2) would reinterpret 1234 from 1.234 to 12.34. Only same-scale precision widening is accepted; a differing scale falls through to CastTo and is rejected, matching Java's requirement that a decimal default's scale match the column type.
f2071f5 to
e524d3e
Compare
wgtmac
left a comment
There was a problem hiding this comment.
This PR overall looks great. I just found a few issues while reviewing this PR and pushed fixes directly to avoid more back-and-forth:
- Removed the added
TableMetadataV3Valid.jsontest fixture and generate v3 metadata in the unit test instead. We don't want to give the confusion that this json file is copied from other repo. - Moved default literal casting out of
update_schema.ccintoSchemaField, closer to Java’sNestedFieldhandling. - Added coverage for default casting, decimal widening, null/range sentinel failures, and
UpdateColumnDefaultno-op behavior. - Fixed
UpdateColumnDefaultto match Java behavior when the new write default is already set. - Added small
SchemaField::With*helpers to make schema field updates less error-prone. - Cleaned up overly verbose comments.
Thanks @huan233usc for working on this and @manuzhang for the great review feedback!
…800) ## What Part 3 of 4 of Iceberg v3 column default-value support (POC #731), built on the schema layer (#746) and the Parquet read path (#792). When a column is present in the read (table) schema but absent from an Avro data file — because the column was added after those rows were written — fill it with the column's v3 `initial-default` instead of `null`. ## Changes - **Avro projection** (`avro_schema_util.cc`): when a field is missing from the file and carries an `initial-default`, project it as `FieldProjection::Kind::kDefault`, mirroring the generic / Parquet paths. - **Avro decode** (`avro_data_util.cc`, `avro_direct_decoder.cc`): materialize the `kDefault` branch through an Avro-local `AppendDefaultToBuilder` helper. It reuses the shared `ToArrowScalar` conversion, while keeping Avro's row-by-row `ArrayBuilder` append behavior out of the shared Arrow utility. ## Tests - `avro_data_test`: `AppendDefaultToBuilder` appends a value and casts to the builder type; `AppendDatumToBuilder` fills missing required and optional default fields. - `avro_test`: end-to-end — write an Avro file with an old schema, then read it through `ReaderFactoryRegistry` with an evolved schema carrying defaults (`ReadMissingFieldsWithDefaults`). ## Stack 1. #746 — schema: represent / serialize / validate (merged) 2. #792 — read path: Parquet (merged) 3. **this PR** — read path: Avro 4. schema evolution: `addColumn` / `updateColumnDefault` (#793)
## What Add casting of numeric literals (`int`, `long`, `float`, `double`) to a decimal target type in `Literal::CastTo`, so a numeric value can be used as a default for a decimal column. Previously `CastFromInt` / `CastFromLong` / `CastFromFloat` / `CastFromDouble` had no `kDecimal` case and fell through to `NotSupported`, so a default like `Literal::Int(12)` or `Literal::Double(9.99)` for a `decimal(9, 2)` column was rejected. Java allows these (`IntegerLiteral.to`, `DoubleLiteral.to`, etc. scale the value to the target scale), so this brings the C++ literal cast layer to parity for numeric sources. ## How - **Integer → decimal**: `CastIntegerToDecimal` treats the integer as scale 0 and rescales it to the target scale via `RescaleHalfUp` (exact when increasing scale, HALF_UP when decreasing it), then verifies the result fits the target precision (`FitsInPrecision`). Example: `12` → `decimal(9,2)` yields unscaled `1200` (`12.00`). - **Float/double → decimal**: `CastRealToDecimal` parses the value's shortest round-tripping decimal representation (matching Java's `BigDecimal.valueOf(double)` via `Double.toString`) into an integer coefficient and exponent-derived scale without expanding scientific notation, then rounds to the target scale with **HALF_UP** rounding (round half away from zero, as Java does — `2.5` → `3`, `-2.5` → `-3`) and checks precision. Non-finite values are rejected. - Both paths reject an out-of-range decimal scale before indexing the powers-of-ten table (`DecimalType` does not bound its scale on construction, so `decimal(9, 40)` would otherwise read past the table). ## Scope Follow-up split out from the v3 default-value work (see the `CastDefaultToType` discussion on #793). It only extends the shared `Literal::CastTo` layer for numeric sources. ## Testing `LiteralTest.IntegerCastToDecimal` (int/long scaling, out-of-precision rejection, out-of-range scale rejection) and `LiteralTest.RealCastToDecimal` (float/double scaling, HALF_UP rounding incl. negative, round-down, out-of-precision and non-finite rejection, scientific-notation overflow rejection and negative-scale acceptance), all verified fail-without / pass-with. Full `expression_test` passes (495 tests).
What
Part 3 of 4 of Iceberg v3 column default-value support (POC #731), built on the
schema-layer support merged in #746. Independent of the read-path PRs (#792 and
the Avro follow-up).
Adds default-value handling to
UpdateSchema(schema evolution).Changes
AddColumn/AddRequiredColumntake an optionaldefault_value. Whenprovided it is set as both the column's
initial-defaultandwrite-default.A non-null default also lets a required column be added without
AllowIncompatibleChanges()— rows written before the change read the defaultinstead of null.
UpdateColumnDefault(name, default)(new) sets, or clears withstd::nullopt, a column'swrite-default; theinitial-defaultis fixed whenthe column is added.
values) and preserved across rename / doc / type-promotion updates and
nested field-id reassignment.
RequireColumnmay now mark a column that was added with a default required.The
SchemaFieldconstructor stores defaults verbatim (it does not coercethem), so the cast/promotion is performed explicitly at each evolution site —
the same effect as Java, where
NestedField's constructor runscastDefault.Same-scale decimal precision widening is handled directly (the unscaled value is
unchanged), since
Literal::CastTodoes not cast between decimal types.Tests
13 cases in
update_schema_test.cc: add optional/required/nested column with adefault, mismatched/narrowing rejection,
UpdateColumnDefault(set / clear / cast-to-type / pre-existing column), require-after-default, and
preservation across doc updates and type promotion — including same-scale
decimal precision promotion.
Stack
addColumn/updateColumnDefault