API, Core: Add geospatial predicates and bounding box literals #14101

Kontinuation · 2025-09-18T11:32:14Z

This PR is based on #12667. 2 spatial predicates were added to operate on geospatial bounds:

ST_INTERSECTS: Test if 2 geospatial bounds intersects with each other
ST_DISJOINT: The negation of ST_INTERSECTS.

A new literal type BoundingBoxLiteral is added for representing a geospatial query window or geospatial bounds in metrics or Parquet stats.

github-actions · 2025-10-19T00:19:23Z

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

github-actions · 2025-10-27T00:19:28Z

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.

jiayuasu · 2025-11-19T17:04:42Z

@stevenzwu @szehon-ho @huaxingao @wgtmac @hsiang-c Would you please take a look at this PR?

api/src/main/java/org/apache/iceberg/expressions/Literal.java

stevenzwu · 2025-12-09T21:06:45Z

api/src/main/java/org/apache/iceberg/expressions/Literals.java

+
+    @Override
+    protected Type.TypeID typeId() {
+      return Type.TypeID.GEOMETRY;


this seems a bit weird to me. bounding box literal can only be GEOMETRY type?

Maybe we should revisit a previous discussion.
#12667 (comment)

Maybe the BoundingBox needs to include the geo type?

Yes, this is indeed strange, although this implementation works in this case since typeId() is only used by BaseLiteral.toByteBuffer for serializing the value of the literal.

I found 2 ways to fix this weirdness:

Making BoundingBoxLiteral directly implementing interface Literal, but not extends from BaseLiteral. The Literal interface does not have the typeId method we don't have to come up with a confusing implementation.

Adding geo type to BoundingBox. Check that the arguments passed to spatial predicates must have the same geo type.

I am working towards approach 2 and see if everything fits perfectly after revising the BoundingBox definition.

I decided to switch to approach 1 and remove the typeId() method from BoundingBoxLiteral. Here are the reasons:

If BoundingBox carries typeId, it should also carry the full fledged type information including CRS and algorithm. Otherwise it would be strange that only the geospatial type ID is explicitly specified while other type attributes inherit from the type of the field.

Iceberg needs to check that the type of the field and the bounding box matches with each other when binding and evaluating spatial predicates. The CRS and algorithm also need to match.

Once the query engine checked that the geospatial field is supported by the engine and decided to generate a bounding box for pushing down a spatial predicate, it needs extract the geospatial type attributes from the Iceberg schema to construct the BoundingBox. The CRS attribute is tightly coupled with the iceberg table since it is the name of the table property defining the full CRS in PROJJSON, so there's a high chance that the engine will extract field type in iceberg table schema directly use it to construct the bounding box. Inferring the type of BoundingBox according to the context would eliminate the need for this extra step.

The untyped BoundingBox looks concise in the REST Catalog SPEC: REST: Add spatial expressions to REST Catalog OpenAPI Spec #14856. Adding type information to the bounding box would make the data model more cluttered.

I agree with the reasoning. Let's go with approach 1 for now.

api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java

api/src/main/java/org/apache/iceberg/expressions/ExpressionVisitors.java

core/src/test/java/org/apache/iceberg/expressions/TestExpressionParser.java

api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java

api/src/main/java/org/apache/iceberg/expressions/Evaluator.java

api/src/main/java/org/apache/iceberg/expressions/Expression.java

api/src/test/java/org/apache/iceberg/types/TestConversions.java

api/src/test/java/org/apache/iceberg/expressions/TestEvaluator.java

api/src/test/java/org/apache/iceberg/expressions/TestExpressionUtil.java

api/src/test/java/org/apache/iceberg/expressions/TestPredicateBinding.java

core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java

api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java

api/src/main/java/org/apache/iceberg/expressions/Expression.java

api/src/main/java/org/apache/iceberg/expressions/Literal.java

api/src/test/java/org/apache/iceberg/expressions/TestExpressionUtil.java

jiayuasu · 2026-01-19T20:16:02Z

@stevenzwu @huaxingao @wgtmac Hi folks, happy new year! Can you please take a look at this PR again?

api/src/main/java/org/apache/iceberg/expressions/Literals.java

huaxingao

LGTM

api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java

api/src/test/java/org/apache/iceberg/expressions/TestExpressionBinding.java

api/src/test/java/org/apache/iceberg/expressions/TestExpressionUtil.java

stevenzwu

LGTM.

@Kontinuation @jiayuasu we would need more eyes on this from both Iceberg and geo community since it mostly touches api module. I will bring it up in the next Iceberg community sync too.

also @szehon-ho not sure if you also want to take a look.

stevenzwu · 2026-02-04T00:06:14Z

cc @hsiang-c @wgtmac @rdblue for reviews

szehon-ho

Lgtm , thanks @Kontinuation for pr, and @stevenzwu @huaxingao @wgtmac for detailed reviews

huaxingao · 2026-02-09T07:21:48Z

Thanks all. This PR looks in good shape and I’d like to merge it soon. I’ll wait until Wednesday for any additional feedback from @hsiang-c @wgtmac @rdblue; if there are no further comments by then, I’ll go ahead and merge.

huaxingao · 2026-02-09T19:55:10Z

Sorry—I didn’t notice this PR is scheduled for discussion in the next community sync. Let’s wait and follow up after the sync.

stevenzwu · 2026-02-10T06:21:22Z

@Kontinuation please resolve the merge conflict

…4856

github-actions bot added API core labels Sep 18, 2025

This was referenced Sep 18, 2025

[WIP] API, Core: Evaluate spatial predicates in inclusive/strict metrics evaluators #14102

Closed

API: Add geospatial bounding box types and implement intersects checking #12667

Merged

github-actions bot added the stale label Oct 19, 2025

github-actions bot closed this Oct 27, 2025

stevenzwu reopened this Nov 10, 2025

github-actions bot removed the stale label Nov 11, 2025

Kontinuation force-pushed the pr-2-geo-expr branch 2 times, most recently from 560a29d to fa54b84 Compare November 18, 2025 08:55

Kontinuation changed the title ~~[WIP] API, Core: Add geospatial predicates and bounding box literals~~ API, Core: Add geospatial predicates and bounding box literals Nov 18, 2025

Kontinuation marked this pull request as ready for review November 18, 2025 16:20

stevenzwu reviewed Dec 9, 2025

View reviewed changes

wgtmac reviewed Dec 10, 2025

View reviewed changes

github-actions bot added the OPENAPI label Dec 11, 2025

Kontinuation force-pushed the pr-2-geo-expr branch 2 times, most recently from c320c60 to 5bb1a6b Compare December 11, 2025 13:28

huaxingao reviewed Dec 13, 2025

View reviewed changes

api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java Outdated Show resolved Hide resolved

Kontinuation mentioned this pull request Dec 16, 2025

REST: Add spatial expressions to REST Catalog OpenAPI Spec #14856

Open

Kontinuation force-pushed the pr-2-geo-expr branch from 5bb1a6b to 2a56c2b Compare December 16, 2025 03:00

github-actions bot removed the OPENAPI label Dec 16, 2025

Kontinuation force-pushed the pr-2-geo-expr branch from 2a56c2b to 6a4a1b3 Compare December 16, 2025 03:20

stevenzwu reviewed Dec 17, 2025

View reviewed changes

stevenzwu reviewed Dec 18, 2025

View reviewed changes

Kontinuation force-pushed the pr-2-geo-expr branch from 4bb3e84 to fc86d0c Compare December 18, 2025 15:21

huaxingao reviewed Jan 20, 2026

View reviewed changes

api/src/main/java/org/apache/iceberg/expressions/Literals.java Outdated Show resolved Hide resolved

huaxingao approved these changes Jan 20, 2026

View reviewed changes

stevenzwu reviewed Feb 3, 2026

View reviewed changes

api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java Outdated Show resolved Hide resolved

stevenzwu reviewed Feb 3, 2026

View reviewed changes

Kontinuation mentioned this pull request Feb 3, 2026

API: Simplify sanitization of literals in predicates #15224

Merged

Kontinuation force-pushed the pr-2-geo-expr branch from c86b436 to c6698fb Compare February 3, 2026 08:48

stevenzwu approved these changes Feb 3, 2026

View reviewed changes

szehon-ho approved these changes Feb 5, 2026

View reviewed changes

wgtmac approved these changes Feb 9, 2026

View reviewed changes

singhpk234 approved these changes Feb 9, 2026

View reviewed changes

Kontinuation added 13 commits February 10, 2026 15:04

Add spatial expressions

879b55f

Fix up after rebasing

8c749a6

Fix some minor problems

628c935

Address review comments

8821dd4

Revert changes made to open-api SPEC, they were submitted as apache#1…

ebb03e4

…4856

Avoid mutating the value of byte buffer literal

4fe15b0

Making BoundingBoxLiteral directly inherit from Literal<ByteBuffer>

2384bd9

Addressed a few review comments

60ce55f

Revert expression parser changes and defer them to apache#14856

61a6dee

Fix more review comments

b1f04db

re-trigger tests

821e1b5

Normalize the byte buffer when constructing BoundingBoxLiteral

54230d2

Add more tests

f55e6e2

Kontinuation force-pushed the pr-2-geo-expr branch from c6698fb to f55e6e2 Compare February 10, 2026 07:08

API, Core: Add geospatial predicates and bounding box literals #14101

Are you sure you want to change the base?

API, Core: Add geospatial predicates and bounding box literals #14101

Conversation

Kontinuation commented Sep 18, 2025

Uh oh!

github-actions bot commented Oct 19, 2025

Uh oh!

github-actions bot commented Oct 27, 2025

Uh oh!

jiayuasu commented Nov 19, 2025

Uh oh!

Uh oh!

stevenzwu Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Kontinuation Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Kontinuation Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

stevenzwu Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jiayuasu commented Jan 19, 2026

Uh oh!

Uh oh!

huaxingao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stevenzwu left a comment

Choose a reason for hiding this comment

Uh oh!

stevenzwu commented Feb 4, 2026

Uh oh!

szehon-ho left a comment

Choose a reason for hiding this comment

Uh oh!

huaxingao commented Feb 9, 2026

Uh oh!

huaxingao commented Feb 9, 2026

Uh oh!

stevenzwu commented Feb 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants