This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new b95d4fc80 fix!: reject NaN literals when binding predicates (#2432)
b95d4fc80 is described below
commit b95d4fc8018ed6a6def40a97e4926401577af005
Author: Revanth <[email protected]>
AuthorDate: Wed Nov 4 12:29:14 2026 -0600
fix!: reject NaN literals when binding predicates (#2432)
---
bound_set_literal_view_internal_test.go | 8 +---
exprs.go | 44 ++++++++++++++++++-
exprs_test.go | 62 +++++++++++++++++++++++++++
table/conflict_validation.go | 32 +++++++++++++-
table/partition_conflict_test.go | 75 +++++++++++++++++++++++++++++++--
table/partition_residual_null_test.go | 2 -
website/src/releases.md | 1 +
website/src/row-filter-syntax.md | 2 +-
8 files changed, 208 insertions(+), 18 deletions(-)
diff --git a/bound_set_literal_view_internal_test.go
b/bound_set_literal_view_internal_test.go
index 87ec273ad..9d21b893b 100644
--- a/bound_set_literal_view_internal_test.go
+++ b/bound_set_literal_view_internal_test.go
@@ -102,9 +102,6 @@ func
TestBoundSetPredicateLiteralsPreserveFloatingPointMembers(t *testing.T) {
name: "float32",
typ: PrimitiveTypes.Float32,
values: []Literal{
- NewLiteral(math.Float32frombits(0x7fc00001)),
- NewLiteral(math.Float32frombits(0x7fc00001)),
- NewLiteral(math.Float32frombits(0x7fc00002)),
NewLiteral(float32(0)),
NewLiteral(math.Float32frombits(0x80000000)),
NewLiteral(float32(math.Inf(-1))),
@@ -119,9 +116,6 @@ func
TestBoundSetPredicateLiteralsPreserveFloatingPointMembers(t *testing.T) {
name: "float64",
typ: PrimitiveTypes.Float64,
values: []Literal{
-
NewLiteral(math.Float64frombits(0x7ff8000000000001)),
-
NewLiteral(math.Float64frombits(0x7ff8000000000001)),
-
NewLiteral(math.Float64frombits(0x7ff8000000000002)),
NewLiteral(float64(0)),
NewLiteral(math.Float64frombits(0x8000000000000000)),
NewLiteral(math.Inf(-1)),
@@ -143,7 +137,7 @@ func
TestBoundSetPredicateLiteralsPreserveFloatingPointMembers(t *testing.T) {
require.NoError(t, err)
original :=
bound.(boundSetLiteralRef).boundSetLiteralsRef()
cloned := bound.(BoundSetPredicate).Literals()
- require.Equal(t, 7, original.Len())
+ require.Equal(t, 4, original.Len())
require.Equal(t, original.Len(), cloned.Len())
counts := func(lits Set[Literal])
map[uint64]int {
diff --git a/exprs.go b/exprs.go
index cb427a761..2bc5d5c1d 100644
--- a/exprs.go
+++ b/exprs.go
@@ -712,7 +712,8 @@ func (bp *boundUnaryPredicate[T]) String() string {
// a single literal argument, such as LessThan or StartsWith.
//
// Panics if the operation provided is not a valid Literal operation,
-// if the term is nil or if the literal is nil.
+// if the term is nil or if the literal is nil. Binding the predicate fails
+// with ErrInvalidArgument for a NaN literal; use IsNaN or NotNaN instead.
func LiteralPredicate(op Operation, t UnboundTerm, lit Literal)
UnboundPredicate {
switch {
case op < OpLT || op > OpNotStartsWith:
@@ -729,6 +730,32 @@ func LiteralPredicate(op Operation, t UnboundTerm, lit
Literal) UnboundPredicate
return &unboundLiteralPredicate{op: op, term: t, lit: lit}
}
+// isNaNLiteral reports whether lit is a floating-point NaN.
+func isNaNLiteral(lit Literal) bool {
+ switch l := lit.(type) {
+ case Float32Literal:
+ return math.IsNaN(float64(l))
+ case Float64Literal:
+ return math.IsNaN(float64(l))
+ }
+
+ return false
+}
+
+// checkNotNaN rejects a NaN literal. x = NaN is never true under IEEE 754, so
+// NaN must be matched with IsNaN or NotNaN, as in Java. Bind checks the
+// literal both before and after converting it to the term's type: a NaN
+// converted to a decimal becomes zero, and the string "NaN" converted to a
+// float becomes NaN.
+func checkNotNaN(op Operation, term BoundTerm, lit Literal) error {
+ if isNaNLiteral(lit) {
+ return fmt.Errorf("%w: cannot bind %s on %q with a NaN literal,
use IsNaN or NotNaN instead",
+ ErrInvalidArgument, op, term.Ref().Field().Name)
+ }
+
+ return nil
+}
+
type unboundLiteralPredicate struct {
op Operation
term UnboundTerm
@@ -764,10 +791,16 @@ func (ul *unboundLiteralPredicate) Bind(schema *Schema,
caseSensitive bool) (Boo
ErrType, bound.Type())
}
+ if err := checkNotNaN(ul.op, bound, ul.lit); err != nil {
+ return nil, err
+ }
lit, err := ul.lit.To(bound.Type())
if err != nil {
return nil, err
}
+ if err := checkNotNaN(ul.op, bound, lit); err != nil {
+ return nil, err
+ }
// A finite literal beyond the range of a floating-point term lies
between
// the largest finite value and infinity, so an ordering comparison
against
@@ -903,7 +936,8 @@ func (blp *boundLiteralPredicate[T]) AsUnbound(r Reference,
l Literal) UnboundPr
// of literals as the argument, like In or NotIn. Duplicate literals will be
folded
// into a set, only maintaining the unique literals.
//
-// Will panic if op is not a valid Set operation
+// Will panic if op is not a valid Set operation. Binding the predicate fails
+// with ErrInvalidArgument if any literal is NaN; use IsNaN or NotNaN instead.
func SetPredicate(op Operation, t UnboundTerm, lits []Literal)
BooleanExpression {
if op < OpIn || op > OpNotIn {
panic(fmt.Errorf("%w: invalid operation for SetPredicate: %s",
@@ -983,10 +1017,16 @@ func createBoundSetPredicate(op Operation, term
BoundTerm, lits Set[Literal]) (B
typedSet := make(literalSet, lits.Len())
for _, v := range lits.Members() {
+ if err := checkNotNaN(op, term, v); err != nil {
+ return nil, err
+ }
casted, err := v.To(boundType)
if err != nil {
return nil, err
}
+ if err := checkNotNaN(op, term, casted); err != nil {
+ return nil, err
+ }
switch casted.(type) {
case AboveMaxLiteral, BelowMinLiteral:
continue
diff --git a/exprs_test.go b/exprs_test.go
index dff135bb7..d381b015b 100644
--- a/exprs_test.go
+++ b/exprs_test.go
@@ -1063,6 +1063,68 @@ func TestBindAboveBelowIntMax(t *testing.T) {
})
}
+func TestBindRejectsNaNLiterals(t *testing.T) {
+ dec, err := iceberg.DecimalTypeOf(9, 2)
+ require.NoError(t, err)
+ schema := iceberg.NewSchema(1,
+ iceberg.NestedField{ID: 1, Name: "f32", Type:
iceberg.PrimitiveTypes.Float32},
+ iceberg.NestedField{ID: 2, Name: "f64", Type:
iceberg.PrimitiveTypes.Float64},
+ iceberg.NestedField{ID: 3, Name: "dec", Type: dec},
+ iceberg.NestedField{ID: 4, Name: "i", Type:
iceberg.PrimitiveTypes.Int32},
+ )
+ f32NaN := iceberg.NewLiteral(float32(math.NaN()))
+ f64NaN := iceberg.NewLiteral(math.NaN())
+ one := iceberg.NewLiteral(1.0)
+
+ var exprs []iceberg.BooleanExpression
+ for _, col := range []string{"f32", "f64"} {
+ ref := iceberg.Reference(col)
+ for _, op := range []iceberg.Operation{
+ iceberg.OpEQ, iceberg.OpNEQ, iceberg.OpLT,
iceberg.OpLTEQ, iceberg.OpGT, iceberg.OpGTEQ,
+ } {
+ exprs = append(exprs,
+ iceberg.LiteralPredicate(op, ref, f32NaN),
+ iceberg.LiteralPredicate(op, ref, f64NaN))
+ }
+ for _, op := range []iceberg.Operation{iceberg.OpIn,
iceberg.OpNotIn} {
+ exprs = append(exprs,
+ iceberg.SetPredicate(op, ref,
[]iceberg.Literal{one, f64NaN}),
+ iceberg.SetPredicate(op, ref,
[]iceberg.Literal{one, f32NaN}))
+ }
+ // "NaN" parses to a NaN float only when the literal is
converted.
+ exprs = append(exprs,
+ iceberg.EqualTo(ref, "NaN"),
+ iceberg.IsIn(ref, "1", "NaN"),
+ iceberg.NewNot(iceberg.EqualTo(ref, math.NaN())))
+ }
+ // A NaN converted to a decimal would otherwise bind as zero, and to an
+ // int it would fail with a cast error that gives no hint.
+ exprs = append(exprs,
+ iceberg.EqualTo(iceberg.Reference("dec"), math.NaN()),
+ iceberg.IsIn(iceberg.Reference("dec"), 1.0, math.NaN()),
+ iceberg.EqualTo(iceberg.Reference("i"), math.NaN()))
+
+ for _, expr := range exprs {
+ t.Run(expr.String(), func(t *testing.T) {
+ _, err := iceberg.BindExpr(schema, expr, true)
+ require.ErrorIs(t, err, iceberg.ErrInvalidArgument)
+ assert.ErrorContains(t, err, "use IsNaN or NotNaN
instead")
+ })
+ }
+
+ t.Run("IsNaN, NotNaN and infinities still bind", func(t *testing.T) {
+ for _, expr := range []iceberg.BooleanExpression{
+ iceberg.IsNaN(iceberg.Reference("f64")),
+ iceberg.NotNaN(iceberg.Reference("f32")),
+ iceberg.EqualTo(iceberg.Reference("f64"), math.Inf(1)),
+ iceberg.IsIn(iceberg.Reference("f32"),
float32(math.Inf(-1)), float32(1)),
+ } {
+ _, err := iceberg.BindExpr(schema, expr, true)
+ assert.NoError(t, err, expr.String())
+ }
+ })
+}
+
func TestBindOutOfRangeDate(t *testing.T) {
sc := iceberg.NewSchema(1,
iceberg.NestedField{ID: 1, Name: "d", Type:
iceberg.PrimitiveTypes.Date},
diff --git a/table/conflict_validation.go b/table/conflict_validation.go
index 2343a3f7b..207b645ff 100644
--- a/table/conflict_validation.go
+++ b/table/conflict_validation.go
@@ -1057,8 +1057,18 @@ func eqDeletePartitionsToFilter(files
[]iceberg.DataFile, meta Metadata) (iceber
return nil, fmt.Errorf("partition field %q:
%w", sourceField.name, err)
}
- // Keep values needing a cast on the equality path,
including
- // strings that can cast to NaN floats during binding.
+ // x = NaN is never true and Bind rejects a NaN
literal, so a NaN
+ // partition value matches through IsNaN, as in
+ // internal.BuildPartitionMatchPredicate. The value is
checked after
+ // conversion to the source type, which also catches a
string that
+ // casts to NaN.
+ if isNaNPartitionLiteral(lit, sourceField.fieldType) {
+ conjuncts = append(conjuncts,
iceberg.IsNaN(iceberg.Reference(sourceField.name)))
+
+ continue
+ }
+
+ // Keep values needing a cast on the equality path.
if len(fieldIDs) == 1 &&
equalityDeleteLiteralCanUseSet(lit, sourceField.fieldType) {
key := equalityDeleteIdentityGroupKey{specID:
specID, fieldID: partFieldID}
group := identityGroups[key]
@@ -1287,6 +1297,24 @@ var filePathFieldID = func() int {
return f.ID
}()
+// isNaNPartitionLiteral reports whether a partition literal is a
floating-point
+// NaN once converted to the source field's type. A literal that cannot be
+// converted is left for Bind to reject.
+func isNaNPartitionLiteral(lit iceberg.Literal, fieldType iceberg.Type) bool {
+ converted, err := lit.To(fieldType)
+ if err != nil {
+ return false
+ }
+ switch v := converted.(type) {
+ case iceberg.Float32Literal:
+ return math.IsNaN(float64(v))
+ case iceberg.Float64Literal:
+ return math.IsNaN(float64(v))
+ }
+
+ return false
+}
+
// canonicalPartitionKey returns a deterministic, injective string key for a
// (specID, partition) tuple. Partition values are compared post-transform
// (direct equality), matching Java's DeleteFileIndex — no transform-aware
diff --git a/table/partition_conflict_test.go b/table/partition_conflict_test.go
index 3b344cb18..62919cf7d 100644
--- a/table/partition_conflict_test.go
+++ b/table/partition_conflict_test.go
@@ -445,6 +445,73 @@ func TestRowDeltaValidate_NullIdentityPartitionRejected(t
*testing.T) {
assert.ErrorIs(t, err, ErrConflictingDataFiles)
}
+// ---------------------------------------------------------------------------
+// NaN identity partition: Bind rejects x = NaN, so the filter uses IsNaN
+// ---------------------------------------------------------------------------
+
+func TestEqDeletePartitionsToFilterUsesIsNaNForNaNPartition(t *testing.T) {
+ schema := iceberg.NewSchema(0,
+ iceberg.NestedField{ID: 1, Name: "id", Type:
iceberg.PrimitiveTypes.Int64, Required: true},
+ iceberg.NestedField{ID: 2, Name: "score", Type:
iceberg.PrimitiveTypes.Float64},
+ )
+ meta := partitionedConflictMeta(t, schema, 2, nil)
+ spec := meta.PartitionSpec()
+ fieldID := spec.Field(0).FieldID
+
+ filter, err := eqDeletePartitionsToFilter([]iceberg.DataFile{
+ equalityDeleteTestFile(t, spec, "eq-nan.parquet",
map[int]any{fieldID: math.NaN()}),
+ }, meta)
+ require.NoError(t, err)
+
+ expected := iceberg.IsNaN(iceberg.Reference("score"))
+ assert.True(t, filter.Equals(expected), "got %s, want %s", filter,
expected)
+ _, err = iceberg.BindExpr(schema, filter, true)
+ require.NoError(t, err)
+}
+
+func checkNaNPartitionEqDeleteConflict(t *testing.T, concurrentValue float64)
error {
+ t.Helper()
+ dir := filepath.ToSlash(t.TempDir())
+
+ schema := iceberg.NewSchema(0,
+ iceberg.NestedField{ID: 1, Name: "id", Type:
iceberg.PrimitiveTypes.Int64, Required: true},
+ iceberg.NestedField{ID: 2, Name: "score", Type:
iceberg.PrimitiveTypes.Float64},
+ )
+
+ writerBaseID := int64(259)
+ baseMeta := partitionedConflictMeta(t, schema, 2, &writerBaseID)
+ concSnapshotID := int64(260)
+
+ spec := baseMeta.PartitionSpec()
+ fieldID := spec.Field(0).FieldID
+
+ mf := writeTestManifest(t, dir, spec, schema, concSnapshotID,
+ map[int]any{fieldID: concurrentValue},
dir+"/score-data.parquet")
+ listPath := writeTestManifestList(t, dir, concSnapshotID,
[]iceberg.ManifestFile{mf})
+
+ ctx := buildPartitionedContext(t, baseMeta, listPath, writerBaseID,
concSnapshotID)
+ require.Len(t, ctx.concurrent, 1)
+
+ eqDf, err := iceberg.NewDataFileBuilder(
+ spec, iceberg.EntryContentEqDeletes,
+ dir+"/nan-score-eq-del.parquet", iceberg.ParquetFile,
+ map[int]any{fieldID: math.NaN()}, nil, nil, 1, 1024,
+ )
+ require.NoError(t, err)
+
+ return validateNoConflictingDataFilesInPartitions(ctx,
[]iceberg.DataFile{eqDf.Build()}, IsolationSerializable)
+}
+
+func TestRowDeltaValidate_NaNIdentityPartitionRejected(t *testing.T) {
+ err := checkNaNPartitionEqDeleteConflict(t, math.NaN())
+ require.Error(t, err)
+ assert.ErrorIs(t, err, ErrConflictingDataFiles)
+}
+
+func TestRowDeltaValidate_NaNIdentityPartitionDifferentAllowed(t *testing.T) {
+ require.NoError(t, checkNaNPartitionEqDeleteConflict(t, 1.5))
+}
+
// ---------------------------------------------------------------------------
// UUID partition column: same UUID rejected, different UUID allowed
// ---------------------------------------------------------------------------
@@ -898,8 +965,8 @@ func
TestEqDeleteConflictLargeSparseSetPreservesMetricsPruning(t *testing.T) {
}
}
-// NaN equality must remain an equality predicate: Go map lookup cannot find
-// a NaN key, so placing it in an IN set can miss a conflicting partition.
+// A NaN partition must stay out of the IN set, because Go map lookup cannot
+// find a NaN key, and match through IsNaN, because Bind rejects x = NaN.
func TestEqDeleteConflictNaNPartition(t *testing.T) {
schema := iceberg.NewSchema(0, iceberg.NestedField{ID: 1, Name:
"value", Type: iceberg.PrimitiveTypes.Float64})
meta := partitionedConflictMeta(t, schema, 1, nil)
@@ -977,9 +1044,9 @@ func
TestEqDeleteConflictCastValuePreservesEqualityPredicate(t *testing.T) {
iceberg.SetPredicate(iceberg.OpIn, iceberg.Reference("value"),
[]iceberg.Literal{
iceberg.Float64Literal(1), iceberg.Float64Literal(2),
}),
- iceberg.LiteralPredicate(iceberg.OpEQ,
iceberg.Reference("value"), iceberg.StringLiteral("NaN")),
+ iceberg.IsNaN(iceberg.Reference("value")),
)
- require.True(t, filter.Equals(want), "cast values must retain
individual equality predicates")
+ require.True(t, filter.Equals(want), "got %s: cast values must stay out
of the IN set, and a cast to NaN must use IsNaN", filter)
_, err = iceberg.BindExpr(meta.CurrentSchema(), filter, true)
require.NoError(t, err)
}
diff --git a/table/partition_residual_null_test.go
b/table/partition_residual_null_test.go
index 42003c9c7..aa5feb0ac 100644
--- a/table/partition_residual_null_test.go
+++ b/table/partition_residual_null_test.go
@@ -41,8 +41,6 @@ func TestPartitionResidualPreservesArrowNaNFiltering(t
*testing.T) {
}{
{"null", nil, "null", iceberg.Float64Literal(5)},
{"nan partition", math.NaN(), `"NaN"`,
iceberg.Float64Literal(5)},
- {"nan literal", float64(5), "5",
iceberg.Float64Literal(math.NaN())},
- {"nan partition and literal", math.NaN(), `"NaN"`,
iceberg.Float64Literal(math.NaN())},
} {
t.Run(tt.name, func(t *testing.T) {
checkPartitionResidualArrowFiltering(t,
iceberg.PrimitiveTypes.Float64, tt.value, tt.json,
diff --git a/website/src/releases.md b/website/src/releases.md
index f7837145c..5e1518b31 100644
--- a/website/src/releases.md
+++ b/website/src/releases.md
@@ -42,6 +42,7 @@ Per-release notes (highlights, breaking changes,
contributors) are published on
### Unreleased compatibility notes
+- **Predicates reject NaN literals when bound.** `EqualTo`, `NotEqualTo`,
`LessThan`, `LessThanEqual`, `GreaterThan`, `GreaterThanEqual`, `IsIn`, `NotIn`
and `LiteralPredicate`/`SetPredicate` with a NaN float literal now fail in
`Bind` with `ErrInvalidArgument`, as Java does. That includes the string
`"NaN"` on a float column and a NaN on a decimal column, which used to bind as
zero. Scans, `ExpressionEvaluator` and the metrics evaluators all bind first,
so they return the error too. Use [...]
- **SQL view APIs align with `catalog.ViewCatalog`.** `CreateView` now takes a
`*view.Version`, schema and options, and returns `*view.View`. `LoadView` now
returns `*view.View`; use `Metadata()` to access its metadata. If the caller
leaves `DefaultCatalog` empty, SQL still sets it to the catalog name.
- **`LocationProvider.NewDataLocation` takes the partition path.** The method
is now `NewDataLocation(dataFileName, partitionPath string)`, matching Iceberg
Java and PyIceberg, so custom `LocationProvider` implementations must add the
argument. Callers pass the partition path there, or `""` for an unpartitioned
file, instead of prefixing the file name.
- **Partitioned data files follow the table's location provider.** With
`write.object-storage.enabled=true` and the default
`write.object-storage.partitioned-paths=true`, new partitioned files land under
`data/<hash>/<partition>/` instead of `data/<partition>/<hash>/`, which can
affect prefix-based lifecycle rules and bucket policies.
`write.object-storage.partitioned-paths=false` and `write.data.path` now apply
to partitioned writes. Partitioned paths also no longer percent-encode the t
[...]
diff --git a/website/src/row-filter-syntax.md b/website/src/row-filter-syntax.md
index 83857ce98..6e0fc6b64 100644
--- a/website/src/row-filter-syntax.md
+++ b/website/src/row-filter-syntax.md
@@ -77,7 +77,7 @@ iceberg.IsNaN(iceberg.Reference("ratio"))
iceberg.NotNaN(iceberg.Reference("ratio"))
```
-Operators `OpIsNan` and `OpNotNan`. Use these instead of `EqualTo(...,
math.NaN())` - NaN is never equal to itself.
+Operators `OpIsNan` and `OpNotNan`. Use these instead of `EqualTo(...,
math.NaN())` - NaN is never equal to itself, so a comparison or `In`/`NotIn`
with a NaN literal fails to bind with `ErrInvalidArgument`.
## String prefix
