Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/iceberg/schema_field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@

#include "iceberg/schema_field.h"

#include <cmath>
#include <format>
#include <string_view>
#include <utility>
#include <variant>

#include "iceberg/expression/literal.h"
#include "iceberg/type.h"
Expand Down Expand Up @@ -124,6 +126,18 @@ Status ValidateDefault(const SchemaField& field, const Literal& value,
return InvalidSchema("{} of field {} has type {} but expected {}", kind, field.name(),
*value.type(), *field.type());
}
// A non-finite float/double default cannot be represented in JSON: the serializer emits
// it as `null`, which reads back as an absent default. Reject it so the default is not
// silently lost when the metadata round-trips.
const auto& literal_value = value.value();
const bool non_finite = (std::holds_alternative<float>(literal_value) &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mentioned NaN in the PR description but doesn't seem to handle that, is that intentional?

@huan233usc huan233usc Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is handled by following std::isfinite returns false for both NaN and infinity, so a NaN default hits the same non_finite branch and is rejected. The test ValidateRejectsNonFiniteFloatingDefault covers NaN, +Inf and -Inf explicitly.

!std::isfinite(std::get<float>(literal_value))) ||
(std::holds_alternative<double>(literal_value) &&
!std::isfinite(std::get<double>(literal_value)));
if (non_finite) {
return InvalidSchema("Invalid {} value for {}: value must be finite", kind,
field.name());
}
return {};
}

Expand Down
33 changes: 33 additions & 0 deletions src/iceberg/test/schema_field_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@
#include "iceberg/schema_field.h"

#include <format>
#include <limits>
#include <memory>

#include <gtest/gtest.h>

#include "iceberg/expression/literal.h"
#include "iceberg/test/matchers.h"
#include "iceberg/type.h"
#include "iceberg/util/formatter.h" // IWYU pragma: keep

Expand Down Expand Up @@ -107,4 +110,34 @@ TEST(SchemaFieldTest, WithDoc) {
}
}

TEST(SchemaFieldTest, ValidateRejectsNonFiniteFloatingDefault) {
// NaN / infinity cannot be represented in JSON (the serializer emits `null`, which
// reads back as an absent default), so a non-finite floating default must be rejected.
SchemaField nan_field(/*field_id=*/1, /*name=*/"f", float32(),
/*optional=*/true, /*doc=*/"",
std::make_shared<const Literal>(
Literal::Float(std::numeric_limits<float>::quiet_NaN())));
EXPECT_THAT(nan_field.Validate(), IsError(ErrorKind::kInvalidSchema));
EXPECT_THAT(nan_field.Validate(), HasErrorMessage("must be finite"));

SchemaField inf_field(/*field_id=*/2, /*name=*/"d", float64(),
/*optional=*/true, /*doc=*/"",
std::make_shared<const Literal>(
Literal::Double(std::numeric_limits<double>::infinity())));
EXPECT_THAT(inf_field.Validate(), IsError(ErrorKind::kInvalidSchema));

SchemaField neg_inf_field(/*field_id=*/3, /*name=*/"d2", float64(),
/*optional=*/true, /*doc=*/"",
std::make_shared<const Literal>(Literal::Double(
-std::numeric_limits<double>::infinity())));
EXPECT_THAT(neg_inf_field.Validate(), IsError(ErrorKind::kInvalidSchema));
}

TEST(SchemaFieldTest, ValidateAcceptsFiniteFloatingDefault) {
SchemaField field(/*field_id=*/1, /*name=*/"f", float32(),
/*optional=*/true, /*doc=*/"",
std::make_shared<const Literal>(Literal::Float(1.5f)));
EXPECT_THAT(field.Validate(), IsOk());
}

} // namespace iceberg
Loading