Skip to content

Commit f2071f5

Browse files
committed
guard decimal default fast path against null and out-of-precision values
1 parent 171e233 commit f2071f5

2 files changed

Lines changed: 45 additions & 4 deletions

File tree

src/iceberg/test/update_schema_test.cc

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,32 @@ TEST_F(UpdateSchemaDefaultValueTest, UpdateColumnDefaultDifferentScaleDecimalFai
418418
EXPECT_THAT(result, HasErrorMessage("Cannot cast default value"));
419419
}
420420

421+
TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithOutOfPrecisionDecimalDefaultFails) {
422+
// A same-scale decimal default whose unscaled value exceeds the column precision is
423+
// rejected here, rather than being relabeled and later rejected by the JSON parser when
424+
// the metadata is read back.
425+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema());
426+
update->AddColumn("new_col", decimal(4, 2), "A decimal column",
427+
Literal::Decimal(1234567, 9, 2));
428+
429+
auto result = update->Apply();
430+
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
431+
EXPECT_THAT(result, HasErrorMessage("Cannot cast default value"));
432+
}
433+
434+
TEST_F(UpdateSchemaDefaultValueTest, AddColumnWithTypedNullDecimalDefaultFails) {
435+
// A typed null default carries decimal type metadata but no Decimal payload; the
436+
// same-scale fast path must not read a Decimal out of it (which would be undefined),
437+
// so it falls through and is rejected as a null default.
438+
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema());
439+
update->AddColumn("new_col", decimal(18, 2), "A decimal column",
440+
Literal::Null(decimal(18, 2)));
441+
442+
auto result = update->Apply();
443+
EXPECT_THAT(result, IsError(ErrorKind::kValidationFailed));
444+
EXPECT_THAT(result, HasErrorMessage("Cannot cast default value"));
445+
}
446+
421447
TEST_F(UpdateSchemaTest, AddMultipleColumns) {
422448
ICEBERG_UNWRAP_OR_FAIL(auto update, table_->NewUpdateSchema());
423449
update->AddColumn("col1", int32(), "First column")

src/iceberg/update/update_schema.cc

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -300,18 +300,33 @@ std::vector<SchemaField> ApplyChangesVisitor::MoveFields(
300300
/// other conversions delegate to `CastTo`, which reports narrowing via AboveMax/BelowMin
301301
/// sentinels that callers reject.
302302
///
303+
/// The fast path only fires for a literal that actually holds a `Decimal` value: a typed
304+
/// null default (`Literal::Null(decimal(...))`) or a sentinel carries decimal type
305+
/// metadata but no `Decimal` payload, so those fall through to `CastTo` and are handled
306+
/// by the same null/sentinel rejection the callers already apply, rather than being
307+
/// force-relabeled.
308+
///
303309
/// \pre `target_type` is non-null; every caller passes a resolved field type. `value` is
304310
/// the caller's dereferenced default, so a missing default must be filtered out before
305311
/// this is reached.
306312
Result<Literal> CastDefaultToType(const Literal& value,
307313
const std::shared_ptr<PrimitiveType>& target_type) {
308-
if (value.type()->type_id() == TypeId::kDecimal &&
309-
target_type->type_id() == TypeId::kDecimal) {
314+
if (target_type->type_id() == TypeId::kDecimal &&
315+
std::holds_alternative<Decimal>(value.value())) {
310316
const auto& source_type = internal::checked_cast<const DecimalType&>(*value.type());
311317
const auto& decimal_type = internal::checked_cast<const DecimalType&>(*target_type);
312318
if (source_type.scale() == decimal_type.scale()) {
313-
return Literal::Decimal(std::get<Decimal>(value.value()).value(),
314-
decimal_type.precision(), decimal_type.scale());
319+
const auto& decimal_value = std::get<Decimal>(value.value());
320+
// Relabeling keeps the unscaled value but adopts the target precision, so reject a
321+
// value that does not fit it. Otherwise the widened literal passes schema
322+
// validation yet serializes to metadata the JSON parser later rejects for exceeding
323+
// precision.
324+
if (!decimal_value.FitsInPrecision(decimal_type.precision())) {
325+
return InvalidArgument("Cannot cast default value to {}: {}", *target_type,
326+
value);
327+
}
328+
return Literal::Decimal(decimal_value.value(), decimal_type.precision(),
329+
decimal_type.scale());
315330
}
316331
}
317332
return value.CastTo(target_type);

0 commit comments

Comments
 (0)