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 69239ba78 fix: match NotNaN on rows of files written before the column
was added (#2219)
69239ba78 is described below
commit 69239ba788cfe335377227425bebce3e21dc8c54
Author: Revanth <[email protected]>
AuthorDate: Mon Oct 19 15:07:51 2026 -0500
fix: match NotNaN on rows of files written before the column was added
(#2219)
---
table/scanner_internal_test.go | 35 +++++++++++++++++++++++++++++++++++
visitors.go | 28 ++++++++++++++++------------
visitors_test.go | 40 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 91 insertions(+), 12 deletions(-)
diff --git a/table/scanner_internal_test.go b/table/scanner_internal_test.go
index 322ae013f..fa90c0d11 100644
--- a/table/scanner_internal_test.go
+++ b/table/scanner_internal_test.go
@@ -2455,6 +2455,41 @@ func TestArrowScanFiltersMissingColumnInitialDefault(t
*testing.T) {
})
}
+// TestArrowScanFiltersMissingColumnWithoutDefault checks that rows of a file
+// written before an optional column was added read the column as null, so a
+// filter matches them as it would a stored null.
+func TestArrowScanFiltersMissingColumnWithoutDefault(t *testing.T) {
+ tbl := buildV3TableWithRows(t,
`[{"id":1,"data":"a"},{"id":2,"data":"b"}]`)
+
+ txn := tbl.NewTransaction()
+ require.NoError(t, txn.UpdateSchema(true, false).
+ AddColumn([]string{"x"}, iceberg.PrimitiveTypes.Float64, "",
false, nil).
+ Commit())
+ var err error
+ tbl, err = txn.Commit(t.Context())
+ require.NoError(t, err)
+
+ x := iceberg.Reference("x")
+ for _, tt := range []struct {
+ name string
+ filter iceberg.BooleanExpression
+ expected int64
+ }{
+ {"is null", iceberg.IsNull(x), 2},
+ {"not null", iceberg.NotNull(x), 0},
+ {"is nan", iceberg.IsNaN(x), 0},
+ {"not nan", iceberg.NotNaN(x), 2},
+ {"not is nan", iceberg.NewNot(iceberg.IsNaN(x)), 2},
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ result, err :=
tbl.Scan(WithRowFilter(tt.filter)).ToArrowTable(t.Context())
+ require.NoError(t, err)
+ defer result.Release()
+ assert.Equal(t, tt.expected, result.NumRows())
+ })
+ }
+}
+
func TestPlanDataManifestTasksHandlesInaccurateCounts(t *testing.T) {
for _, counts := range [][]int32{{5, 5, 5}, {0, 2, 1}, {-1, -1, -1}} {
scan, schema, manifests, posIndex, dvIndex, eqIndex :=
newPlanTasksBenchmarkFixture(t, 3, 3)
diff --git a/visitors.go b/visitors.go
index da3c266d9..4deb926d7 100644
--- a/visitors.go
+++ b/visitors.go
@@ -714,6 +714,18 @@ func (*scanTranslator) VisitUnbound(pred UnboundPredicate)
BooleanExpression {
panic(fmt.Errorf("%w: expected bound predicate, got: %s",
ErrInvalidArgument, pred.Term()))
}
+// 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.
+func nullColumnResult(op Operation) BooleanExpression {
+ switch op {
+ case OpIsNull, OpNotNan:
+ return AlwaysTrue{}
+ default:
+ return AlwaysFalse{}
+ }
+}
+
func (t *scanTranslator) extractRef(ext BoundExtract) Reference {
key := ext.String()
idx, ok := t.byKey[key]
@@ -802,14 +814,10 @@ func (t *scanTranslator) VisitBound(pred BoundPredicate)
BooleanExpression {
var ref Reference
if ext, ok := pred.Term().(BoundExtract); ok {
// If the variant column is absent from this file, the
extracted path is null
- // for every row: IsNull matches, everything else cannot.
Handle it here rather
- // than fabricating an all-null derived column downstream.
+ // for every row. Handle it here rather than fabricating an
all-null derived
+ // column downstream.
if _, found :=
t.fileSchema.FindColumnName(ext.Ref().Field().ID); !found {
- if pred.Op() == OpIsNull {
- return AlwaysTrue{}
- }
-
- return AlwaysFalse{}
+ return nullColumnResult(pred.Op())
}
ref = t.extractRef(ext)
} else {
@@ -822,11 +830,7 @@ func (t *scanTranslator) VisitBound(pred BoundPredicate)
BooleanExpression {
// its default is not a file-wide constant. Preserve
the existing
// missing-column behavior until translation has
row-level parent state.
if field.InitialDefault == nil ||
len(pred.Ref().PosPath()) > 1 {
- if pred.Op() == OpIsNull {
- return AlwaysTrue{}
- }
-
- return AlwaysFalse{}
+ return nullColumnResult(pred.Op())
}
// The spec requires geo defaults to be null, but fail
open for metadata
// written by non-conforming V3 writers rather than
aborting the scan.
diff --git a/visitors_test.go b/visitors_test.go
index 7379adb99..e7d11044e 100644
--- a/visitors_test.go
+++ b/visitors_test.go
@@ -604,6 +604,46 @@ func
TestTranslateColumnNamesNestedInitialDefaultDoesNotAssumeParentPresent(t *t
}
}
+// TestTranslateColumnNamesMissingColumnIsNull checks that a predicate over a
+// column absent from the file, with no initial default, folds as it would for
+// a stored null: IsNull and NotNaN match, every other predicate does not.
+func TestTranslateColumnNamesMissingColumnIsNull(t *testing.T) {
+ currentSchema := iceberg.NewSchema(1,
+ iceberg.NestedField{ID: 1, Name: "id", Type:
iceberg.PrimitiveTypes.Int64, Required: true},
+ iceberg.NestedField{ID: 2, Name: "x", Type:
iceberg.PrimitiveTypes.Float64},
+ iceberg.NestedField{ID: 3, Name: "payload", Type:
iceberg.VariantType{}},
+ )
+ fileSchema := iceberg.NewSchema(0,
+ iceberg.NestedField{ID: 1, Name: "id", Type:
iceberg.PrimitiveTypes.Int64, Required: true},
+ )
+ x := iceberg.Reference("x")
+ extracted := iceberg.Extract("payload", "$.v",
iceberg.PrimitiveTypes.Float64)
+
+ for _, tt := range []struct {
+ name string
+ expr iceberg.BooleanExpression
+ expected iceberg.BooleanExpression
+ }{
+ {"is null", iceberg.IsNull(x), iceberg.AlwaysTrue{}},
+ {"not null", iceberg.NotNull(x), iceberg.AlwaysFalse{}},
+ {"is nan", iceberg.IsNaN(x), iceberg.AlwaysFalse{}},
+ {"not nan", iceberg.NotNaN(x), iceberg.AlwaysTrue{}},
+ {"equal", iceberg.EqualTo(x, 1.0), iceberg.AlwaysFalse{}},
+ {"variant is null", iceberg.IsNull(extracted),
iceberg.AlwaysTrue{}},
+ {"variant is nan", iceberg.IsNaN(extracted),
iceberg.AlwaysFalse{}},
+ {"variant not nan", iceberg.NotNaN(extracted),
iceberg.AlwaysTrue{}},
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ bound, err := iceberg.BindExpr(currentSchema, tt.expr,
true)
+ require.NoError(t, err)
+
+ translated, err := iceberg.TranslateColumnNames(bound,
fileSchema)
+ require.NoError(t, err)
+ assert.Equal(t, tt.expected, translated)
+ })
+ }
+}
+
func TestBoundBoolExprVisitor(t *testing.T) {
tests := []struct {
expr iceberg.BooleanExpression
