Skip to content

Commit f0ac7a2

Browse files
huan233uscwgtmac
authored andcommitted
reject decimal default with mismatched scale
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.
1 parent 2bec976 commit f0ac7a2

2 files changed

Lines changed: 33 additions & 5 deletions

File tree

src/iceberg/test/update_schema_test.cc

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,28 @@ TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDefaultWiderPrecisionDecimal) {
396396
EXPECT_EQ(*field.write_default(), Literal::Decimal(1234, 18, 2));
397397
}
398398

399+
TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithDifferentScaleDecimalDefaultFails) {
400+
// A decimal default whose scale differs from the column is rejected, not silently
401+
// reinterpreted: the unscaled value only keeps its meaning at the same scale.
402+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema());
403+
update->AddColumn("new_col", decimal(18, 2), "A decimal column",
404+
Literal::Decimal(1234, 9, 3));
405+
406+
auto result = update->Apply();
407+
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
408+
EXPECT_THAT(result, HasErrorMessage("Cannot cast default value"));
409+
}
410+
411+
TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDefaultDifferentScaleDecimalFails) {
412+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema());
413+
update->AddColumn("new_col", decimal(18, 2), "A decimal column")
414+
.UpdateColumnDefault("new_col", Literal::Decimal(1234, 9, 3));
415+
416+
auto result = update->Apply();
417+
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
418+
EXPECT_THAT(result, HasErrorMessage("Cannot cast default value"));
419+
}
420+
399421
TEST_F(UpdateSchemaTest, AddMultipleColumns) {
400422
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema());
401423
update->AddColumn("col1", int32(), "First column")

src/iceberg/update/update_schema.cc

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -292,17 +292,23 @@ std::vector<SchemaField> ApplyChangesVisitor::MoveFields(
292292
/// \brief Cast a default value literal to a target primitive type.
293293
///
294294
/// `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
295+
/// evolution permits widening a decimal to a larger precision at the SAME scale. Such a
296296
/// 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.
297+
/// target type instead of going through `CastTo`. A scale change is NOT handled here: the
298+
/// unscaled value only keeps its meaning at the same scale (a decimal default must match
299+
/// the column scale), so differing scales fall through to `CastTo` and are rejected. All
300+
/// other conversions delegate to `CastTo`, which reports narrowing via AboveMax/BelowMin
301+
/// sentinels that callers reject.
299302
Result<Literal> CastDefaultToType(const Literal& value,
300303
const std::shared_ptr<PrimitiveType>& target_type) {
301304
if (value.type()->type_id() == TypeId::kDecimal &&
302305
target_type->type_id() == TypeId::kDecimal) {
306+
const auto& source_type = internal::checked_cast<const DecimalType&>(*value.type());
303307
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());
308+
if (source_type.scale() == decimal_type.scale()) {
309+
return Literal::Decimal(std::get<Decimal>(value.value()).value(),
310+
decimal_type.precision(), decimal_type.scale());
311+
}
306312
}
307313
return value.CastTo(target_type);
308314
}

0 commit comments

Comments
 (0)