Skip to content

Commit 2bec976

Browse files
huan233uscwgtmac
authored andcommitted
share decimal default promotion across add/update column default
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.
1 parent ea6c4ee commit 2bec976

2 files changed

Lines changed: 58 additions & 14 deletions

File tree

src/iceberg/test/update_schema_test.cc

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,41 @@ TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnTypePromotesDecimalDefault) {
361361
EXPECT_EQ(*field.write_default(), Literal::Decimal(1234, 18, 2));
362362
}
363363

364+
TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithWiderPrecisionDecimalDefault) {
365+
// A decimal default whose precision differs from the column (same scale) is accepted:
366+
// Literal::CastTo does not cast between decimal types, so the default is rebuilt at the
367+
// column's precision instead of being rejected.
368+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema());
369+
update->AddColumn("new_col", decimal(18, 2), "A decimal column",
370+
Literal::Decimal(1234, 9, 2));
371+
372+
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
373+
ICEBERG_UNWRAP_OR_FAIL(auto field_opt, result.schema->FindFieldByName("new_col"));
374+
ASSERT_TRUE(field_opt.has_value());
375+
376+
const auto& field = field_opt->get();
377+
ASSERT_NE(field.initial_default(), nullptr);
378+
EXPECT_EQ(*field.initial_default(), Literal::Decimal(1234, 18, 2));
379+
ASSERT_NE(field.write_default(), nullptr);
380+
EXPECT_EQ(*field.write_default(), Literal::Decimal(1234, 18, 2));
381+
}
382+
383+
TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDefaultWiderPrecisionDecimal) {
384+
// UpdateColumnDefault accepts a decimal default whose precision differs from the
385+
// column (same scale), rebuilding it at the column's precision.
386+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema());
387+
update->AddColumn("new_col", decimal(18, 2), "A decimal column")
388+
.UpdateColumnDefault("new_col", Literal::Decimal(1234, 9, 2));
389+
390+
ICEBERG_UNWRAP_OR_FAIL(auto result, update->Apply());
391+
ICEBERG_UNWRAP_OR_FAIL(auto field_opt, result.schema->FindFieldByName("new_col"));
392+
ASSERT_TRUE(field_opt.has_value());
393+
394+
const auto& field = field_opt->get();
395+
ASSERT_NE(field.write_default(), nullptr);
396+
EXPECT_EQ(*field.write_default(), Literal::Decimal(1234, 18, 2));
397+
}
398+
364399
TEST_F(UpdateSchemaTest, AddMultipleColumns) {
365400
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema());
366401
update->AddColumn("col1", int32(), "First column")

src/iceberg/update/update_schema.cc

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,24 @@ std::vector<SchemaField> ApplyChangesVisitor::MoveFields(
289289
return reordered;
290290
}
291291

292+
/// \brief Cast a default value literal to a target primitive type.
293+
///
294+
/// `Literal::CastTo` only returns a value for identical decimal types, but schema
295+
/// evolution permits widening a decimal to a larger precision at the same scale. Such a
296+
/// promotion leaves the unscaled value unchanged, so the literal is rebuilt with the
297+
/// target type instead of going through `CastTo`. All other conversions delegate to
298+
/// `CastTo`, which reports narrowing via AboveMax/BelowMin sentinels that callers reject.
299+
Result<Literal> CastDefaultToType(const Literal& value,
300+
const std::shared_ptr<PrimitiveType>& target_type) {
301+
if (value.type()->type_id() == TypeId::kDecimal &&
302+
target_type->type_id() == TypeId::kDecimal) {
303+
const auto& decimal_type = internal::checked_cast<const DecimalType&>(*target_type);
304+
return Literal::Decimal(std::get<Decimal>(value.value()).value(),
305+
decimal_type.precision(), decimal_type.scale());
306+
}
307+
return value.CastTo(target_type);
308+
}
309+
292310
} // namespace
293311

294312
Result<std::shared_ptr<UpdateSchema>> UpdateSchema::Make(
@@ -419,17 +437,7 @@ UpdateSchema& UpdateSchema::UpdateColumn(std::string_view name,
419437
if (value == nullptr) {
420438
return nullptr;
421439
}
422-
// IsPromotionAllowed permits widening a decimal to a larger precision at the same
423-
// scale, but Literal::CastTo only handles identical decimal types. Such a promotion
424-
// leaves the unscaled value unchanged, so rebuild the literal with the new type.
425-
if (value->type()->type_id() == TypeId::kDecimal &&
426-
new_type->type_id() == TypeId::kDecimal) {
427-
const auto& decimal_type = internal::checked_cast<const DecimalType&>(*new_type);
428-
return std::make_shared<const Literal>(
429-
Literal::Decimal(std::get<Decimal>(value->value()).value(),
430-
decimal_type.precision(), decimal_type.scale()));
431-
}
432-
ICEBERG_ASSIGN_OR_RAISE(Literal promoted, value->CastTo(new_type));
440+
ICEBERG_ASSIGN_OR_RAISE(Literal promoted, CastDefaultToType(*value, new_type));
433441
return std::make_shared<const Literal>(std::move(promoted));
434442
};
435443
ICEBERG_BUILDER_ASSIGN_OR_RETURN(std::shared_ptr<const Literal> initial_default,
@@ -496,7 +504,8 @@ UpdateSchema& UpdateSchema::UpdateColumnDefault(std::string_view name,
496504
*new_default);
497505
ICEBERG_BUILDER_ASSIGN_OR_RETURN_WITH_ERROR(
498506
Literal typed_default,
499-
new_default->CastTo(internal::checked_pointer_cast<PrimitiveType>(field.type())),
507+
CastDefaultToType(*new_default,
508+
internal::checked_pointer_cast<PrimitiveType>(field.type())),
500509
"Cannot cast default value to {}: {}", *field.type(), *new_default);
501510
// CastTo reports narrowing by returning sentinel values instead of failing.
502511
ICEBERG_BUILDER_CHECK(!typed_default.IsNull() && !typed_default.IsAboveMax() &&
@@ -797,8 +806,8 @@ UpdateSchema& UpdateSchema::AddColumnInternal(std::optional<std::string_view> pa
797806
*type_with_fresh_ids, *default_value);
798807
ICEBERG_BUILDER_ASSIGN_OR_RETURN_WITH_ERROR(
799808
Literal typed_default,
800-
default_value->CastTo(
801-
internal::checked_pointer_cast<PrimitiveType>(type_with_fresh_ids)),
809+
CastDefaultToType(*default_value, internal::checked_pointer_cast<PrimitiveType>(
810+
type_with_fresh_ids)),
802811
"Cannot cast default value to {}: {}", *type_with_fresh_ids, *default_value);
803812
// CastTo reports narrowing by returning sentinel values instead of failing.
804813
ICEBERG_BUILDER_CHECK(!typed_default.IsNull() && !typed_default.IsAboveMax() &&

0 commit comments

Comments
 (0)