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 5bd08d6d7 fix: make In and NotIn NULL for a NULL value in the row
filter (#2356)
5bd08d6d7 is described below
commit 5bd08d6d700f4a0c8dfafb8335326cf3dd88c183
Author: Revanth <[email protected]>
AuthorDate: Fri Oct 30 13:50:16 2026 -0500
fix: make In and NotIn NULL for a NULL value in the row filter (#2356)
* fix: make In and NotIn NULL for a NULL value in the row filter
* docs: link the Java issue and note the NULL semantics change
* docs: name NOT(NotIn) in the NULL semantics release note
---
table/in_null_rows_test.go | 211 ++++++++++++++++++++++++++++++++++++++
table/substrait/substrait.go | 22 ++--
table/substrait/substrait_test.go | 6 +-
table/transaction_delete.go | 7 +-
visitors.go | 50 ++++++++-
website/src/releases.md | 1 +
6 files changed, 278 insertions(+), 19 deletions(-)
diff --git a/table/in_null_rows_test.go b/table/in_null_rows_test.go
new file mode 100644
index 000000000..625451b76
--- /dev/null
+++ b/table/in_null_rows_test.go
@@ -0,0 +1,211 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package table_test
+
+import (
+ "context"
+ "path/filepath"
+ "slices"
+ "testing"
+
+ "github.com/apache/arrow-go/v18/arrow"
+ "github.com/apache/arrow-go/v18/arrow/array"
+ "github.com/apache/arrow-go/v18/arrow/memory"
+ "github.com/apache/iceberg-go"
+ iceio "github.com/apache/iceberg-go/io"
+ "github.com/apache/iceberg-go/table"
+ "github.com/stretchr/testify/require"
+)
+
+// In and NotIn are NULL for a NULL value, as Equal and NotEqual are, so a
+// NULL row matches neither, also under NOT and whatever the number of values.
+// Scans and both delete modes must agree.
+// https://github.com/apache/iceberg-go/issues/2132
+func TestInAndNotInNeverMatchNull(t *testing.T) {
+ age := iceberg.Reference("age")
+
+ // ids 1 to 4
+ ages := []int64{25, 30, 0, 40}
+ valid := []bool{true, true, false, true}
+
+ cases := []struct {
+ name string
+ filter iceberg.BooleanExpression
+ want []int64
+ }{
+ {"not in", iceberg.NotIn(age, int64(30), int64(40)),
[]int64{1}},
+ // binding reduces the set to one value and the predicate to
NotEqual
+ {"not in with duplicate values", iceberg.NotIn(age, int64(30),
int64(30)), []int64{1, 4}},
+ {"not equal", iceberg.NotEqualTo(age, int64(30)), []int64{1,
4}},
+ {"in", iceberg.IsIn(age, int64(30), int64(40)), []int64{2, 4}},
+ {"not of in", iceberg.NewNot(iceberg.IsIn(age, int64(30),
int64(40))), []int64{1}},
+ {"not of not in", iceberg.NewNot(iceberg.NotIn(age, int64(30),
int64(40))), []int64{2, 4}},
+ {"not of and", iceberg.NewNot(iceberg.NewAnd(iceberg.IsIn(age,
int64(30), int64(40)), iceberg.GreaterThan(iceberg.Reference("id"),
int64(0)))), []int64{1}},
+ {"is null or not in", iceberg.NewOr(iceberg.IsNull(age),
iceberg.NotIn(age, int64(30), int64(40))), []int64{1, 3}},
+ }
+
+ for _, partitioned := range []bool{false, true} {
+ for _, mode := range []string{table.WriteModeCopyOnWrite,
table.WriteModeMergeOnRead} {
+ for _, c := range cases {
+ name := mode + "/" + c.name
+ if partitioned {
+ name = "partitioned/" + name
+ }
+ t.Run(name, func(t *testing.T) {
+ tbl := newNullAgeTable(t, mode,
partitioned, ages, valid)
+ requireNullRowsScanAndDelete(t, tbl,
c.filter, c.want, len(ages))
+ })
+ }
+ }
+ }
+}
+
+// A column added after a file was written reads as NULL for that file's rows,
+// and a filter must treat them like a stored NULL, also under NOT. Ids 1 to 4
+// are in a file without x, id 5 has a stored NULL and id 6 has x = 5.
+func TestMissingColumnMatchesLikeStoredNull(t *testing.T) {
+ x, id := iceberg.Reference("x"), iceberg.Reference("id")
+
+ cases := []struct {
+ name string
+ filter iceberg.BooleanExpression
+ want []int64
+ }{
+ {"not of in", iceberg.NewNot(iceberg.IsIn(x, int64(5),
int64(6))), []int64{}},
+ {"not of not in", iceberg.NewNot(iceberg.NotIn(x, int64(5),
int64(6))), []int64{6}},
+ {"not in", iceberg.NotIn(x, int64(7), int64(8)), []int64{6}},
+ {"not of equal", iceberg.NewNot(iceberg.EqualTo(x, int64(7))),
[]int64{6}},
+ {"not of less than", iceberg.NewNot(iceberg.LessThan(x,
int64(1))), []int64{6}},
+ {"not of and", iceberg.NewNot(iceberg.NewAnd(iceberg.IsIn(x,
int64(5), int64(6)), iceberg.GreaterThan(id, int64(0)))), []int64{}},
+ {"not of or", iceberg.NewNot(iceberg.NewOr(iceberg.EqualTo(x,
int64(7)), iceberg.LessThan(id, int64(3)))), []int64{6}},
+ {"is null", iceberg.IsNull(x), []int64{1, 2, 3, 4, 5}},
+ {"not of is null", iceberg.NewNot(iceberg.IsNull(x)),
[]int64{6}},
+ {"not of not null", iceberg.NewNot(iceberg.NotNull(x)),
[]int64{1, 2, 3, 4, 5}},
+ }
+
+ for _, mode := range []string{table.WriteModeCopyOnWrite,
table.WriteModeMergeOnRead} {
+ for _, c := range cases {
+ t.Run(mode+"/"+c.name, func(t *testing.T) {
+ ctx := context.Background()
+ // ids 1 to 4, written before x exists
+ tbl := newCopyOnWriteNullRowsTable(t, "2")
+
+ txn := tbl.NewTransaction()
+ require.NoError(t,
txn.SetProperties(iceberg.Properties{table.WriteDeleteModeKey: mode}))
+ require.NoError(t, txn.UpdateSchema(true,
false).
+ AddColumn([]string{"x"},
iceberg.PrimitiveTypes.Int64, "", false, nil).Commit())
+ tbl, err := txn.Commit(ctx)
+ require.NoError(t, err)
+
+ arrowSchema, err :=
table.SchemaToArrowSchema(tbl.Schema(), nil, false, false)
+ require.NoError(t, err)
+ bldr :=
array.NewRecordBuilder(memory.DefaultAllocator, arrowSchema)
+ defer bldr.Release()
+
bldr.Field(0).(*array.Int64Builder).AppendValues([]int64{5, 6}, nil)
+
bldr.Field(1).(*array.Int64Builder).AppendValues([]int64{50, 60}, nil)
+
bldr.Field(2).(*array.StringBuilder).AppendValues([]string{"e", "f"}, nil)
+
bldr.Field(3).(*array.Int64Builder).AppendValues([]int64{0, 5}, []bool{false,
true})
+ rec := bldr.NewRecordBatch()
+ defer rec.Release()
+
+ rdr, err := array.NewRecordReader(rec.Schema(),
[]arrow.RecordBatch{rec})
+ require.NoError(t, err)
+ defer rdr.Release()
+
+ tbl, err = tbl.Append(ctx, rdr, nil)
+ require.NoError(t, err)
+
+ requireNullRowsScanAndDelete(t, tbl, c.filter,
c.want, 6)
+ })
+ }
+ }
+}
+
+// newNullAgeTable writes ages with ids from 1, NULL where valid is false. A
+// partitioned table has an identity partition on age, so one file per value.
+func newNullAgeTable(t *testing.T, mode string, partitioned bool, ages
[]int64, valid []bool) *table.Table {
+ t.Helper()
+
+ schema := iceberg.NewSchema(0,
+ iceberg.NestedField{ID: 1, Name: "id", Type:
iceberg.PrimitiveTypes.Int64, Required: true},
+ iceberg.NestedField{ID: 2, Name: "age", Type:
iceberg.PrimitiveTypes.Int64, Required: false},
+ )
+ spec := iceberg.UnpartitionedSpec
+ if partitioned {
+ identity := iceberg.NewPartitionSpec(iceberg.PartitionField{
+ SourceIDs: []int{2}, FieldID: 1000, Name: "age",
Transform: iceberg.IdentityTransform{},
+ })
+ spec = &identity
+ }
+
+ location := filepath.ToSlash(t.TempDir())
+ meta, err := table.NewMetadata(schema, spec, table.UnsortedSortOrder,
location,
+ iceberg.Properties{
+ table.PropertyFormatVersion: "2",
+ table.WriteDeleteModeKey: mode,
+ })
+ require.NoError(t, err)
+
+ metaLoc := location + "/metadata/v1.metadata.json"
+ fsF := func(context.Context) (iceio.IO, error) { return
iceio.LocalFS{}, nil }
+ cat := &concurrentTestCatalog{metadata: meta, location: metaLoc, fsF:
fsF}
+ tbl := table.New(table.Identifier{"db", "null_age"}, meta, metaLoc,
fsF, cat)
+
+ bldr := array.NewRecordBuilder(memory.DefaultAllocator,
arrow.NewSchema([]arrow.Field{
+ {Name: "id", Type: arrow.PrimitiveTypes.Int64},
+ {Name: "age", Type: arrow.PrimitiveTypes.Int64, Nullable: true},
+ }, nil))
+ defer bldr.Release()
+ for i := range ages {
+ bldr.Field(0).(*array.Int64Builder).Append(int64(i + 1))
+ }
+ bldr.Field(1).(*array.Int64Builder).AppendValues(ages, valid)
+ rec := bldr.NewRecordBatch()
+ defer rec.Release()
+
+ rdr, err := array.NewRecordReader(rec.Schema(),
[]arrow.RecordBatch{rec})
+ require.NoError(t, err)
+ defer rdr.Release()
+
+ tbl, err = tbl.Append(context.Background(), rdr, nil)
+ require.NoError(t, err)
+
+ return tbl
+}
+
+// requireNullRowsScanAndDelete checks that a scan with filter returns want,
and
+// that a delete with it removes exactly those rows.
+func requireNullRowsScanAndDelete(t *testing.T, tbl *table.Table, filter
iceberg.BooleanExpression, want []int64, total int) {
+ t.Helper()
+ ctx := context.Background()
+
+ matched := scanIDsMatching(t, tbl, filter)
+ slices.Sort(matched)
+ require.Equal(t, want, matched)
+
+ txn := tbl.NewTransaction()
+ require.NoError(t, txn.Delete(ctx, filter, nil))
+ tbl, err := txn.Commit(ctx)
+ require.NoError(t, err)
+
+ remaining := scanIDs(t, tbl)
+ for _, id := range remaining {
+ require.NotContains(t, want, id)
+ }
+ require.Len(t, remaining, total-len(want))
+}
diff --git a/table/substrait/substrait.go b/table/substrait/substrait.go
index e781f47a3..81aac851f 100644
--- a/table/substrait/substrait.go
+++ b/table/substrait/substrait.go
@@ -405,17 +405,25 @@ func (t *toSubstraitExpr) makeSetFunc(id extensions.ID,
term iceberg.BoundTerm,
t.bldr.Literal(expr.NewNestedLiteral(val, false)))
}
+// VisitIn is NULL for a NULL term, as SQL IN and Equal are. Arrow's is_in is
+// false for it, so it is or-ed with (is_null(x) AND NULL), which is false for
+// any other value. NotIn negates the result, so it never matches NULL either.
+// Java's Evaluator matches NULL for notIn instead, see
+// https://github.com/apache/iceberg/issues/18701.
func (t *toSubstraitExpr) VisitIn(term iceberg.BoundTerm, lits
iceberg.Set[iceberg.Literal]) expr.Builder {
- return t.makeSetFunc(isInID, term, lits)
+ nullIfNull := t.bldr.ScalarFunc(andID).Args(
+ t.makeRefFunc(isNullID, term).(expr.FuncArgBuilder),
+ t.bldr.Wrap(expr.NewNullLiteral(&types.BooleanType{Nullability:
types.NullabilityNullable}), nil),
+ )
+
+ return t.bldr.ScalarFunc(orID).Args(
+ t.makeSetFunc(isInID, term, lits).(expr.FuncArgBuilder),
+ nullIfNull,
+ )
}
func (t *toSubstraitExpr) VisitNotIn(term iceberg.BoundTerm, lits
iceberg.Set[iceberg.Literal]) expr.Builder {
- notIn := t.bldr.ScalarFunc(notID).Args(t.makeSetFunc(isInID, term,
lits).(expr.FuncArgBuilder))
-
- return t.bldr.ScalarFunc(andID).Args(
- t.makeRefFunc(isNotNullID, term).(expr.FuncArgBuilder),
- notIn,
- )
+ return t.bldr.ScalarFunc(notID).Args(t.VisitIn(term,
lits).(expr.FuncArgBuilder))
}
func (t *toSubstraitExpr) makeRefFunc(id extensions.ID, term
iceberg.BoundTerm) expr.Builder {
diff --git a/table/substrait/substrait_test.go
b/table/substrait/substrait_test.go
index fbc771eea..3152ddf2e 100644
--- a/table/substrait/substrait_test.go
+++ b/table/substrait/substrait_test.go
@@ -167,7 +167,7 @@ func TestExprs(t *testing.T) {
{iceberg.NotStartsWith(ref, "he"), tableSchemaSimple,
"not(starts_with(.field(0) => string?, string(he)) => boolean?) => boolean?"},
{
iceberg.NotIn(ref, "hello", "world"), tableSchemaSimple,
- "and(is_not_null(.field(0) => string?) => boolean,
not(is_in(.field(0) => string?, list<string>([string(hello) string(world)])) =>
boolean?) => boolean?) => boolean?",
+ "not(or(is_in(.field(0) => string?,
list<string>([string(hello) string(world)])) => boolean?, and(is_null(.field(0)
=> string?) => boolean, null(boolean?)) => boolean?) => boolean?) => boolean?",
},
{
iceberg.NewAnd(iceberg.EqualTo(ref, "hello"),
iceberg.IsNull(ref)), tableSchemaSimple,
@@ -239,13 +239,13 @@ func TestNanosecondTimestampLiterals(t *testing.T) {
name: "timestamp in",
fieldType: iceberg.PrimitiveTypes.TimestampNs,
predicate: iceberg.IsIn(iceberg.Reference("ts"),
iceberg.TimestampNano(1), iceberg.TimestampNano(1001)),
- wantExpr: "is_in(.field(0) => precision_timestamp?<9>,
list<precision_timestamp<9>>([precision_timestamp<9>(1970-01-01
00:00:00.000000001) precision_timestamp<9>(1970-01-01 00:00:00.000001001)])) =>
boolean?",
+ wantExpr: "or(is_in(.field(0) =>
precision_timestamp?<9>,
list<precision_timestamp<9>>([precision_timestamp<9>(1970-01-01
00:00:00.000000001) precision_timestamp<9>(1970-01-01 00:00:00.000001001)])) =>
boolean?, and(is_null(.field(0) => precision_timestamp?<9>) => boolean,
null(boolean?)) => boolean?) => boolean?",
},
{
name: "timestamp with timezone not in",
fieldType: iceberg.PrimitiveTypes.TimestampTzNs,
predicate: iceberg.NotIn(iceberg.Reference("ts"),
iceberg.TimestampNano(-1), iceberg.TimestampNano(1001)),
- wantExpr: "and(is_not_null(.field(0) =>
precision_timestamp_tz?<9>) => boolean, not(is_in(.field(0) =>
precision_timestamp_tz?<9>,
list<precision_timestamp_tz<9>>([precision_timestamp_tz<9>(1969-12-31T23:59:59.999999999Z)
precision_timestamp_tz<9>(1970-01-01T00:00:00.000001001Z)])) => boolean?) =>
boolean?) => boolean?",
+ wantExpr: "not(or(is_in(.field(0) =>
precision_timestamp_tz?<9>,
list<precision_timestamp_tz<9>>([precision_timestamp_tz<9>(1969-12-31T23:59:59.999999999Z)
precision_timestamp_tz<9>(1970-01-01T00:00:00.000001001Z)])) => boolean?,
and(is_null(.field(0) => precision_timestamp_tz?<9>) => boolean,
null(boolean?)) => boolean?) => boolean?) => boolean?",
},
{
name: "timestamp greater than",
diff --git a/table/transaction_delete.go b/table/transaction_delete.go
index ccf383295..aae3c0af1 100644
--- a/table/transaction_delete.go
+++ b/table/transaction_delete.go
@@ -691,11 +691,8 @@ func (isNotTrueVisitor) VisitUnbound(pred
iceberg.UnboundPredicate) truthExprs {
negated = iceberg.NewOr(negated, iceberg.IsNaN(pred.Term()))
}
- // Any other predicate is NULL when its term is NULL. Guard In and NotIn
- // too: binding turns a set that ends up with one value into Equal or
- // NotEqual, which are NULL for a NULL term. A scan with a multi-valued
- // NotIn still matches NULL rows, see
- // https://github.com/apache/iceberg-go/issues/2132.
+ // Any other predicate is NULL when its term is NULL, In and NotIn
+ // included, as the row filter evaluates them.
isNull := iceberg.IsNull(pred.Term())
return truthExprs{
diff --git a/visitors.go b/visitors.go
index 4deb926d7..6161c686c 100644
--- a/visitors.go
+++ b/visitors.go
@@ -645,7 +645,8 @@ func (expressionFieldIDs) VisitBound(pred BoundPredicate)
map[int]struct{} {
// TranslateColumnNames converts the names of columns in an expression by
looking up
// the field IDs in the file schema. If columns don't exist they are replaced
with
-// AlwaysFalse or AlwaysTrue depending on the operator.
+// AlwaysFalse or AlwaysTrue depending on the operator and, for a predicate
that
+// is NULL for a null value, on whether it is under NOT.
func TranslateColumnNames(expr BooleanExpression, fileSchema *Schema)
(BooleanExpression, error) {
res, extracts, err := TranslateColumnNamesForScan(expr, fileSchema)
if err != nil {
@@ -679,7 +680,46 @@ func TranslateColumnNamesForScan(expr BooleanExpression,
fileSchema *Schema) (Bo
return nil, nil, err
}
- return res, tr.columns, nil
+ return resolveNullColumns(res, false), tr.columns, nil
+}
+
+// nullColumn stands for a predicate over a column that is null for every row
+// of a file, which is NULL rather than false. resolveNullColumns replaces it
+// before translation returns, and it must not escape
TranslateColumnNamesForScan:
+// Op reports OpFalse, which is only right where no NOT applies.
+type nullColumn struct{}
+
+func (nullColumn) String() string { return "NullColumn()" }
+func (nullColumn) Op() Operation { return OpFalse }
+func (nullColumn) Negate() BooleanExpression { return nullColumn{} }
+func (nullColumn) Equals(other BooleanExpression) bool {
+ _, ok := other.(nullColumn)
+
+ return ok
+}
+
+// resolveNullColumns replaces each nullColumn so that the expression is true
+// for exactly the rows it is true for under three-valued logic, as the row
+// filter evaluates a stored null. AND and OR are monotone, so a NULL can be
+// read as false under an even number of NOTs and as true under an odd number:
+// NOT(x = 5) must stay NULL, not become true.
+func resolveNullColumns(expr BooleanExpression, negated bool)
BooleanExpression {
+ switch e := expr.(type) {
+ case nullColumn:
+ if negated {
+ return AlwaysTrue{}
+ }
+
+ return AlwaysFalse{}
+ case NotExpr:
+ return NewNot(resolveNullColumns(e.child, !negated))
+ case AndExpr:
+ return NewAnd(resolveNullColumns(e.left, negated),
resolveNullColumns(e.right, negated))
+ case OrExpr:
+ return NewOr(resolveNullColumns(e.left, negated),
resolveNullColumns(e.right, negated))
+ }
+
+ return expr
}
type scanTranslator struct {
@@ -716,13 +756,15 @@ func (*scanTranslator) VisitUnbound(pred
UnboundPredicate) BooleanExpression {
// nullColumnResult folds a predicate over a column that is null for every row
// of a file. IsNull and NotNaN match a null, as they do for a stored null in
-// the row filter; every other predicate does not.
+// the row filter, and NotNull and IsNaN don't. Every other predicate is NULL.
func nullColumnResult(op Operation) BooleanExpression {
switch op {
case OpIsNull, OpNotNan:
return AlwaysTrue{}
- default:
+ case OpNotNull, OpIsNan:
return AlwaysFalse{}
+ default:
+ return nullColumn{}
}
}
diff --git a/website/src/releases.md b/website/src/releases.md
index 5041e88e4..598ff1136 100644
--- a/website/src/releases.md
+++ b/website/src/releases.md
@@ -93,6 +93,7 @@ Per-release notes (highlights, breaking changes,
contributors) are published on
- **Row-lineage column predicate renamed.** Use `iceberg.IsRowLineageColumn`
instead of `iceberg.IsMetadataColumn` to identify `_row_id` and
`_last_updated_sequence_number`. Use `iceberg.IsReservedFieldID` to check the
full reserved metadata field ID range.
- **File writes and directory walks now take an operation context.** Update
`WriteFileIO` and `ListableIO` implementations and calls to `Create`,
`WriteFile`, and `WalkDir` to pass `context.Context`; cloud writes and listings
use it for cancellation.
+- **In and NotIn follow SQL semantics for NULL.** A multi-valued `NotIn`,
`NOT(In(...))` and `NOT(NotIn(...))` no longer match rows where the column is
NULL, consistent with `NotEqual`. A predicate under NOT on a column missing
from an older data file no longer matches every row of that file; those rows
match as a stored NULL would. Java's `Evaluator` still matches NULL for `notEq`
and `notIn`, see
[apache/iceberg#18701](https://github.com/apache/iceberg/issues/18701).
## Verifying and producing releases
