Skip to content

Commit 955562b

Browse files
authored
fix: reject non-finite floating-point default values (#810)
## What A `float` / `double` column default of `NaN` or `±Infinity` passes `SchemaField::Validate` but cannot survive a metadata round-trip: the JSON serializer emits a non-finite number as `null`, which reads back as an *absent* default. The declared default is silently lost (and, because the key is present but null, the reader then errors on it). ## How `ValidateDefault` now rejects a non-finite floating default with a clear "value must be finite" error, so an unrepresentable default is caught up front rather than silently dropped on serialization. The check uses `std::isfinite`, which is false for **both** `NaN` and `±Infinity` — so a single condition covers all three cases (there is no separate `IsNaN` branch because `isfinite` already rejects NaN). ## Testing `SchemaFieldTest.ValidateRejectsNonFiniteFloatingDefault` explicitly covers `NaN`, `+Inf` and `-Inf`, and `ValidateAcceptsFiniteFloatingDefault` covers a normal value; verified fail-without / pass-with. Full `schema_test` passes (551 tests).
1 parent 9221381 commit 955562b

2 files changed

Lines changed: 50 additions & 0 deletions

File tree

src/iceberg/schema_field.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919

2020
#include "iceberg/schema_field.h"
2121

22+
#include <cmath>
2223
#include <format>
2324
#include <string_view>
2425
#include <utility>
2526
#include <variant>
2627

2728
#include "iceberg/expression/literal.h"
29+
#include "iceberg/result.h"
2830
#include "iceberg/type.h"
2931
#include "iceberg/util/checked_cast.h"
3032
#include "iceberg/util/formatter.h" // IWYU pragma: keep
@@ -168,6 +170,21 @@ Status ValidateDefault(const SchemaField& field, const Literal& value,
168170
return InvalidSchema("{} of field {} has type {} but expected {}", kind, field.name(),
169171
*value.type(), *field.type());
170172
}
173+
// A non-finite float/double default cannot be represented in JSON: the serializer emits
174+
// it as `null`, which reads back as an absent default. Reject it so the default is not
175+
// silently lost when the metadata round-trips. The literal type already matches the
176+
// field type here, so this only inspects the value for actual floating fields and other
177+
// types pay nothing.
178+
if (field.type()->type_id() == TypeId::kFloat &&
179+
!std::isfinite(std::get<float>(value.value()))) {
180+
return InvalidSchema("Invalid {} value for {}: value must be finite", kind,
181+
field.name());
182+
}
183+
if (field.type()->type_id() == TypeId::kDouble &&
184+
!std::isfinite(std::get<double>(value.value()))) {
185+
return InvalidSchema("Invalid {} value for {}: value must be finite", kind,
186+
field.name());
187+
}
171188
return {};
172189
}
173190

src/iceberg/test/schema_field_test.cc

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,14 @@
2020
#include "iceberg/schema_field.h"
2121

2222
#include <format>
23+
#include <limits>
2324
#include <memory>
2425

2526
#include <gtest/gtest.h>
2627

2728
#include "iceberg/expression/literal.h"
29+
#include "iceberg/result.h"
30+
#include "iceberg/test/matchers.h"
2831
#include "iceberg/type.h"
2932
#include "iceberg/util/formatter.h" // IWYU pragma: keep
3033

@@ -170,4 +173,34 @@ TEST(SchemaFieldTest, CastDefaultValue) {
170173
}
171174
}
172175

176+
TEST(SchemaFieldTest, ValidateRejectsNonFiniteFloatingDefault) {
177+
// NaN / infinity cannot be represented in JSON (the serializer emits `null`, which
178+
// reads back as an absent default), so a non-finite floating default must be rejected.
179+
SchemaField nan_field(/*field_id=*/1, /*name=*/"f", float32(),
180+
/*optional=*/true, /*doc=*/"",
181+
std::make_shared<const Literal>(
182+
Literal::Float(std::numeric_limits<float>::quiet_NaN())));
183+
EXPECT_THAT(nan_field.Validate(), IsError(ErrorKind::kInvalidSchema));
184+
EXPECT_THAT(nan_field.Validate(), HasErrorMessage("must be finite"));
185+
186+
SchemaField inf_field(/*field_id=*/2, /*name=*/"d", float64(),
187+
/*optional=*/true, /*doc=*/"",
188+
std::make_shared<const Literal>(
189+
Literal::Double(std::numeric_limits<double>::infinity())));
190+
EXPECT_THAT(inf_field.Validate(), IsError(ErrorKind::kInvalidSchema));
191+
192+
SchemaField neg_inf_field(/*field_id=*/3, /*name=*/"d2", float64(),
193+
/*optional=*/true, /*doc=*/"",
194+
std::make_shared<const Literal>(Literal::Double(
195+
-std::numeric_limits<double>::infinity())));
196+
EXPECT_THAT(neg_inf_field.Validate(), IsError(ErrorKind::kInvalidSchema));
197+
}
198+
199+
TEST(SchemaFieldTest, ValidateAcceptsFiniteFloatingDefault) {
200+
SchemaField field(/*field_id=*/1, /*name=*/"f", float32(),
201+
/*optional=*/true, /*doc=*/"",
202+
std::make_shared<const Literal>(Literal::Float(1.5f)));
203+
EXPECT_THAT(field.Validate(), IsOk());
204+
}
205+
173206
} // namespace iceberg

0 commit comments

Comments
 (0)