This is an automated email from the ASF dual-hosted git repository.
laskoviymishka 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 59a02254a feat(expr): bbox predicate evaluation for geo pruning (#1597)
59a02254a is described below
commit 59a02254a25d74e041a0b26b795cb58b9cd8fb29
Author: Tanmay Rauth <[email protected]>
AuthorDate: Wed Jul 29 14:32:22 2026 -0700
feat(expr): bbox predicate evaluation for geo pruning (#1597)
Summary
Adds geospatial BBoxIntersects / BBoxNotIntersects predicates and wires
them through the bound-expression visitor so data files whose geometry
bounds cannot overlap a query bounding box are pruned during scan
planning. Reuses the single-value geo bound codec from #993.
What changed
- New predicate (exprs.go) — BoundingBox (planar XY box with
Valid/Equals) and the BBoxIntersects(term, bbox) constructor producing
an UnboundPredicate. Binding rejects non-geo columns (ErrType) and
transform terms; the constructor panics on a nil term or an invalid box
(NaN / inverted min-max) to prevent silent mis-pruning. Adds
OpBBoxIntersects / OpBBoxNotIntersects (kept after the boolean ops so
existing op-group ranges are undisturbed) with negation and String()
support.
- Visitor interface (visitors.go) — adds VisitBBoxIntersects /
VisitBBoxNotIntersects to BoundBooleanExprVisitor, dispatches them in
VisitBoundPredicate, and collapses bbox predicates to AlwaysTrue in the
sanitize visitor (they carry query-box coordinates, not user literals,
and have no REST JSON form). The row-level exprEvaluator conservatively
keeps every row — full ST_Intersects remains a query-engine
concern.
- Data-file pruning (table/evaluators.go) —
inclusiveMetricsEval.VisitBBoxIntersects prunes files whose decoded
geometry bounds are disjoint from the query box (all-null files also
prune). Geography columns are never pruned (geodesic bounds may cross
the antimeridian). BBoxNotIntersects, strict eval, manifest eval, and
the bloom collector all conservatively keep files, since bounds alone
can't answer them.
- Codec helpers (table/internal/geo_codec.go) — GeoBoundsXY decodes
lower/upper bounds to planar extents (fails safe on
missing/malformed/NaN), and BBoxIntersectsXY does the closed-interval
overlap test.
- Serialization guards — bbox predicates return an error from
MarshalJSON (no REST grammar) and panic in the substrait converter,
since they exist only for local scan planning.
Testing
expr_bbox_test.go, table/evaluators_geo_test.go,
table/internal/geo_codec_internal_test.go, and additions to
visitors_test.go cover construction/validation, binding, pruning
decisions (geometry vs geography, null-only, malformed bounds), and
codec edge cases.
Closes: #994
---
errors.go | 7 +
expr_bbox_test.go | 229 ++++++++++++++++++++++++++
expr_json.go | 7 +
exprs.go | 176 ++++++++++++++++++++
operation_string.go | 6 +-
table/evaluators.go | 81 ++++++++++
table/evaluators_geo_test.go | 259 ++++++++++++++++++++++++++++++
table/internal/geo_codec.go | 36 +++++
table/internal/geo_codec_internal_test.go | 57 +++++++
table/substrait/substrait.go | 15 ++
table/substrait/substrait_test.go | 19 +++
visitors.go | 91 +++++++++++
visitors_test.go | 122 ++++++++++++++
13 files changed, 1103 insertions(+), 2 deletions(-)
diff --git a/errors.go b/errors.go
index 44e64d967..45e03770e 100644
--- a/errors.go
+++ b/errors.go
@@ -35,4 +35,11 @@ var (
ErrBadLiteral = errors.New("invalid literal value")
ErrInvalidBinSerialization = errors.New("invalid binary serialization")
ErrResolve = errors.New("cannot resolve type")
+ // ErrBBoxNotSerializable is returned when marshaling a geospatial bbox
+ // predicate to REST expression JSON: bbox predicates exist only for
local
+ // scan planning and have no representation in the REST expression
grammar.
+ // It wraps ErrNotImplemented (not ErrInvalidArgument) because this is a
+ // contract/not-implemented condition, not a bad caller argument -
matching
+ // how the substrait backstop surfaces the same predicate.
+ ErrBBoxNotSerializable = fmt.Errorf("%w: geospatial bbox predicates
cannot be serialized to REST expression JSON", ErrNotImplemented)
)
diff --git a/expr_bbox_test.go b/expr_bbox_test.go
new file mode 100644
index 000000000..cc18b4dd3
--- /dev/null
+++ b/expr_bbox_test.go
@@ -0,0 +1,229 @@
+// 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 iceberg_test
+
+import (
+ "encoding/json"
+ "math"
+ "testing"
+
+ "github.com/apache/iceberg-go"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+func geoTestSchema() *iceberg.Schema {
+ return iceberg.NewSchema(1,
+ iceberg.NestedField{ID: 1, Name: "geom", Type:
iceberg.GeometryType{}, Required: false},
+ iceberg.NestedField{ID: 2, Name: "geog", Type:
iceberg.GeographyType{}, Required: false},
+ iceberg.NestedField{ID: 3, Name: "num", Type:
iceberg.PrimitiveTypes.Int32, Required: false},
+ )
+}
+
+func TestBBoxIntersectsConstruction(t *testing.T) {
+ bbox := iceberg.BoundingBox{MinX: 0, MinY: 0, MaxX: 10, MaxY: 10}
+ pred := iceberg.BBoxIntersects(iceberg.Reference("geom"), bbox)
+
+ assert.Equal(t, iceberg.OpBBoxIntersects, pred.Op())
+ assert.Equal(t, iceberg.Reference("geom"), pred.Term())
+ assert.Contains(t, pred.String(), "BBoxIntersects")
+
+ // Negation flips the operation and round-trips back.
+ neg := pred.Negate()
+ assert.Equal(t, iceberg.OpBBoxNotIntersects, neg.Op())
+ assert.True(t, pred.Equals(neg.Negate()))
+ assert.False(t, pred.Equals(neg))
+}
+
+func TestBBoxIntersectsNilTermPanics(t *testing.T) {
+ assert.Panics(t, func() {
+ iceberg.BBoxIntersects(nil, iceberg.BoundingBox{})
+ })
+}
+
+func TestBoundingBoxValid(t *testing.T) {
+ nan := math.NaN()
+ inf := math.Inf(1)
+ tests := []struct {
+ name string
+ bbox iceberg.BoundingBox
+ want bool
+ }{
+ {"zero", iceberg.BoundingBox{}, true},
+ {"normal", iceberg.BoundingBox{MinX: 0, MinY: 0, MaxX: 10,
MaxY: 10}, true},
+ {"open half-plane via inf", iceberg.BoundingBox{MinX: -inf,
MinY: -inf, MaxX: inf, MaxY: inf}, true},
+ {"inverted x", iceberg.BoundingBox{MinX: 10, MinY: 0, MaxX: 0,
MaxY: 10}, false},
+ {"inverted y", iceberg.BoundingBox{MinX: 0, MinY: 10, MaxX: 10,
MaxY: 0}, false},
+ {"nan min", iceberg.BoundingBox{MinX: nan, MinY: 0, MaxX: 10,
MaxY: 10}, false},
+ {"nan max", iceberg.BoundingBox{MinX: 0, MinY: 0, MaxX: 10,
MaxY: nan}, false},
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ assert.Equal(t, tt.want, tt.bbox.Valid())
+ })
+ }
+}
+
+func TestBBoxIntersectsInvalidBoxPanics(t *testing.T) {
+ // An inverted or NaN box would silently mis-prune, so construction
rejects it.
+ assert.Panics(t, func() {
+ iceberg.BBoxIntersects(iceberg.Reference("geom"),
+ iceberg.BoundingBox{MinX: 10, MinY: 0, MaxX: 0, MaxY:
10})
+ })
+ assert.Panics(t, func() {
+ iceberg.BBoxIntersects(iceberg.Reference("geom"),
+ iceberg.BoundingBox{MinX: math.NaN(), MinY: 0, MaxX:
10, MaxY: 10})
+ })
+}
+
+func TestBBoxIntersectsBind(t *testing.T) {
+ sc := geoTestSchema()
+ bbox := iceberg.BoundingBox{MinX: 1, MinY: 2, MaxX: 3, MaxY: 4}
+
+ for _, name := range []string{"geom", "geog"} {
+ t.Run(name, func(t *testing.T) {
+ bound, err :=
iceberg.BBoxIntersects(iceberg.Reference(name), bbox).Bind(sc, true)
+ require.NoError(t, err, "binding to %s", name)
+
+ bp, ok := bound.(iceberg.BoundBBoxPredicate)
+ require.True(t, ok)
+ assert.Equal(t, iceberg.OpBBoxIntersects, bp.Op())
+ assert.True(t, bbox.Equals(bp.BBox()))
+ assert.Equal(t, name, bp.Ref().Field().Name)
+ })
+ }
+}
+
+func TestBBoxIntersectsBindNonGeoRejected(t *testing.T) {
+ sc := geoTestSchema()
+ _, err := iceberg.BBoxIntersects(iceberg.Reference("num"),
iceberg.BoundingBox{}).Bind(sc, true)
+ require.Error(t, err)
+ assert.ErrorIs(t, err, iceberg.ErrType)
+}
+
+// TestBBoxIntersectsNotAUnaryPredicate locks in the invariant that a bound
bbox
+// predicate must NOT satisfy BoundUnaryPredicate. If it did (e.g. by
regaining an
+// AsUnbound(Reference) method), every type-switch on BoundUnaryPredicate -
+// column-name translation, transform projection - would silently match it and
+// rebuild it as a generic unary predicate, which reaches substrait (panic) or
+// drops rows on a missing column. Bbox is dispatched via BoundBBoxPredicate
only.
+func TestBBoxIntersectsNotAUnaryPredicate(t *testing.T) {
+ sc := geoTestSchema()
+ bound, err := iceberg.BBoxIntersects(iceberg.Reference("geom"),
+ iceberg.BoundingBox{MinX: 0, MinY: 0, MaxX: 10, MaxY:
10}).Bind(sc, true)
+ require.NoError(t, err)
+
+ _, isBBox := bound.(iceberg.BoundBBoxPredicate)
+ require.True(t, isBBox)
+ _, isUnary := bound.(iceberg.BoundUnaryPredicate)
+ assert.False(t, isUnary, "a bound bbox predicate must not satisfy
BoundUnaryPredicate")
+}
+
+func TestBBoxIntersectsBindUnknownField(t *testing.T) {
+ sc := geoTestSchema()
+ _, err := iceberg.BBoxIntersects(iceberg.Reference("missing"),
iceberg.BoundingBox{}).Bind(sc, true)
+ require.Error(t, err)
+}
+
+// Bound negation preserves the box and can be rewritten through
RewriteNotExpr.
+func TestBBoxNotIntersectsRewrite(t *testing.T) {
+ sc := geoTestSchema()
+ bbox := iceberg.BoundingBox{MinX: 1, MinY: 2, MaxX: 3, MaxY: 4}
+
+ bound, err := iceberg.BBoxIntersects(iceberg.Reference("geom"),
bbox).Bind(sc, true)
+ require.NoError(t, err)
+
+ rewritten, err := iceberg.RewriteNotExpr(iceberg.NewNot(bound))
+ require.NoError(t, err)
+ assert.Equal(t, iceberg.OpBBoxNotIntersects, rewritten.Op())
+}
+
+// Geospatial predicates have no REST JSON representation and must surface an
+// error rather than silently marshalling to an empty object.
+func TestBBoxIntersectsNotJSONSerializable(t *testing.T) {
+ sc := geoTestSchema()
+ unbound := iceberg.BBoxIntersects(iceberg.Reference("geom"),
iceberg.BoundingBox{})
+
+ _, err := json.Marshal(unbound)
+ require.Error(t, err)
+
+ bound, err := unbound.Bind(sc, true)
+ require.NoError(t, err)
+ _, err = json.Marshal(bound)
+ require.Error(t, err)
+}
+
+// TestBBoxTranslateColumnNames guards column-name translation, the single
+// function where a bbox predicate previously went wrong in two directions:
with
+// the geo column present it survived as a BoundUnaryPredicate and later
panicked
+// in substrait, and with the column absent (a file written before the geo
column
+// existed) it collapsed to AlwaysFalse and silently dropped every row. Both
must
+// resolve to AlwaysTrue so the record filter conservatively keeps every row -
+// bbox pruning is the metrics evaluator's job, not the record filter's.
+func TestBBoxTranslateColumnNames(t *testing.T) {
+ sc := geoTestSchema()
+ bound, err := iceberg.BBoxIntersects(iceberg.Reference("geom"),
+ iceberg.BoundingBox{MinX: 0, MinY: 0, MaxX: 10, MaxY:
10}).Bind(sc, true)
+ require.NoError(t, err)
+
+ t.Run("column present", func(t *testing.T) {
+ out, err := iceberg.TranslateColumnNames(bound, sc)
+ require.NoError(t, err)
+ assert.Equal(t, iceberg.AlwaysTrue{}, out)
+ })
+
+ t.Run("column absent (schema evolution)", func(t *testing.T) {
+ // A file schema that predates the geometry column.
+ fileSchema := iceberg.NewSchema(1,
+ iceberg.NestedField{ID: 3, Name: "num", Type:
iceberg.PrimitiveTypes.Int32})
+ out, err := iceberg.TranslateColumnNames(bound, fileSchema)
+ require.NoError(t, err)
+ assert.Equal(t, iceberg.AlwaysTrue{}, out,
+ "a file missing the geo column must keep every row, not
drop them")
+ })
+}
+
+// SanitizeExpression must not fail on a bbox predicate: it has no user
literal to
+// mask and no REST JSON form, so it collapses to always-true while the rest of
+// the expression sanitizes normally. Covers both the unbound path (how a
scan's
+// row filter is stored) and the bound path.
+func TestBBoxIntersectsSanitize(t *testing.T) {
+ sc := geoTestSchema()
+ bbox := iceberg.BoundingBox{MinX: 0, MinY: 0, MaxX: 10, MaxY: 10}
+ unbound := iceberg.BBoxIntersects(iceberg.Reference("geom"), bbox)
+
+ // Unbound path: scan.rowFilter is stored unbound.
+ sanitized, err := iceberg.SanitizeExpression(unbound)
+ require.NoError(t, err)
+ assert.Equal(t, iceberg.AlwaysTrue{}, sanitized)
+
+ // Bound path.
+ bound, err := unbound.Bind(sc, true)
+ require.NoError(t, err)
+ sanitized, err = iceberg.SanitizeExpression(bound)
+ require.NoError(t, err)
+ assert.Equal(t, iceberg.AlwaysTrue{}, sanitized)
+
+ // Within a larger expression, the non-geo conjunct still sanitizes and
the
+ // result serializes to Expression JSON.
+ combined, err := iceberg.SanitizeExpression(iceberg.NewAnd(unbound,
+ iceberg.EqualTo(iceberg.Reference("num"), int32(5))))
+ require.NoError(t, err)
+ _, err = json.Marshal(combined)
+ require.NoError(t, err)
+}
diff --git a/expr_json.go b/expr_json.go
index 5fd5f1bb1..a81b5f4bd 100644
--- a/expr_json.go
+++ b/expr_json.go
@@ -195,6 +195,13 @@ func (bsp *boundSetPredicate[T]) MarshalJSON() ([]byte,
error) {
return marshalSetPredicate(bsp.op, bsp.term, bsp.lits.Members())
}
+// Geospatial bbox predicates have no representation in the REST expression
JSON
+// grammar; they are used only for local scan planning. Report an error rather
+// than silently emitting an empty object. The sentinel lives in errors.go with
+// the other wrapped sentinels (see ErrBBoxNotSerializable).
+func (u *unboundBBoxPredicate) MarshalJSON() ([]byte, error) { return nil,
ErrBBoxNotSerializable }
+func (b *boundBBoxPredicate) MarshalJSON() ([]byte, error) { return nil,
ErrBBoxNotSerializable }
+
// predicateType returns the wire "type" string for a predicate operation.
func predicateType(op Operation) (string, error) {
s, ok := opToJSON[op]
diff --git a/exprs.go b/exprs.go
index 139044929..ed95848a6 100644
--- a/exprs.go
+++ b/exprs.go
@@ -19,6 +19,7 @@ package iceberg
import (
"fmt"
+ "math"
"reflect"
"github.com/apache/arrow-go/v18/parquet/variant"
@@ -59,6 +60,12 @@ const (
OpNot // Not
OpAnd // And
OpOr // Or
+ // geospatial ops. Kept after the boolean ops so the group ranges above
+ // (used for quick op-kind validation) are undisturbed. These have
their own
+ // predicate constructor (BBoxIntersects) rather than belonging to the
+ // unary/literal/set groups.
+ OpBBoxIntersects // BBoxIntersects
+ OpBBoxNotIntersects // BBoxNotIntersects
)
// Negate returns the inverse operation for a given op
@@ -92,6 +99,10 @@ func (op Operation) Negate() Operation {
return OpNotStartsWith
case OpNotStartsWith:
return OpStartsWith
+ case OpBBoxIntersects:
+ return OpBBoxNotIntersects
+ case OpBBoxNotIntersects:
+ return OpBBoxIntersects
default:
panic("no negation for operation " + op.String())
}
@@ -1132,3 +1143,168 @@ func rejectTransformTerm(term BoundTerm) error {
return nil
}
+
+// BoundingBox is a planar (XY) query bounding box used by the geospatial
+// BBoxIntersects predicate. MinX/MinY are the lower-left corner and MaxX/MaxY
+// the upper-right corner (X is longitude/easting, Y is latitude/northing).
+//
+// The box is two-dimensional: pruning only compares the X and Y extents of a
+// geometry column's bounds, which is all the Iceberg spec requires for
+// bbox-based data skipping. Z/M extents in a column's bounds are ignored.
+//
+// Z/M are intentionally omitted for now rather than reserved as fields: XY is
+// sufficient for spec-compliant pruning, and adding optional
MinZ/MaxZ/MinM/MaxM
+// later is source-additive. The forward-compat contract is that Valid and
Equals
+// are defined over whichever extents the box carries - today X and Y - so
adding
+// higher dimensions later extends, rather than reinterprets, existing XY
boxes.
+type BoundingBox struct {
+ MinX, MinY, MaxX, MaxY float64
+}
+
+func (b BoundingBox) String() string {
+ return fmt.Sprintf("BoundingBox(minX=%g, minY=%g, maxX=%g, maxY=%g)",
+ b.MinX, b.MinY, b.MaxX, b.MaxY)
+}
+
+// Equals reports whether two bounding boxes have identical extents.
+func (b BoundingBox) Equals(other BoundingBox) bool {
+ return b.MinX == other.MinX && b.MinY == other.MinY &&
+ b.MaxX == other.MaxX && b.MaxY == other.MaxY
+}
+
+// Valid reports whether the box is well-formed: no coordinate is NaN and the
+// minimum of each axis does not exceed its maximum. An infinite bound is
allowed
+// (an open half-plane). An invalid box would silently mis-prune - a NaN makes
+// every intersection test false (pruning matching files), and an inverted box
+// (min > max) reports the wrong overlap - so BBoxIntersects rejects one.
+func (b BoundingBox) Valid() bool {
+ if math.IsNaN(b.MinX) || math.IsNaN(b.MinY) ||
+ math.IsNaN(b.MaxX) || math.IsNaN(b.MaxY) {
+ return false
+ }
+
+ return b.MinX <= b.MaxX && b.MinY <= b.MaxY
+}
+
+// isGeoType reports whether t is a geometry or geography type, the only types
a
+// bbox predicate may bind to.
+func isGeoType(t Type) bool {
+ switch t.(type) {
+ case GeometryType, GeographyType:
+ return true
+ default:
+ return false
+ }
+}
+
+// BBoxIntersects constructs an unbound geospatial predicate that matches rows
+// whose geometry/geography value's bounding box intersects the query box. It
is
+// used to prune data files whose stored geo bounds cannot overlap the query
+// region; the spec only requires bbox-based pruning, so full geometric
predicate
+// evaluation (ST_Intersects/ST_Within) remains a query-engine concern.
+//
+// Panics if the term is nil or the bbox is not Valid (NaN coordinate or an
+// inverted min/max), either of which would cause silent mis-pruning. Panicking
+// on invalid construction is consistent with the other predicate constructors
in
+// this package (UnaryPredicate, LiteralPredicate, SetPredicate, NewAnd,
NewNot),
+// which all panic rather than return an error; a caller building a box from
+// untrusted input should validate it with BoundingBox.Valid first.
+func BBoxIntersects(t UnboundTerm, bbox BoundingBox) UnboundPredicate {
+ if t == nil {
+ panic(fmt.Errorf("%w: cannot create bbox predicate with nil
term",
+ ErrInvalidArgument))
+ }
+ if !bbox.Valid() {
+ panic(fmt.Errorf("%w: invalid bounding box %s (NaN coordinate
or min > max)",
+ ErrInvalidArgument, bbox))
+ }
+
+ return &unboundBBoxPredicate{op: OpBBoxIntersects, term: t, bbox: bbox}
+}
+
+type unboundBBoxPredicate struct {
+ op Operation
+ term UnboundTerm
+ bbox BoundingBox
+}
+
+func (u *unboundBBoxPredicate) String() string {
+ return fmt.Sprintf("%s(term=%s, bbox=%s)", u.op, u.term, u.bbox)
+}
+
+func (u *unboundBBoxPredicate) Op() Operation { return u.op }
+func (u *unboundBBoxPredicate) Term() UnboundTerm { return u.term }
+func (u *unboundBBoxPredicate) Negate() BooleanExpression {
+ return &unboundBBoxPredicate{op: u.op.Negate(), term: u.term, bbox:
u.bbox}
+}
+
+func (u *unboundBBoxPredicate) Equals(other BooleanExpression) bool {
+ rhs, ok := other.(*unboundBBoxPredicate)
+ if !ok {
+ return false
+ }
+
+ return u.op == rhs.op && u.term.Equals(rhs.term) &&
u.bbox.Equals(rhs.bbox)
+}
+
+func (u *unboundBBoxPredicate) Bind(schema *Schema, caseSensitive bool)
(BooleanExpression, error) {
+ bound, err := u.term.Bind(schema, caseSensitive)
+ if err != nil {
+ return nil, err
+ }
+ if err := rejectTransformTerm(bound); err != nil {
+ return nil, err
+ }
+
+ if !isGeoType(bound.Type()) {
+ return nil, fmt.Errorf("%w: BBoxIntersects must bind to a
geometry or geography type, not %s",
+ ErrType, bound.Type())
+ }
+
+ return &boundBBoxPredicate{op: u.op, term: bound, bbox: u.bbox}, nil
+}
+
+// BoundBBoxPredicate is a bound geospatial predicate that tests whether a
+// geometry/geography column's bounds intersect a query bounding box.
+type BoundBBoxPredicate interface {
+ BoundPredicate
+
+ BBox() BoundingBox
+}
+
+// boundBBoxPredicate carries a bound geo term plus the query box. Unlike the
+// other bound predicates it intentionally has no AsUnbound(Reference) method,
so
+// it does NOT satisfy BoundUnaryPredicate: a bbox predicate must never be
rebuilt
+// as a generic unary predicate (it would reach substrait and error, or be
+// dropped to AlwaysFalse when a column is absent). It has no record-filter or
+// REST-JSON form at all, so the two visitors that would otherwise rebuild a
bound
+// predicate - columnNameTranslator.VisitBound and sanitizeVisitor.VisitBound -
+// special-case *boundBBoxPredicate and collapse it to AlwaysTrue. Data-file
+// pruning is done separately by inclusiveMetricsEval.
+type boundBBoxPredicate struct {
+ op Operation
+ term BoundTerm
+ bbox BoundingBox
+}
+
+func (b *boundBBoxPredicate) String() string {
+ return fmt.Sprintf("Bound%s(term=%s, bbox=%s)", b.op, b.term, b.bbox)
+}
+
+func (b *boundBBoxPredicate) Op() Operation { return b.op }
+func (b *boundBBoxPredicate) Term() BoundTerm { return b.term }
+func (b *boundBBoxPredicate) Ref() BoundReference { return b.term.Ref() }
+func (b *boundBBoxPredicate) BBox() BoundingBox { return b.bbox }
+
+func (b *boundBBoxPredicate) Negate() BooleanExpression {
+ return &boundBBoxPredicate{op: b.op.Negate(), term: b.term, bbox:
b.bbox}
+}
+
+func (b *boundBBoxPredicate) Equals(other BooleanExpression) bool {
+ rhs, ok := other.(*boundBBoxPredicate)
+ if !ok {
+ return false
+ }
+
+ return b.op == rhs.op && b.term.Equals(rhs.term) &&
b.bbox.Equals(rhs.bbox)
+}
diff --git a/operation_string.go b/operation_string.go
index 3af65e3e0..25f6340b3 100644
--- a/operation_string.go
+++ b/operation_string.go
@@ -27,11 +27,13 @@ func _() {
_ = x[OpNot-16]
_ = x[OpAnd-17]
_ = x[OpOr-18]
+ _ = x[OpBBoxIntersects-19]
+ _ = x[OpBBoxNotIntersects-20]
}
-const _Operation_name =
"TrueFalseIsNullNotNullIsNaNNotNaNLessThanLessThanEqualGreaterThanGreaterThanEqualEqualNotEqualStartsWithNotStartsWithInNotInNotAndOr"
+const _Operation_name =
"TrueFalseIsNullNotNullIsNaNNotNaNLessThanLessThanEqualGreaterThanGreaterThanEqualEqualNotEqualStartsWithNotStartsWithInNotInNotAndOrBBoxIntersectsBBoxNotIntersects"
-var _Operation_index = [...]uint8{0, 4, 9, 15, 22, 27, 33, 41, 54, 65, 81, 86,
94, 104, 117, 119, 124, 127, 130, 132}
+var _Operation_index = [...]uint8{0, 4, 9, 15, 22, 27, 33, 41, 54, 65, 81, 86,
94, 104, 117, 119, 124, 127, 130, 132, 146, 163}
func (i Operation) String() string {
if i < 0 || i >= Operation(len(_Operation_index)-1) {
diff --git a/table/evaluators.go b/table/evaluators.go
index e914dde83..03a13dbb8 100644
--- a/table/evaluators.go
+++ b/table/evaluators.go
@@ -530,6 +530,31 @@ func (m *manifestEvalVisitor) VisitNotStartsWith(term
iceberg.BoundTerm, lit ice
return rowsMightMatch
}
+// Manifest (partition-summary) pruning does not apply to geo predicates: no
+// partition transform accepts a geometry/geography source
(Transform.CanTransform
+// rejects them), so a geo column is never a partition field and a bbox
predicate
+// always projects to AlwaysTrue before reaching this evaluator. These methods
+// exist only to satisfy the visitor interface and conservatively keep every
+// manifest. Data-file pruning is handled by inclusiveMetricsEval.
+func (m *manifestEvalVisitor) VisitBBoxIntersects(iceberg.BoundTerm,
iceberg.BoundingBox) bool {
+ return rowsMightMatch
+}
+
+func (m *manifestEvalVisitor) VisitBBoxNotIntersects(iceberg.BoundTerm,
iceberg.BoundingBox) bool {
+ return rowsMightMatch
+}
+
+// These evaluators handle geospatial predicates, so they implement the
optional
+// iceberg.BoundGeospatialExprVisitor. The assertions keep that wiring
+// compile-checked now that the geo methods live on that extension interface
+// rather than on BoundBooleanExprVisitor.
+var (
+ _ iceberg.BoundGeospatialExprVisitor[bool] =
(*manifestEvalVisitor)(nil)
+ _ iceberg.BoundGeospatialExprVisitor[bool] =
(*inclusiveMetricsEval)(nil)
+ _ iceberg.BoundGeospatialExprVisitor[bool] =
(*strictMetricsEval)(nil)
+ _ iceberg.BoundGeospatialExprVisitor[[]internal.RowGroupBloomPred] =
(*bloomPredicateCollector)(nil)
+)
+
func (m *manifestEvalVisitor) VisitTrue() bool {
return rowsMightMatch
}
@@ -1209,6 +1234,43 @@ func (m *inclusiveMetricsEval) VisitNotStartsWith(t
iceberg.BoundTerm, lit icebe
return rowsMightMatch
}
+func (m *inclusiveMetricsEval) VisitBBoxIntersects(t iceberg.BoundTerm, bbox
iceberg.BoundingBox) bool {
+ fieldID := t.Ref().Field().ID
+
+ // If the column is entirely null, no geometry can intersect the query
box.
+ if m.containsNullsOnly(fieldID) {
+ return rowsCannotMatch
+ }
+
+ // Only geometry bounds are safe to prune with a planar min/max compare.
+ // Geography bounds are geodesic and may cross the antimeridian
(lower_x >
+ // upper_x; spec Appendix D), which scalar XY intersection would
mis-handle
+ // and wrongly prune. iceberg-go emits no geography bounds, but files
written
+ // by other engines can, so guard on the column type rather than on
presence.
+ if _, isGeography := t.Ref().Field().Type.(iceberg.GeographyType);
isGeography {
+ return rowsMightMatch
+ }
+
+ // Prune only when both geo bounds are present and decode cleanly. A
missing
+ // or malformed bound leaves the file unprunable, which is always safe.
+ minX, minY, maxX, maxY, ok :=
internal.GeoBoundsXY(m.lowerBounds[fieldID], m.upperBounds[fieldID])
+ if !ok {
+ return rowsMightMatch
+ }
+
+ if internal.BBoxIntersectsXY(minX, minY, maxX, maxY, bbox.MinX,
bbox.MinY, bbox.MaxX, bbox.MaxY) {
+ return rowsMightMatch
+ }
+
+ return rowsCannotMatch
+}
+
+func (m *inclusiveMetricsEval) VisitBBoxNotIntersects(iceberg.BoundTerm,
iceberg.BoundingBox) bool {
+ // A file whose bounds intersect the query box may still hold only
geometries
+ // outside it, so not-intersects cannot be answered from bounds alone.
+ return rowsMightMatch
+}
+
func newStrictMetricsEvaluator(s *iceberg.Schema, expr
iceberg.BooleanExpression,
caseSensitive bool, includeEmptyFiles bool,
) (func(iceberg.DataFile) (bool, error), error) {
@@ -1560,6 +1622,17 @@ func (m *strictMetricsEval)
VisitNotStartsWith(iceberg.BoundTerm, iceberg.Litera
return rowsMightNotMatch
}
+// Strict evaluation asks whether every row must match. Bounds can prove a bbox
+// predicate false for a file (disjoint bounds) but never that every geometry
+// intersects the query box, so neither variant can assert rowsMustMatch.
+func (m *strictMetricsEval) VisitBBoxIntersects(iceberg.BoundTerm,
iceberg.BoundingBox) bool {
+ return rowsMightNotMatch
+}
+
+func (m *strictMetricsEval) VisitBBoxNotIntersects(iceberg.BoundTerm,
iceberg.BoundingBox) bool {
+ return rowsMightNotMatch
+}
+
func (m *strictMetricsEval) mayContainNulls(field iceberg.NestedField) bool {
cnt, exists := m.nullCounts[field.ID]
if !exists {
@@ -1756,6 +1829,14 @@ func (c *bloomPredicateCollector) VisitNotStartsWith(_
iceberg.BoundTerm, _ iceb
return nil
}
+func (c *bloomPredicateCollector) VisitBBoxIntersects(_ iceberg.BoundTerm, _
iceberg.BoundingBox) []internal.RowGroupBloomPred {
+ return nil
+}
+
+func (c *bloomPredicateCollector) VisitBBoxNotIntersects(_ iceberg.BoundTerm,
_ iceberg.BoundingBox) []internal.RowGroupBloomPred {
+ return nil
+}
+
// newBloomFilterPredicates walks expr and returns bloom-filter-checkable
// predicates for row group pruning. Returns nil (no predicates) for
// AlwaysTrue or any expression with no EqualTo/In conjuncts.
diff --git a/table/evaluators_geo_test.go b/table/evaluators_geo_test.go
new file mode 100644
index 000000000..e225c5150
--- /dev/null
+++ b/table/evaluators_geo_test.go
@@ -0,0 +1,259 @@
+// 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
+
+import (
+ "bytes"
+ "context"
+ "encoding/binary"
+ "math"
+ "testing"
+
+ "github.com/apache/iceberg-go"
+ iceio "github.com/apache/iceberg-go/io"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// geoBound2D encodes a single bound point in the Iceberg geospatial
single-value
+// serialization: little-endian float64 X then Y (16 bytes).
+func geoBound2D(x, y float64) []byte {
+ b := make([]byte, 16)
+ binary.LittleEndian.PutUint64(b[0:], math.Float64bits(x))
+ binary.LittleEndian.PutUint64(b[8:], math.Float64bits(y))
+
+ return b
+}
+
+var geoMetricsSchema = iceberg.NewSchema(0,
+ iceberg.NestedField{ID: 1, Name: "geom", Type: iceberg.GeometryType{},
Required: false},
+ iceberg.NestedField{ID: 2, Name: "geog", Type: iceberg.GeographyType{},
Required: false},
+)
+
+// TestInclusiveMetricsBBoxIntersects verifies that a data file is dropped only
+// when its geometry bounds cannot intersect the query bbox.
+func TestInclusiveMetricsBBoxIntersects(t *testing.T) {
+ // A file whose geometry column spans the box [0,0]-[10,10].
+ lower, upper := geoBound2D(0, 0), geoBound2D(10, 10)
+ file := &mockDataFile{
+ count: 2,
+ valueCounts: map[int]int64{1: 2},
+ nullCounts: map[int]int64{1: 0},
+ lowerBounds: map[int][]byte{1: lower},
+ upperBounds: map[int][]byte{1: upper},
+ }
+
+ tests := []struct {
+ name string
+ bbox iceberg.BoundingBox
+ want bool // true => might match (kept), false => pruned
+ }{
+ {"overlapping", iceberg.BoundingBox{MinX: 5, MinY: 5, MaxX: 15,
MaxY: 15}, true},
+ {"contained", iceberg.BoundingBox{MinX: 2, MinY: 2, MaxX: 3,
MaxY: 3}, true},
+ {"touching corner", iceberg.BoundingBox{MinX: 10, MinY: 10,
MaxX: 20, MaxY: 20}, true},
+ {"disjoint right", iceberg.BoundingBox{MinX: 11, MinY: 0, MaxX:
20, MaxY: 10}, false},
+ {"disjoint diagonal", iceberg.BoundingBox{MinX: 20, MinY: 20,
MaxX: 30, MaxY: 30}, false},
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ eval, err := newInclusiveMetricsEvaluator(
+ geoMetricsSchema,
iceberg.BBoxIntersects(iceberg.Reference("geom"), tt.bbox), true, true)
+ require.NoError(t, err)
+
+ got, err := eval(file)
+ require.NoError(t, err)
+ assert.Equal(t, tt.want, got)
+ })
+ }
+}
+
+// TestInclusiveMetricsBBoxIntersectsAllNull prunes a file whose geometry
column
+// is entirely null: no geometry can intersect any query box.
+func TestInclusiveMetricsBBoxIntersectsAllNull(t *testing.T) {
+ file := &mockDataFile{
+ count: 3,
+ valueCounts: map[int]int64{1: 3},
+ nullCounts: map[int]int64{1: 3},
+ }
+
+ eval, err := newInclusiveMetricsEvaluator(
+ geoMetricsSchema,
+ iceberg.BBoxIntersects(iceberg.Reference("geom"),
iceberg.BoundingBox{MinX: 0, MinY: 0, MaxX: 10, MaxY: 10}),
+ true, true)
+ require.NoError(t, err)
+
+ got, err := eval(file)
+ require.NoError(t, err)
+ assert.False(t, got, "all-null geometry column must be pruned")
+}
+
+// TestInclusiveMetricsBBoxIntersectsNoBounds keeps a file when the geometry
+// column has no usable bounds. Geography columns never emit bounds, so a
+// geography predicate can never prune - which is always safe.
+func TestInclusiveMetricsBBoxIntersectsNoBounds(t *testing.T) {
+ file := &mockDataFile{
+ count: 2,
+ valueCounts: map[int]int64{2: 2},
+ nullCounts: map[int]int64{2: 0},
+ }
+
+ eval, err := newInclusiveMetricsEvaluator(
+ geoMetricsSchema,
+ iceberg.BBoxIntersects(iceberg.Reference("geog"),
iceberg.BoundingBox{MinX: 100, MinY: 100, MaxX: 200, MaxY: 200}),
+ true, true)
+ require.NoError(t, err)
+
+ got, err := eval(file)
+ require.NoError(t, err)
+ assert.True(t, got, "geography column has no bounds, so the file cannot
be pruned")
+}
+
+// TestInclusiveMetricsBBoxIntersectsGeographyWithBounds guards the
antimeridian
+// hazard: a geography file written by another engine can carry bounds, and
those
+// bounds may cross the antimeridian (lower_x > upper_x). A planar min/max
compare
+// would mis-handle the wrapped box and wrongly prune, so geography columns
must
+// never be pruned from bounds - even when the bounds look disjoint from the
query.
+func TestInclusiveMetricsBBoxIntersectsGeographyWithBounds(t *testing.T) {
+ file := &mockDataFile{
+ count: 2,
+ valueCounts: map[int]int64{2: 2},
+ nullCounts: map[int]int64{2: 0},
+ // Bounds that look disjoint from the query box under a planar
compare.
+ lowerBounds: map[int][]byte{2: geoBound2D(0, 0)},
+ upperBounds: map[int][]byte{2: geoBound2D(10, 10)},
+ }
+
+ eval, err := newInclusiveMetricsEvaluator(
+ geoMetricsSchema,
+ iceberg.BBoxIntersects(iceberg.Reference("geog"),
iceberg.BoundingBox{MinX: 100, MinY: 100, MaxX: 200, MaxY: 200}),
+ true, true)
+ require.NoError(t, err)
+
+ got, err := eval(file)
+ require.NoError(t, err)
+ assert.True(t, got, "geography must not be pruned from planar bounds
(antimeridian hazard)")
+}
+
+// TestInclusiveMetricsBBoxNotIntersects never prunes: intersecting bounds
don't
+// prove any geometry lies outside the query box.
+func TestInclusiveMetricsBBoxNotIntersects(t *testing.T) {
+ file := &mockDataFile{
+ count: 2,
+ valueCounts: map[int]int64{1: 2},
+ nullCounts: map[int]int64{1: 0},
+ lowerBounds: map[int][]byte{1: geoBound2D(0, 0)},
+ upperBounds: map[int][]byte{1: geoBound2D(10, 10)},
+ }
+
+ pred := iceberg.BBoxIntersects(iceberg.Reference("geom"),
+ iceberg.BoundingBox{MinX: 20, MinY: 20, MaxX: 30, MaxY:
30}).Negate()
+ eval, err := newInclusiveMetricsEvaluator(geoMetricsSchema, pred, true,
true)
+ require.NoError(t, err)
+
+ got, err := eval(file)
+ require.NoError(t, err)
+ assert.True(t, got, "not-intersects cannot prune from bounds alone")
+}
+
+// TestScanPrunesDisjointGeometryFile drives a bbox filter through a real
+// table.Scan - binding, projection, and the manifest->metrics pruning path -
+// rather than calling the metrics evaluator directly. It proves the
+// metrics-pruning path end-to-end: a data file whose geometry bounds are
+// disjoint from the query box is pruned, while an overlapping one survives.
+//
+// PlanFiles does not run ReadTasks/arrowScan, so this does not exercise
+// column-name translation or the substrait conversion; those regressions (a
+// bbox predicate dropped to AlwaysFalse during translation, or panicking in
+// substrait) are pinned by TestBBoxTranslateColumnNames and
+// TestBBoxPredicateConvertsToTypedError, not here.
+func TestScanPrunesDisjointGeometryFile(t *testing.T) {
+ ctx := context.Background()
+ spec := iceberg.NewPartitionSpec() // geometry cannot be a partition
source
+ memIO := iceio.NewMemFS()
+
+ const geoFieldID = 1
+ schema := iceberg.NewSchema(0,
+ iceberg.NestedField{ID: geoFieldID, Name: "geom", Type:
iceberg.GeometryType{}, Required: false},
+ )
+
+ meta, err := NewMetadata(schema, &spec, UnsortedSortOrder,
"mem://default/table",
+ iceberg.Properties{PropertyFormatVersion: "3"}) // geometry
requires v3
+ require.NoError(t, err)
+
+ builder, err := MetadataBuilderFromBase(meta, "")
+ require.NoError(t, err)
+
+ snapshotID := int64(1)
+ newGeoEntry := func(path string, lower, upper []byte)
iceberg.ManifestEntry {
+ df, err := iceberg.NewDataFileBuilder(spec,
iceberg.EntryContentData, path,
+ iceberg.ParquetFile, nil, nil, nil, 2, 1024)
+ require.NoError(t, err)
+
+ return iceberg.NewManifestEntry(iceberg.EntryStatusADDED,
&snapshotID, nil, nil,
+ df.LowerBoundValues(map[int][]byte{geoFieldID: lower}).
+ UpperBoundValues(map[int][]byte{geoFieldID:
upper}).
+ Build())
+ }
+
+ const overlappingPath = "mem://default/table/data/overlapping.parquet"
+ // File [0,0]-[10,10] overlaps the query box; file [20,20]-[30,30] is
disjoint.
+ entries := []iceberg.ManifestEntry{
+ newGeoEntry(overlappingPath, geoBound2D(0, 0), geoBound2D(10,
10)),
+ newGeoEntry("mem://default/table/data/disjoint.parquet",
geoBound2D(20, 20), geoBound2D(30, 30)),
+ }
+
+ manifestPath := "mem://default/table/metadata/manifest.avro"
+ var manifestBuf bytes.Buffer
+ manifest, err := iceberg.WriteManifest(manifestPath, &manifestBuf, 3,
spec, schema, snapshotID, entries)
+ require.NoError(t, err)
+ require.NoError(t, memIO.WriteFile(manifestPath, manifestBuf.Bytes()))
+
+ manifestListPath :=
"mem://default/table/metadata/snap-1-manifest-list.avro"
+ var listBuf bytes.Buffer
+ seqNum := int64(1)
+ require.NoError(t, iceberg.WriteManifestList(3, &listBuf, snapshotID,
nil, &seqNum, 0,
+ []iceberg.ManifestFile{manifest}))
+ require.NoError(t, memIO.WriteFile(manifestListPath, listBuf.Bytes()))
+
+ firstRowID, addedRows := int64(0), int64(4)
+ require.NoError(t, builder.AddSnapshot(&Snapshot{
+ SnapshotID: snapshotID,
+ SequenceNumber: seqNum,
+ TimestampMs: meta.LastUpdatedMillis() + 1,
+ ManifestList: manifestListPath,
+ Summary: &Summary{Operation: OpAppend},
+ FirstRowID: &firstRowID, // v3 row-lineage bookkeeping
+ AddedRows: &addedRows,
+ }))
+ require.NoError(t, builder.SetSnapshotRef(MainBranch, snapshotID,
BranchRef))
+
+ built, err := builder.Build()
+ require.NoError(t, err)
+
+ tbl := New(Identifier{"db", "tbl"}, built, "metadata.json",
+ func(context.Context) (iceio.IO, error) { return memIO, nil },
nil)
+
+ // Query box [5,5]-[15,15]: overlaps the first file, disjoint from the
second.
+ scan :=
tbl.Scan(WithRowFilter(iceberg.BBoxIntersects(iceberg.Reference("geom"),
+ iceberg.BoundingBox{MinX: 5, MinY: 5, MaxX: 15, MaxY: 15})))
+
+ tasks, err := scan.PlanFiles(ctx)
+ require.NoError(t, err)
+ require.Len(t, tasks, 1, "only the overlapping geometry file survives
pruning")
+ assert.Equal(t, overlappingPath, tasks[0].File.FilePath())
+}
diff --git a/table/internal/geo_codec.go b/table/internal/geo_codec.go
index c36ae8ae7..fe905f502 100644
--- a/table/internal/geo_codec.go
+++ b/table/internal/geo_codec.go
@@ -315,6 +315,42 @@ func decodeGeoBound(data []byte) (vals
[geoNumDims]float64, layout geom.Layout,
return vals, layout, true
}
+// GeoBoundsXY decodes a geometry column's lower and upper geo bounds (Iceberg
+// single-value serialization; see encodeGeoBound) into their planar XY
extents.
+// ok is false when either bound is missing, malformed, or carries a NaN X/Y
+// coordinate - all cases where the bound is unusable for pruning.
+func GeoBoundsXY(lower, upper []byte) (minX, minY, maxX, maxY float64, ok
bool) {
+ lo, _, okLo := decodeGeoBound(lower)
+ hi, _, okHi := decodeGeoBound(upper)
+ if !okLo || !okHi {
+ return 0, 0, 0, 0, false
+ }
+ if math.IsNaN(lo[geoDimX]) || math.IsNaN(lo[geoDimY]) ||
+ math.IsNaN(hi[geoDimX]) || math.IsNaN(hi[geoDimY]) {
+ return 0, 0, 0, 0, false
+ }
+ // Reject inverted bounds (lower > upper). iceberg-go's own accumulator
never
+ // emits them, but this is a trust-on-read decode path: a bound written
by
+ // another engine with lo > hi would make BBoxIntersectsXY always report
+ // no-overlap and prune a file that should be kept. Treating it as
unusable
+ // leaves the file unprunable, which is always safe.
+ if lo[geoDimX] > hi[geoDimX] || lo[geoDimY] > hi[geoDimY] {
+ return 0, 0, 0, 0, false
+ }
+
+ return lo[geoDimX], lo[geoDimY], hi[geoDimX], hi[geoDimY], true
+}
+
+// BBoxIntersectsXY reports whether two planar (XY) bounding boxes intersect.
+// Boxes touching only at an edge or corner count as intersecting (closed
+// intervals), matching Iceberg's inclusive bbox pruning: a box that might
+// contain a matching value must not be pruned. This is the geometry (planar)
+// rule; geography columns emit no bounds (see Bounds), so the antimeridian
+// wrap-around case never reaches pruning.
+func BBoxIntersectsXY(aMinX, aMinY, aMaxX, aMaxY, bMinX, bMinY, bMaxX, bMaxY
float64) bool {
+ return aMinX <= bMaxX && aMaxX >= bMinX && aMinY <= bMaxY && aMaxY >=
bMinY
+}
+
// layoutHasZM reports which optional dimensions a bound layout carries.
func layoutHasZM(l geom.Layout) (hasZ, hasM bool) {
switch l {
diff --git a/table/internal/geo_codec_internal_test.go
b/table/internal/geo_codec_internal_test.go
index 24dbfcfc9..601f0ac25 100644
--- a/table/internal/geo_codec_internal_test.go
+++ b/table/internal/geo_codec_internal_test.go
@@ -423,3 +423,60 @@ func TestNewGeoBoundsAggregatorGeometry(t *testing.T) {
assert.Equal(t, []float64{1, 5}, decodeBound(t, lower))
assert.Equal(t, []float64{30, 20}, decodeBound(t, upper))
}
+
+func TestGeoBoundsXY(t *testing.T) {
+ minX, minY, maxX, maxY, ok := GeoBoundsXY(encGeo(5, 10), encGeo(30, 40))
+ require.True(t, ok)
+ assert.Equal(t, [4]float64{5, 10, 30, 40}, [4]float64{minX, minY, maxX,
maxY})
+
+ // Higher-dimension bounds still yield their XY extents.
+ minX, minY, maxX, maxY, ok = GeoBoundsXY(encGeoZ(1, 2, 3), encGeoZ(4,
5, 6))
+ require.True(t, ok)
+ assert.Equal(t, [4]float64{1, 2, 4, 5}, [4]float64{minX, minY, maxX,
maxY})
+
+ // Missing or malformed bounds are unusable for pruning.
+ _, _, _, _, ok = GeoBoundsXY(nil, encGeo(1, 2))
+ assert.False(t, ok, "missing lower bound")
+ _, _, _, _, ok = GeoBoundsXY(encGeo(1, 2), []byte{0x01})
+ assert.False(t, ok, "malformed upper bound")
+
+ // A NaN X/Y coordinate cannot bound anything.
+ var nan [geoNumDims]float64
+ nan[geoDimX], nan[geoDimY] = math.NaN(), 2
+ _, _, _, _, ok = GeoBoundsXY(encodeGeoBound(nan, geom.XY), encGeo(3, 4))
+ assert.False(t, ok, "NaN coordinate")
+
+ // Inverted bounds (lower > upper) from an untrusted writer are
unusable:
+ // treating them as valid would make BBoxIntersectsXY always report
no-overlap
+ // and prune a file that should be kept.
+ _, _, _, _, ok = GeoBoundsXY(encGeo(30, 10), encGeo(5, 40))
+ assert.False(t, ok, "inverted X bound must be rejected")
+ _, _, _, _, ok = GeoBoundsXY(encGeo(5, 40), encGeo(30, 10))
+ assert.False(t, ok, "inverted Y bound must be rejected")
+}
+
+func TestBBoxIntersectsXY(t *testing.T) {
+ // file box [0,0]-[10,10]
+ tests := []struct {
+ name string
+ qMinX, qMinY, qMaxX, qMaxY float64
+ want bool
+ }{
+ {"overlapping", 5, 5, 15, 15, true},
+ {"contained", 2, 2, 3, 3, true},
+ {"touching edge", 10, 0, 20, 10, true},
+ {"touching corner", 10, 10, 20, 20, true},
+ {"disjoint right", 11, 0, 20, 10, false},
+ {"disjoint above", 0, 11, 10, 20, false},
+ {"disjoint diagonal", 20, 20, 30, 30, false},
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ assert.Equal(t, tt.want,
+ BBoxIntersectsXY(0, 0, 10, 10, tt.qMinX,
tt.qMinY, tt.qMaxX, tt.qMaxY))
+ // intersection is symmetric
+ assert.Equal(t, tt.want,
+ BBoxIntersectsXY(tt.qMinX, tt.qMinY, tt.qMaxX,
tt.qMaxY, 0, 0, 10, 10))
+ })
+ }
+}
diff --git a/table/substrait/substrait.go b/table/substrait/substrait.go
index c6c369570..be6e026a9 100644
--- a/table/substrait/substrait.go
+++ b/table/substrait/substrait.go
@@ -443,3 +443,18 @@ func (t *toSubstraitExpr) VisitNotStartsWith(term
iceberg.BoundTerm, lit iceberg
t.makeLitFunc(startsWithID, term, lit).(expr.FuncArgBuilder),
)
}
+
+// toSubstraitExpr implements the optional BoundGeospatialExprVisitor as a
+// backstop: bbox predicates are dropped to always-true during column-name
+// translation and never reach substrait, but if one ever did these panic with
a
+// wrapped ErrNotImplemented (recovered into a typed, wrappable error by
+// iceberg.VisitExpr) rather than a bare string.
+var _ iceberg.BoundGeospatialExprVisitor[expr.Builder] =
(*toSubstraitExpr)(nil)
+
+func (t *toSubstraitExpr) VisitBBoxIntersects(iceberg.BoundTerm,
iceberg.BoundingBox) expr.Builder {
+ panic(fmt.Errorf("%w: geospatial bbox predicates cannot be converted to
substrait", iceberg.ErrNotImplemented))
+}
+
+func (t *toSubstraitExpr) VisitBBoxNotIntersects(iceberg.BoundTerm,
iceberg.BoundingBox) expr.Builder {
+ panic(fmt.Errorf("%w: geospatial bbox predicates cannot be converted to
substrait", iceberg.ErrNotImplemented))
+}
diff --git a/table/substrait/substrait_test.go
b/table/substrait/substrait_test.go
index e7882575f..e42ceeb1d 100644
--- a/table/substrait/substrait_test.go
+++ b/table/substrait/substrait_test.go
@@ -155,3 +155,22 @@ func TestVariantSchemaConversionDoesNotPanic(t *testing.T)
{
require.NoError(t, err)
assert.NotNil(t, expr)
}
+
+// TestBBoxPredicateConvertsToTypedError ensures a geospatial bbox predicate
that
+// somehow reaches substrait surfaces a wrapped, typed ErrNotImplemented rather
+// than a bare panic string. In normal scans a bbox predicate is dropped to
+// AlwaysTrue during column-name translation and never reaches ConvertExpr;
this
+// pins the backstop so a caller can errors.Is the failure instead of matching
on
+// panic text.
+func TestBBoxPredicateConvertsToTypedError(t *testing.T) {
+ sc := iceberg.NewSchema(1,
+ iceberg.NestedField{ID: 1, Name: "geom", Type:
iceberg.GeometryType{}},
+ )
+ bound, err := iceberg.BBoxIntersects(iceberg.Reference("geom"),
+ iceberg.BoundingBox{MinX: 0, MinY: 0, MaxX: 10, MaxY:
10}).Bind(sc, true)
+ require.NoError(t, err)
+
+ _, _, err = substrait.ConvertExpr(sc, bound, true)
+ require.Error(t, err)
+ assert.ErrorIs(t, err, iceberg.ErrNotImplemented)
+}
diff --git a/visitors.go b/visitors.go
index 47594abf1..19d7b8034 100644
--- a/visitors.go
+++ b/visitors.go
@@ -62,6 +62,21 @@ type BoundBooleanExprVisitor[T any] interface {
VisitNotStartsWith(BoundTerm, Literal) T
}
+// BoundGeospatialExprVisitor is an optional extension a
BoundBooleanExprVisitor
+// may implement to handle the geospatial bbox predicates (BBoxIntersects /
+// BBoxNotIntersects). It is deliberately kept out of BoundBooleanExprVisitor:
+// adding methods to that interface would break every existing implementation -
+// including external ones (query-engine adapters, downstream visitors) - which
+// would then fail to compile until they grew geo methods they may not care
+// about. This mirrors Java, which dispatches geospatial predicates outside its
+// BoundExpressionVisitor. VisitBoundPredicate routes a bbox predicate here
only
+// when the visitor implements this interface; a visitor that does not is
treated
+// as not supporting geospatial predicates.
+type BoundGeospatialExprVisitor[T any] interface {
+ VisitBBoxIntersects(BoundTerm, BoundingBox) T
+ VisitBBoxNotIntersects(BoundTerm, BoundingBox) T
+}
+
// VisitExpr is a convenience function to use a given visitor to visit all
parts of
// a boolean expression in-order. Values returned from the methods are passed
to the
// subsequent methods, effectively "bubbling up" the results.
@@ -110,6 +125,12 @@ func visitBoolExpr[T any](e BooleanExpression, visitor
BooleanExprVisitor[T]) T
// based on the type of operation in the predicate. This is a convenience
function
// for implementing the VisitBound method of a BoundBooleanExprVisitor by
simply calling
// iceberg.VisitBoundPredicate(pred, this).
+//
+// If the predicate is a geospatial bbox op
(OpBBoxIntersects/OpBBoxNotIntersects)
+// and the visitor does not also implement BoundGeospatialExprVisitor[T], this
+// panics with an error wrapping ErrNotImplemented. When reached through
VisitExpr
+// that panic is recovered into a returned error; a caller invoking this
directly
+// must implement the extension or recover the panic itself.
func VisitBoundPredicate[T any](e BoundPredicate, visitor
BoundBooleanExprVisitor[T]) T {
switch e.Op() {
case OpIn:
@@ -140,6 +161,23 @@ func VisitBoundPredicate[T any](e BoundPredicate, visitor
BoundBooleanExprVisito
return visitor.VisitStartsWith(e.Term(),
e.(BoundLiteralPredicate).Literal())
case OpNotStartsWith:
return visitor.VisitNotStartsWith(e.Term(),
e.(BoundLiteralPredicate).Literal())
+ case OpBBoxIntersects, OpBBoxNotIntersects:
+ // Geospatial predicates are dispatched through the optional
+ // BoundGeospatialExprVisitor extension so visitors that
predate (or don't
+ // care about) geo support are not forced to implement them. A
visitor that
+ // doesn't implement it doesn't support geo predicates; panic
here is
+ // recovered into an error by VisitExpr.
+ gv, ok := visitor.(BoundGeospatialExprVisitor[T])
+ if !ok {
+ panic(fmt.Errorf("%w: visitor %T does not support
geospatial bbox predicates",
+ ErrNotImplemented, visitor))
+ }
+ bbox := e.(BoundBBoxPredicate).BBox()
+ if e.Op() == OpBBoxIntersects {
+ return gv.VisitBBoxIntersects(e.Term(), bbox)
+ }
+
+ return gv.VisitBBoxNotIntersects(e.Term(), bbox)
}
panic(fmt.Errorf("%w: unhandled bound predicate type: %s",
ErrNotImplemented, e))
}
@@ -385,6 +423,29 @@ func (e *exprEvaluator) VisitNotStartsWith(term BoundTerm,
lit Literal) bool {
return !e.VisitStartsWith(term, lit)
}
+// VisitBBoxIntersects evaluates a geospatial bbox predicate at the row level.
+// Row-level geometric evaluation (parsing each row's WKB and testing its
+// bounding box) is a query-engine concern - the Iceberg spec only requires
+// bbox-based data skipping, which happens in the metrics/manifest evaluators.
+// Here the predicate conservatively keeps every row so it never drops a row
that
+// an engine's own ST_Intersects would retain.
+func (e *exprEvaluator) VisitBBoxIntersects(BoundTerm, BoundingBox) bool {
+ return true
+}
+
+func (e *exprEvaluator) VisitBBoxNotIntersects(BoundTerm, BoundingBox) bool {
+ // Keep every row, for the same reason as VisitBBoxIntersects above:
row-level
+ // geometric evaluation is a query-engine concern, and bounds alone
cannot
+ // prove a row does not intersect. Must stay true - returning false
here would
+ // silently drop rows an engine's own predicate would retain.
+ return true
+}
+
+// exprEvaluator handles geospatial predicates, so it implements the optional
+// BoundGeospatialExprVisitor. The assertion keeps that wiring compile-checked
now
+// that the geo methods are no longer part of BoundBooleanExprVisitor.
+var _ BoundGeospatialExprVisitor[bool] = (*exprEvaluator)(nil)
+
// RewriteNotExpr rewrites a boolean expression to remove "Not" nodes from the
expression
// tree. This is because Projections assume there are no "not" nodes.
//
@@ -497,6 +558,20 @@ func (columnNameTranslator) VisitUnbound(pred
UnboundPredicate) BooleanExpressio
}
func (c columnNameTranslator) VisitBound(pred BoundPredicate)
BooleanExpression {
+ // A bbox predicate has no substrait/record-filter form; it is
evaluated only
+ // during metrics-based file pruning, so the record filter must
conservatively
+ // keep every row. It must be matched before the column-not-found early
return,
+ // which would otherwise return AlwaysFalse and silently drop every row
of a
+ // file that predates the geo column, and before the switch below, whose
+ // default panics on an unhandled predicate. Collapse it to always-true
+ // (mirrors exprEvaluator and sanitizeVisitor); this also keeps it out
of
+ // substrait, where it would otherwise error. Match the exported
+ // BoundBBoxPredicate interface, not just the concrete type, so a future
+ // implementation is covered too.
+ if _, ok := pred.(BoundBBoxPredicate); ok {
+ return AlwaysTrue{}
+ }
+
fileColName, found :=
c.fileSchema.FindColumnName(pred.Term().Ref().Field().ID)
if !found {
// in the case of schema evolution, the column might not be
present
@@ -560,6 +635,12 @@ func (sanitizeVisitor) VisitOr(left, right
BooleanExpression) BooleanExpression
func (sanitizeVisitor) VisitUnbound(pred UnboundPredicate) BooleanExpression {
switch p := pred.(type) {
+ case *unboundBBoxPredicate:
+ // A bbox predicate carries query-box coordinates, not a
per-row user
+ // literal, and has no REST expression-JSON form (see
MarshalJSON).
+ // Collapse it to always-true so the surrounding expression
still
+ // sanitizes and serializes; nothing user-provided leaks.
+ return AlwaysTrue{}
case *unboundUnaryPredicate:
// No literal to mask; the op and term are safe to keep as-is.
return pred
@@ -573,6 +654,16 @@ func (sanitizeVisitor) VisitUnbound(pred UnboundPredicate)
BooleanExpression {
}
func (sanitizeVisitor) VisitBound(pred BoundPredicate) BooleanExpression {
+ // A bbox predicate carries query-box coordinates, not a per-row user
literal,
+ // and has no REST expression-JSON form (see MarshalJSON), so it cannot
be
+ // rebuilt as a reference predicate like the cases below. Collapse it to
+ // always-true so the surrounding expression still sanitizes and
serializes;
+ // nothing user-provided leaks. Matched before ref is taken and on the
exported
+ // BoundBBoxPredicate interface (mirrors columnNameTranslator).
+ if _, ok := pred.(BoundBBoxPredicate); ok {
+ return AlwaysTrue{}
+ }
+
// Rebuild over the column name as an unbound reference: the sanitized
form is
// only serialized or logged, and an unbound predicate with a masked
string
// literal serializes without needing the field's type.
diff --git a/visitors_test.go b/visitors_test.go
index bb8f7160b..638a09b10 100644
--- a/visitors_test.go
+++ b/visitors_test.go
@@ -172,6 +172,18 @@ func (e *FooBoundExprVisitor)
VisitNotStartsWith(iceberg.BoundTerm, iceberg.Lite
return e.visitHistory
}
+func (e *FooBoundExprVisitor) VisitBBoxIntersects(iceberg.BoundTerm,
iceberg.BoundingBox) []string {
+ e.visitHistory = append(e.visitHistory, "BBOX_INTERSECTS")
+
+ return e.visitHistory
+}
+
+func (e *FooBoundExprVisitor) VisitBBoxNotIntersects(iceberg.BoundTerm,
iceberg.BoundingBox) []string {
+ e.visitHistory = append(e.visitHistory, "BBOX_NOT_INTERSECTS")
+
+ return e.visitHistory
+}
+
func TestBooleanExprVisitor(t *testing.T) {
expr := iceberg.NewAnd(
iceberg.NewOr(
@@ -276,6 +288,116 @@ func TestBoundBoolExprVisitor(t *testing.T) {
}
}
+// noGeoBoundVisitor is a BoundBooleanExprVisitor that deliberately does NOT
+// implement BoundGeospatialExprVisitor. It models an external visitor written
+// before geo support: VisitBoundPredicate must reject a bbox predicate with an
+// ErrNotImplemented error rather than mis-dispatching it. The
literal/set/unary
+// methods are stubs - a bbox expr panics before any of them is reached.
+type noGeoBoundVisitor struct {
+ ExampleVisitor
+}
+
+func (e *noGeoBoundVisitor) VisitBound(pred iceberg.BoundPredicate) []string {
+ return iceberg.VisitBoundPredicate(pred, e)
+}
+
+func (e *noGeoBoundVisitor) VisitUnbound(iceberg.UnboundPredicate) []string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitIn(iceberg.BoundTerm,
iceberg.Set[iceberg.Literal]) []string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitNotIn(iceberg.BoundTerm,
iceberg.Set[iceberg.Literal]) []string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitIsNan(iceberg.BoundTerm) []string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitNotNan(iceberg.BoundTerm) []string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitIsNull(iceberg.BoundTerm) []string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitNotNull(iceberg.BoundTerm) []string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitEqual(iceberg.BoundTerm, iceberg.Literal)
[]string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitNotEqual(iceberg.BoundTerm, iceberg.Literal)
[]string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitGreaterEqual(iceberg.BoundTerm,
iceberg.Literal) []string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitGreater(iceberg.BoundTerm, iceberg.Literal)
[]string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitLessEqual(iceberg.BoundTerm, iceberg.Literal)
[]string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitLess(iceberg.BoundTerm, iceberg.Literal)
[]string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitStartsWith(iceberg.BoundTerm,
iceberg.Literal) []string {
+ return e.visitHistory
+}
+
+func (e *noGeoBoundVisitor) VisitNotStartsWith(iceberg.BoundTerm,
iceberg.Literal) []string {
+ return e.visitHistory
+}
+
+func geoVisitorSchema() *iceberg.Schema {
+ return iceberg.NewSchema(1,
+ iceberg.NestedField{ID: 1, Name: "geom", Type:
iceberg.GeometryType{}, Required: false},
+ )
+}
+
+// TestVisitBoundPredicateDispatchesBBox drives a bound bbox predicate through
the
+// type-assert-and-dispatch path in VisitBoundPredicate: a visitor implementing
+// BoundGeospatialExprVisitor must have VisitBBoxIntersects invoked. This
guards
+// the extension wiring - a refactor breaking the BoundGeospatialExprVisitor[T]
+// assertion would be caught here.
+func TestVisitBoundPredicateDispatchesBBox(t *testing.T) {
+ bound, err := iceberg.BBoxIntersects(iceberg.Reference("geom"),
+ iceberg.BoundingBox{MinX: 0, MinY: 0, MaxX: 10, MaxY:
10}).Bind(geoVisitorSchema(), true)
+ require.NoError(t, err)
+
+ visitor := FooBoundExprVisitor{ExampleVisitor:
ExampleVisitor{visitHistory: []string{}}}
+ result, err := iceberg.VisitExpr(bound, &visitor)
+ require.NoError(t, err)
+ assert.Equal(t, []string{"BBOX_INTERSECTS"}, result)
+}
+
+// TestVisitBoundPredicateBBoxWithoutGeoVisitor pins the load-bearing error
path an
+// external caller hits: a BoundBooleanExprVisitor that does not implement
+// BoundGeospatialExprVisitor, handed a bbox predicate, surfaces an error
wrapping
+// ErrNotImplemented (the panic recovered by VisitExpr) rather than
mis-dispatching.
+func TestVisitBoundPredicateBBoxWithoutGeoVisitor(t *testing.T) {
+ bound, err := iceberg.BBoxIntersects(iceberg.Reference("geom"),
+ iceberg.BoundingBox{MinX: 0, MinY: 0, MaxX: 10, MaxY:
10}).Bind(geoVisitorSchema(), true)
+ require.NoError(t, err)
+
+ visitor := noGeoBoundVisitor{ExampleVisitor:
ExampleVisitor{visitHistory: []string{}}}
+ _, err = iceberg.VisitExpr(bound, &visitor)
+ require.Error(t, err)
+ assert.ErrorIs(t, err, iceberg.ErrNotImplemented)
+}
+
type rowTester []any
func (r rowTester) Size() int { return len(r) }
