Skip to content

Commit 4bdf2eb

Browse files
authored
fix: validate decimal default fits the field precision (#809)
## What `SchemaField::Validate` accepts a decimal default whose unscaled value does not fit the field's declared precision, even though the JSON reader rejects exactly that value on read-back — so a validated field can serialize to metadata that the same reader then refuses to parse. Example: a `decimal(2, 1)` field with an `initial-default` of `Literal::Decimal(999, 2, 1)` (`99.9`, which needs 3 significant digits). `ValidateDefault` only checked that the literal's type matches the field type (`decimal(2,1) == decimal(2,1)`), so it passed; the value was serialized to `"99.9"`; on read-back `Decimal::FromString` yields precision 3 > 2 and the decimal literal parser (`expression/json_serde.cc`, and `Literal::CastTo`) rejects it. The field's own metadata becomes unreadable. ## How `ValidateDefault` now checks `Decimal::FitsInPrecision(field precision)` for decimal defaults after the type-equality check, rejecting an out-of-range value up front so it is consistent with what the reader accepts. This mirrors the same guard already applied when casting defaults during schema evolution. ## Testing `SchemaFieldTest.ValidateRejectsDecimalDefaultExceedingPrecision` (rejects `99.9` for `decimal(2,1)`) and `ValidateAcceptsDecimalDefaultWithinPrecision` (accepts a fitting value), verified fail-without / pass-with. Full `schema_test` passes (551 tests).
1 parent 882fc17 commit 4bdf2eb

2 files changed

Lines changed: 34 additions & 0 deletions

File tree

‎src/iceberg/schema_field.cc‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "iceberg/result.h"
3030
#include "iceberg/type.h"
3131
#include "iceberg/util/checked_cast.h"
32+
#include "iceberg/util/decimal.h"
3233
#include "iceberg/util/formatter.h" // IWYU pragma: keep
3334
#include "iceberg/util/macros.h"
3435

@@ -185,6 +186,20 @@ Status ValidateDefault(const SchemaField& field, const Literal& value,
185186
return InvalidSchema("Invalid {} value for {}: value must be finite", kind,
186187
field.name());
187188
}
189+
// A decimal literal carries a precision in its type but stores only an unscaled value,
190+
// so a value that does not fit the field precision would pass the type check above yet
191+
// be rejected when the metadata is parsed back. Reject it here to keep the default
192+
// consistent with what the reader accepts.
193+
if (field.type()->type_id() == TypeId::kDecimal) {
194+
if (const auto* dec = std::get_if<Decimal>(&value.value())) {
195+
const auto& decimal_type =
196+
internal::checked_cast<const DecimalType&>(*field.type());
197+
if (!dec->FitsInPrecision(decimal_type.precision())) {
198+
return InvalidSchema("{} of field {} does not fit precision {}", kind,
199+
field.name(), decimal_type.precision());
200+
}
201+
}
202+
}
188203
return {};
189204
}
190205

‎src/iceberg/test/schema_field_test.cc‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,4 +203,23 @@ TEST(SchemaFieldTest, ValidateAcceptsFiniteFloatingDefault) {
203203
EXPECT_THAT(field.Validate(), IsOk());
204204
}
205205

206+
TEST(SchemaFieldTest, ValidateRejectsDecimalDefaultExceedingPrecision) {
207+
// A decimal default whose unscaled value needs more digits than the field precision has
208+
// the matching literal type (decimal(2, 1)) but does not fit; Validate must reject it
209+
// so it is never serialized to metadata the reader would later refuse to parse.
210+
SchemaField field(/*field_id=*/1, /*name=*/"d", decimal(2, 1),
211+
/*optional=*/true, /*doc=*/"",
212+
std::make_shared<const Literal>(Literal::Decimal(999, 2, 1)));
213+
auto status = field.Validate();
214+
EXPECT_THAT(status, IsError(ErrorKind::kInvalidSchema));
215+
EXPECT_THAT(status, HasErrorMessage("does not fit precision"));
216+
}
217+
218+
TEST(SchemaFieldTest, ValidateAcceptsDecimalDefaultWithinPrecision) {
219+
SchemaField field(/*field_id=*/1, /*name=*/"d", decimal(4, 1),
220+
/*optional=*/true, /*doc=*/"",
221+
std::make_shared<const Literal>(Literal::Decimal(999, 4, 1)));
222+
EXPECT_THAT(field.Validate(), IsOk());
223+
}
224+
206225
} // namespace iceberg

0 commit comments

Comments
 (0)