Skip to content

Commit 462d2d9

Browse files
committed
fix(schema): normalize default value to the field type at construction
A cross-type default (e.g. an int literal on a long field, accepted to match Java's cast behavior) was stored un-normalized, so projection materialized a wrong-typed fill value and a ToJson/FieldFromJson round-trip compared unequal (Int(34) vs Long(34)). Cast the default to the field type when constructing the field; values that already match are untouched and uncastable ones are left for Validate() to reject. Fixes the projection and round-trip-equality paths.
1 parent 26b1f1b commit 462d2d9

3 files changed

Lines changed: 48 additions & 2 deletions

File tree

src/iceberg/schema_field.cc

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,31 @@
3131

3232
namespace iceberg {
3333

34+
namespace {
35+
36+
// Normalizes a default value to the field type. A cross-type default (e.g. an int
37+
// literal on a long field) is accepted, so it is cast to the field type up front;
38+
// otherwise projection, JSON round-trip and equality would observe a literal whose type
39+
// differs from the field. A value that already matches the field type is returned as-is
40+
// (no needless copy), and a value that cannot be cast is left unchanged so Validate()
41+
// can report it.
42+
std::shared_ptr<const Literal> NormalizeDefault(std::shared_ptr<const Literal> value,
43+
const std::shared_ptr<Type>& field_type) {
44+
if (value == nullptr || field_type == nullptr || !field_type->is_primitive()) {
45+
return value;
46+
}
47+
if (*value->type() == *field_type) {
48+
return value;
49+
}
50+
auto cast = value->CastTo(internal::checked_pointer_cast<PrimitiveType>(field_type));
51+
if (!cast.has_value() || cast->IsAboveMax() || cast->IsBelowMin()) {
52+
return value;
53+
}
54+
return std::make_shared<const Literal>(std::move(cast).value());
55+
}
56+
57+
} // namespace
58+
3459
SchemaField::SchemaField(int32_t field_id, std::string_view name,
3560
std::shared_ptr<Type> type, bool optional, std::string_view doc,
3661
std::shared_ptr<const Literal> initial_default,
@@ -40,8 +65,8 @@ SchemaField::SchemaField(int32_t field_id, std::string_view name,
4065
type_(std::move(type)),
4166
optional_(optional),
4267
doc_(doc),
43-
initial_default_(std::move(initial_default)),
44-
write_default_(std::move(write_default)) {}
68+
initial_default_(NormalizeDefault(std::move(initial_default), type_)),
69+
write_default_(NormalizeDefault(std::move(write_default), type_)) {}
4570

4671
SchemaField SchemaField::MakeOptional(int32_t field_id, std::string_view name,
4772
std::shared_ptr<Type> type, std::string_view doc) {

src/iceberg/test/schema_json_test.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,22 @@ TEST(SchemaJsonTest, NestedFieldWithDefaultValuesRoundTrip) {
241241
ASSERT_EQ(schema_json.dump(), json);
242242
}
243243

244+
TEST(SchemaJsonTest, CrossTypeDefaultNormalizedRoundTrip) {
245+
// A programmatically-constructed cross-type default (an int literal on a long field) is
246+
// normalized to the field type at construction, so it survives a ToJson/SchemaFromJson
247+
// round-trip as an equal schema (an un-normalized int literal would compare unequal to
248+
// the re-parsed long literal).
249+
Schema original({SchemaField(1, "id", int64(), /*optional=*/false, /*doc=*/{},
250+
std::make_shared<const Literal>(Literal::Int(34)))});
251+
const auto& field = original.fields()[0];
252+
ASSERT_TRUE(field.initial_default().has_value());
253+
EXPECT_EQ(field.initial_default()->get(), Literal::Long(34));
254+
255+
ICEBERG_UNWRAP_OR_FAIL(auto json, ToJson(original));
256+
ICEBERG_UNWRAP_OR_FAIL(auto reparsed, SchemaFromJson(json));
257+
EXPECT_EQ(original, *reparsed);
258+
}
259+
244260
TEST(SchemaJsonTest, UnknownFieldRoundTrip) {
245261
constexpr std::string_view json =
246262
R"({"fields":[{"id":1,"name":"mystery","required":false,"type":"unknown"}],"schema-id":1,"type":"struct"})";

src/iceberg/test/schema_test.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,11 @@ TEST(SchemaTest, ValidateCastsDefaultToFieldType) {
177177
std::make_shared<const iceberg::Literal>(iceberg::Literal::Int(34)))});
178178
EXPECT_THAT(widened.Validate(iceberg::TableMetadata::kSupportedTableFormatVersion),
179179
iceberg::IsOk());
180+
// The stored default is normalized to the field type (long), not kept as the narrower
181+
// int literal, so projection/serialization/equality all observe a long.
182+
const auto& widened_field = widened.fields()[0];
183+
ASSERT_TRUE(widened_field.initial_default().has_value());
184+
EXPECT_EQ(widened_field.initial_default()->get(), iceberg::Literal::Long(34));
180185

181186
// A default outside the field type's range is rejected (CastTo returns an
182187
// above-max/below-min sentinel).

0 commit comments

Comments
 (0)