From 010803a7ab6bafa13a55e157018f1089c28ccdab Mon Sep 17 00:00:00 2001 From: Chris Dobbins Date: Fri, 17 Jul 2026 18:06:19 +0000 Subject: [PATCH 1/5] GEOMESA-3582: Removed BBOX/CASE_WHEN boosting from datastore, retaining datastore SQL as limited to ST_* predicates. --- .../trino/datastore/TrinoFilterToSQL.java | 378 ++--------- .../trino/datastore/FilterParityIT.java | 629 ++++++++++++++++++ .../trino/datastore/TrinoFilterToSQLTest.java | 604 ++++++----------- 3 files changed, 897 insertions(+), 714 deletions(-) create mode 100644 geomesa-trino/geomesa-trino-datastore/src/test/java/org/locationtech/geomesa/trino/datastore/FilterParityIT.java diff --git a/geomesa-trino/geomesa-trino-datastore/src/main/java/org/locationtech/geomesa/trino/datastore/TrinoFilterToSQL.java b/geomesa-trino/geomesa-trino-datastore/src/main/java/org/locationtech/geomesa/trino/datastore/TrinoFilterToSQL.java index 6dd58d6e67b..93458ce7575 100644 --- a/geomesa-trino/geomesa-trino-datastore/src/main/java/org/locationtech/geomesa/trino/datastore/TrinoFilterToSQL.java +++ b/geomesa-trino/geomesa-trino-datastore/src/main/java/org/locationtech/geomesa/trino/datastore/TrinoFilterToSQL.java @@ -32,8 +32,8 @@ import org.geotools.data.jdbc.FilterToSQLException; import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.Point; -import org.locationtech.jts.geom.Polygon; import org.locationtech.jts.io.WKTWriter; import java.io.IOException; @@ -47,15 +47,15 @@ import java.util.stream.Collectors; /** - * Translates GeoTools filters to Trino SQL WHERE clauses against tables that follow - * this project's spatial-column convention: {@code geom VARBINARY} (raw WKB) plus - * companions {@code __geom_bbox__ row(xmin, ymin, xmax, ymax)} and - * {@code __geom_z2__ varchar} / {@code __geom_xz2__ varchar}. If using the spatial - * catalog, applies bbox/Z2-stat pushdown on top of the same SQL via - * {@code SpatialConnectorMetadata.applyFilter}. + * Translates GeoTools filters to Trino SQL WHERE clauses of pure row-level + * {@code ST_*} function calls (plus temporal/attribute predicates). Geom column + * references are always wrapped in {@code ST_GeomFromBinary(geom)} and geometry + * literals in {@code ST_GeometryFromText('')} — matches the shape recognized + * by the {@code spatial_iceberg} connector's {@code SpatialConnectorMetadata.applyFilter}. * - *

Geom column references are always wrapped in {@code ST_GeomFromBinary(geom)} - * before being handed to Trino's stock spatial functions. + *

Swapped operand forms are normalized column-first via the OGC complement + * ({@code within(a, b) ⇔ contains(b, a)}) so the connector's + * {@code ST_*(ST_GeomFromBinary(col), literal)} recognizer always matches. */ public class TrinoFilterToSQL extends FilterToSQL { @@ -88,26 +88,20 @@ private String geomFromText(Geometry geom) { * concurrent queries. */ private final WKTWriter wkt = new WKTWriter(); + /** For envelope→rectangle conversion (BBOX filters, DWithin outer bounds). */ + private static final GeometryFactory GEOMETRY_FACTORY = new GeometryFactory(); + /** Approximate meters per degree of latitude (and of longitude at the equator). */ private static final double METERS_PER_DEGREE = 111_111.0; - /** Outward margin on the DWITHIN outer bbox, absorbing flat-vs-spherical + /** Outward margin on the DWITHIN outer envelope, absorbing flat-vs-spherical * projection error so rows whose true distance is just under d aren't excluded. */ private static final double OUTER_SAFETY_MARGIN = 1.1; - /** Inward margin on the DWITHIN inner inscribed rectangle, applied conservatively - * so the bbox-contained shortcut never produces a false TRUE. */ - private static final double INNER_SAFETY_MARGIN = 0.9; - - /** Half-diagonal-to-half-side factor for a square inscribed in the d-circle. */ - private static final double INSCRIBED_FACTOR = 1.0 / Math.sqrt(2.0); - /** Latitude (deg) beyond which the flat cos(lat) longitude scaling degenerates: as * {@code cos(lat) -> 0} the per-degree-longitude distance vanishes, so a within-d region * spans (nearly) all longitudes. Past this we use a full-longitude outer band (so the - * prefilter never drops matching rows) and drop the inscribed-rectangle shortcut (whose - * flat approximation is unsound near the poles), falling through to the exact spherical - * distance check. Also avoids the division-by-zero at exactly ±90°. */ + * prefilter never drops matching rows). Also avoids the division-by-zero at exactly ±90°. */ private static final double NEAR_POLE_LAT = 85.0; // ── Spatial ─────────────────────────────────────────────────────────────── @@ -121,8 +115,8 @@ private String geomFromText(Geometry geom) { /** Single translation point for every binary spatial operator. {@code swapped} * means the geometry literal was expression1 (e.g. {@code INTERSECTS(POLYGON, * geom)}) — irrelevant for symmetric operators, semantics-reversing for - * Within/Contains (which are each other's complement: {@code contains(a, b) ⇔ - * within(b, a)}). Disjoint and Beyond get NO bbox prefilter */ + * Within/Contains, which are normalized column-first via their OGC complement + * ({@code within(a, b) ⇔ contains(b, a)}). */ @Override protected Object visitBinarySpatialOperator(BinarySpatialOperator filter, PropertyName property, Literal literal, boolean swapped, Object extraData) { @@ -132,34 +126,25 @@ protected Object visitBinarySpatialOperator(BinarySpatialOperator filter, "Unsupported spatial filter literal (expected a geometry): " + filter); } if (filter instanceof Intersects) { // symmetric - writeIntersects(col, geom); + writeSpatial("ST_Intersects", col, geom); } else if (filter instanceof Within) { - if (swapped) { - writeLiteralWithinColumn(col, geom); - } else { - writeWithin(col, geom); - } + writeSpatial(swapped ? "ST_Contains" : "ST_Within", col, geom); } else if (filter instanceof Contains) { - if (swapped) { - writeWithin(col, geom); // CONTAINS(lit, geom) ⇔ WITHIN(geom, lit) - } else { - writeColumnContainsLiteral(col, geom); - } + writeSpatial(swapped ? "ST_Within" : "ST_Contains", col, geom); } else if (filter instanceof DWithin d) { // symmetric writeDWithin(col, geom, convertToMeters(d.getDistance(), d.getDistanceUnits())); } else if (filter instanceof Beyond b) { // symmetric writeBeyond(col, geom, convertToMeters(b.getDistance(), b.getDistanceUnits())); } else if (filter instanceof Crosses) { // symmetric - writeIntersectionImplyingOp("ST_Crosses", col, geom); + writeSpatial("ST_Crosses", col, geom); } else if (filter instanceof Touches) { // symmetric - writeIntersectionImplyingOp("ST_Touches", col, geom); + writeSpatial("ST_Touches", col, geom); } else if (filter instanceof Overlaps) { // symmetric - writeIntersectionImplyingOp("ST_Overlaps", col, geom); + writeSpatial("ST_Overlaps", col, geom); } else if (filter instanceof Equals) { // symmetric - writeIntersectionImplyingOp("ST_Equals", col, geom); + writeSpatial("ST_Equals", col, geom); } else if (filter instanceof Disjoint) { // symmetric - write("ST_Disjoint(ST_GeomFromBinary(" + quoteIdent(col) + ")," - + " " + geomFromText(geom) + ")"); + writeSpatial("ST_Disjoint", col, geom); } else { throw new IllegalArgumentException("Unsupported spatial operator: " + filter); } @@ -176,7 +161,10 @@ protected Object visitBinarySpatialOperator(BinarySpatialOperator filter, } /** - * Translates a BBOX filter into a bbox-overlap predicate. + * Translates a BBOX filter into {@code ST_Intersects} against the envelope + * rectangle — {@code BBOX(geom, env) ⇔ INTERSECTS(geom, envelope-rectangle)}. + * The connector derives bbox/Z2 pruning from the call; the exact row-level test + * keeps the predicate correct against the float32-rounded stored bboxes. * * @param filter bbox filter * @param extraData caller-supplied context, returned unchanged @@ -187,30 +175,26 @@ public Object visit(BBOX filter, Object extraData) { String col = filter.getExpression1() instanceof PropertyName pn ? pn.getPropertyName() : defaultGeomCol(); BoundingBox b = filter.getBounds(); - // BBOX(geom, env) ⇔ INTERSECTS(geom, envelope-rectangle) — route through the - // rectangle intersects emission (overlap prefilter + shrunk-contained shortcut - // + exact ST_Intersects fallback). A bare float32 bbox-overlap is NOT exact: - // the stored bbox is rounded to nearest, so it admits rows up to ½ ulp outside - // the envelope (boundary rows made this visible in the filter-parity suite). Envelope env = new Envelope(b.getMinX(), b.getMaxX(), b.getMinY(), b.getMaxY()); - writeIntersects(col, new org.locationtech.jts.geom.GeometryFactory().toGeometry(env)); + writeSpatial("ST_Intersects", col, GEOMETRY_FACTORY.toGeometry(env)); return extraData; } - /** WITHIN(geom, literal): rectangular query polygons take the shrunk-contained - * shortcut with an exact ST_Within fallback (see {@link #writeWithinRectangle}); - * anything else emits bbox-overlap (Z2 pushdown) AND exact ST_Within at row - * level. Also serves swapped Contains - * ({@code CONTAINS(lit, geom) ⇔ WITHIN(geom, lit)}). */ - private void writeWithin(String col, Geometry geom) { - if (geom instanceof Polygon p && p.isRectangle()) { - writeWithinRectangle(col, p); - } else { - writeWithinNonRectangle(col, geom); - } + /** The single spatial emission shape: {@code ST_Fn(ST_GeomFromBinary(col), + * ST_GeometryFromText(''))}, column-first — exactly what the connector's + * {@code extractGeomColumnName}/{@code tryExtractEnvelope} recognize for + * bbox/Z2/XZ2 pushdown (ST_Disjoint is deliberately never pushed there). */ + private void writeSpatial(String function, String col, Geometry geom) { + write(function + "(ST_GeomFromBinary(" + quoteIdent(col) + ")," + + " " + geomFromText(geom) + ")"); } - /** DWithin translation: outer/inner bbox bounds plus an exact spherical distance check. */ + /** + * DWithin translation: a necessary {@code ST_Intersects} against the outer + * envelope rectangle (every geometry within d of the reference overlaps it, so + * the connector derives bbox/Z2 pruning from the call) AND the exact spherical + * distance check at row level. + */ private void writeDWithin(String col, Geometry refGeom, double distanceMeters) { // The reference envelope. For a point this degenerates to the point itself; for an @@ -219,9 +203,9 @@ private void writeDWithin(String col, Geometry refGeom, double distanceMeters) { // around the centroid would wrongly prune rows near the reference's far ends. Envelope ref = refGeom.getEnvelopeInternal(); - // OUTER bbox: bounding box of "every point within d of ref" — necessary-overlap - // prefilter. The safety margin absorbs flat-vs-spherical projection error so we - // don't accidentally exclude rows whose true distance is just under d. + // OUTER envelope: bounding box of "every point within d of ref". The safety margin + // absorbs flat-vs-spherical projection error so we don't accidentally exclude rows + // whose true distance is just under d. // Longitude scaling: degrees of longitude shrink toward the poles, so cos() is // evaluated at the POLEWARD EDGE of the expanded latitude band (not its center) — // the narrowest point, where the required longitude span is widest; sizing by the @@ -241,72 +225,16 @@ private void writeDWithin(String col, Geometry refGeom, double distanceMeters) { : new Envelope(ref.getMinX() - outerDegLon, ref.getMaxX() + outerDegLon, ref.getMinY() - outerDegLat, ref.getMaxY() + outerDegLat); - String ptWkt = wkt.write(refGeom).replace("'", "''"); - String bboxCol = bboxColName(col); - // Exact spherical distance — always correct, used as the fallback (and, for - // non-point references and near the poles, the sole) row-level check. + String refWkt = wkt.write(refGeom).replace("'", "''"); String distanceCheck = String.format(Locale.ROOT, "%s <= %.0f", - sphericalDistanceSql(col, refGeom, ptWkt), distanceMeters); - - if (nearPole || !(refGeom instanceof Point)) { - // No inscribed-rectangle shortcut here. Near the poles the flat approximation - // is unsound where longitude lines converge. For an extended reference the - // rectangle would sit on the envelope centroid — a point that need not lie ON - // the geometry, so rectangle containment proves nothing about distance to it. - // Every candidate row falls through to the exact distance check. - write("(" + bboxOverlapSql(bboxCol, outer) + ") AND " + distanceCheck); - } else { - double lon = ((Point) refGeom).getX(); - double lat = ((Point) refGeom).getY(); - // INNER inscribed rectangle (point references only): half-sides (d × - // INNER_SAFETY_MARGIN × INSCRIBED_FACTOR) scaled to lat/lon. Corners land at - // distance ≤ INNER_SAFETY_MARGIN × d from ref, so any bbox(geom) ⊆ this - // rectangle ⇒ every point of geom is within d ⇒ TRUE. - // Longitude scaling mirrors the outer box in the conservative direction: cos() - // is evaluated at the EQUATORWARD EDGE of the band (its physically widest - // point), so the rectangle's real half-width never exceeds the target anywhere - // in the band — sizing by the center latitude lets the equatorward corners land - // beyond d (e.g. lat 70°, d=1000 km: corner ≈ 1.03 d), wrongly including rows. - double innerDegLat = (distanceMeters / METERS_PER_DEGREE) * INSCRIBED_FACTOR * INNER_SAFETY_MARGIN; - double equatorwardLat = Math.max(Math.abs(lat) - innerDegLat, 0.0); - double innerDegLon = (distanceMeters / (METERS_PER_DEGREE * Math.cos(Math.toRadians(equatorwardLat)))) - * INSCRIBED_FACTOR * INNER_SAFETY_MARGIN; - Envelope inner = new Envelope(lon - innerDegLon, lon + innerDegLon, - lat - innerDegLat, lat + innerDegLat); - // Outer bbox-overlap (file/Z2 pruning) AND CASE WHEN inner-rectangle-contained - // (sufficient for distance ≤ d) THEN TRUE ELSE exact spherical distance check. - write("(" + bboxOverlapSql(bboxCol, outer) + ") AND " - + "CASE WHEN " + bboxContainedSql(bboxCol, inner) + " THEN TRUE" - + " ELSE " + distanceCheck + " END"); - } - } - - // ── Other binary spatial operators ──────────────────────────────────────── - // - // Crosses/Touches/Overlaps/Equals all imply a non-empty intersection, so each - // gets the pushable bbox-overlap prefilter (Z2/file pruning) plus the exact - // row-level ST_* test. No CASE WHEN shortcuts: envelope containment proves - // nothing for these predicates (see the Intersects rectangle gate). - - /** {@code CONTAINS(geom, literal)} — the row geometry contains the literal, so - * the pushable prefilter is bbox-COVERS (the row bbox must contain the literal's - * envelope) plus exact {@code ST_Contains}. The literal-first form is handled in - * the dispatcher as {@code WITHIN(geom, literal)}. */ - private void writeColumnContainsLiteral(String col, Geometry geom) { - String q = quoteIdent(bboxColName(col)); - Envelope env = geom.getEnvelopeInternal(); - // Float32-aligned bounds keep the covers-prefilter necessary against the - // nearest-rounded stored bbox; see bboxOverlapSql. - String bboxCovers = String.format( - "%s.xmin <= %s AND %s.xmax >= %s AND %s.ymin <= %s AND %s.ymax >= %s", - q, ceilF32(env.getMinX()), q, floorF32(env.getMaxX()), - q, ceilF32(env.getMinY()), q, floorF32(env.getMaxY())); - write("(" + bboxCovers + ")" - + " AND ST_Contains(ST_GeomFromBinary(" + quoteIdent(col) + ")," - + " " + geomFromText(geom) + ")"); + sphericalDistanceSql(col, refGeom, refWkt), distanceMeters); + write("ST_Intersects(ST_GeomFromBinary(" + quoteIdent(col) + ")," + + " " + geomFromText(GEOMETRY_FACTORY.toGeometry(outer)) + ")" + + " AND " + distanceCheck); } - /** Beyond — DWithin's complement: exact spherical distance check, no prefilter. */ + /** Beyond — DWithin's complement: exact spherical distance check, no prefilter + * (the matching rows are OUTSIDE the neighborhood, like ST_Disjoint's). */ private void writeBeyond(String col, Geometry refGeom, double distanceMeters) { String refWkt = wkt.write(refGeom).replace("'", "''"); write(String.format(Locale.ROOT, "%s > %.0f", @@ -334,16 +262,6 @@ private String sphericalDistanceSql(String col, Geometry refGeom, String refWkt) + " to_spherical_geography(" + np + "[2]))"; } - /** Shared shape for the intersection-implying operators (Crosses, Touches, - * Overlaps, Equals): the pushable bbox-overlap prefilter (necessary — each of - * these predicates implies a non-empty intersection, which implies overlapping - * envelopes) AND the exact row-level test. */ - private void writeIntersectionImplyingOp(String function, String col, Geometry geom) { - write("(" + bboxOverlapSql(bboxColName(col), geom.getEnvelopeInternal()) + ")" - + " AND " + function + "(ST_GeomFromBinary(" + quoteIdent(col) + ")," - + " " + geomFromText(geom) + ")"); - } - // ── Temporal ────────────────────────────────────────────────────────────── /** @@ -403,66 +321,6 @@ public Object visit(Id filter, Object extraData) { // ── Helpers ─────────────────────────────────────────────────────────────── - /** - * Emit the 4-clause necessary-overlap predicate against {@code ___bbox__}. - * Single source of truth for the bbox-overlap shape that SI's - * {@code tryExtractBboxEnvelope} matches for file-level Z2 / bbox-stat pruning. - */ - private void writeBboxOverlap(String col, Envelope env) { - write(bboxOverlapSql(bboxColName(col), env)); - } - - /** - * The 4-clause "bbox(geom) overlaps env" fragment. Necessary condition for any - * geom/env interaction, and the exact shape SI's connector reads to push Z2 - * partition pruning + per-file bbox-stat pruning. - * - *

Bounds are emitted float32-aligned ({@link #floorF32}/{@link #ceilF32}): - * the stored bbox values are float32 rounded to NEAREST, so comparing them - * against the raw double bound can drop a row whose true coordinate is within - * ~½ ulp inside the envelope edge (the stored value rounds across it). Float - * rounding is monotone, so relaxing each bound to the nearest float in the - * admitting direction keeps the prefilter necessary without admitting more - * than one extra float32 quantum per side. - */ - private static String bboxOverlapSql(String bboxCol, Envelope env) { - String q = quoteIdent(bboxCol); - return String.format( - "%s.xmax >= %s AND %s.xmin <= %s" + - " AND %s.ymax >= %s AND %s.ymin <= %s", - q, floorF32(env.getMinX()), q, ceilF32(env.getMaxX()), - q, floorF32(env.getMinY()), q, ceilF32(env.getMaxY())); - } - - /** Largest float32 value ≤ v (as a double literal); see {@link #bboxOverlapSql}. - * Mirrors the connector's {@code pruneSafeLowerBound}. Exactly-representable - * values pass through unchanged. */ - private static double floorF32(double v) { - float f = (float) v; - return f > v ? Math.nextDown(f) : f; - } - - /** Smallest float32 value ≥ v (as a double literal); see {@link #floorF32}. */ - private static double ceilF32(double v) { - float f = (float) v; - return f < v ? Math.nextUp(f) : f; - } - - /** The 4-clause "bbox(geom) fully contained in env" fragment. */ - private static String bboxContainedSql(String bboxCol, Envelope env) { - String q = quoteIdent(bboxCol); - return String.format( - "%s.xmin >= %s AND %s.xmax <= %s" + - " AND %s.ymin >= %s AND %s.ymax <= %s", - q, env.getMinX(), q, env.getMaxX(), - q, env.getMinY(), q, env.getMaxY()); - } - - /** Returns the synthetic bbox struct column name for a given geometry column. */ - private static String bboxColName(String col) { - return "__" + col + "_bbox__"; - } - /** Fallback geometry column name when no PropertyName is available. */ private String defaultGeomCol() { return featureType != null && featureType.getGeometryDescriptor() != null @@ -470,138 +328,6 @@ private String defaultGeomCol() { : "geom"; } - /** - * For an axis-aligned rectangular query polygon: bbox-overlap (necessary; lets - * SI reconstruct the envelope and push Z2/bbox pruning) AND CASE WHEN the bbox - * is contained in a slightly-SHRUNK rectangle THEN TRUE ELSE the exact - * {@code ST_Within} test. - * - *

The shortcut cannot use the rectangle itself: WITHIN is boundary-exclusive - * (a geometry touching the rectangle's edge is NOT within it), while the - * inclusive bbox-contained comparison would admit it — boundary-snapped GPS - * points made exactly that mismatch visible in the filter-parity suite. The - * stored bbox is also float32 (rounded to nearest, up to ½ ulp off the true - * bounds). Shrinking each side by two float ulps makes containment prove the - * geometry lies strictly inside the rectangle; anything on or near the boundary - * falls through to the exact row-level test. - */ - private void writeWithinRectangle(String col, Polygon rect) { - String bboxCol = bboxColName(col); - Envelope env = rect.getEnvelopeInternal(); - Envelope shrunk = new Envelope( - shrinkLo(env.getMinX()), shrinkHi(env.getMaxX()), - shrinkLo(env.getMinY()), shrinkHi(env.getMaxY())); - write("(" + bboxOverlapSql(bboxCol, env) + ") AND " - + "CASE WHEN " + bboxContainedSql(bboxCol, shrunk) + " THEN TRUE" - + " ELSE ST_Within(ST_GeomFromBinary(" + quoteIdent(col) + ")," - + " " + geomFromText(rect) + ") END"); - } - - /** A lower bound moved two float32 ulps inward; see {@link #writeWithinRectangle}. */ - private static double shrinkLo(double v) { - return Math.nextUp(Math.nextUp((float) v)); - } - - /** An upper bound moved two float32 ulps inward; see {@link #writeWithinRectangle}. */ - private static double shrinkHi(double v) { - return Math.nextDown(Math.nextDown((float) v)); - } - - /** - * Non-rectangular polygons require the exact ST_Within row-level test — - * bbox⊆env(polygon) doesn't prove geom⊆polygon since the polygon is a strict - * subset of its envelope. We still emit bbox-overlap as a leading conjunct so - * SI's connector can push Z2 partition pruning + bbox file-stat pruning. - */ - private void writeWithinNonRectangle(String col, Geometry queryGeom) { - String bboxCol = bboxColName(col); - Envelope env = queryGeom.getEnvelopeInternal(); - write("(" + bboxOverlapSql(bboxCol, env) + ")" - + " AND ST_Within(ST_GeomFromBinary(" + quoteIdent(col) + ")," - + " " + geomFromText(queryGeom) + ")"); - } - - /** - * {@code WITHIN(literal, geom)} — the literal contained in the row geometry. The - * necessary pushable prefilter is bbox-COVERS: the row bbox must contain the - * literal's envelope (literal ⊆ geom ⇒ env(literal) ⊆ env(geom) = bbox). The exact - * test is {@code ST_Within(literal, geom)} with the operands in the original order. - */ - private void writeLiteralWithinColumn(String col, Geometry literalGeom) { - String q = quoteIdent(bboxColName(col)); - Envelope env = literalGeom.getEnvelopeInternal(); - // Float32-aligned bounds keep the covers-prefilter necessary against the - // nearest-rounded stored bbox; see bboxOverlapSql. - String bboxCovers = String.format( - "%s.xmin <= %s AND %s.xmax >= %s AND %s.ymin <= %s AND %s.ymax >= %s", - q, ceilF32(env.getMinX()), q, floorF32(env.getMaxX()), - q, ceilF32(env.getMinY()), q, floorF32(env.getMaxY())); - write("(" + bboxCovers + ")" - + " AND ST_Within(" + geomFromText(literalGeom) + "," - + " ST_GeomFromBinary(" + quoteIdent(col) + "))"); - } - - /** - * Emits a two-part predicate that gives both file-level Z2/bbox pruning AND - * row-level short-circuiting: - * - * 

-     * (bbox-overlap)  AND  CASE WHEN bbox-contained THEN TRUE ELSE ST_Intersects(...) END
-     *
- * - *

The leading bbox-overlap conjunct is the same shape that - * {@code SpatialConnectorMetadata.tryExtractBboxEnvelope} already recognizes — - * the SI connector extracts the envelope from it and pushes Z2 partition - * pruning + per-file bbox-stat pruning. The CASE WHEN tail survives Trino's - * optimizer intact and short-circuits the WKB decode + ST_Intersects test - * for any row whose bbox is fully inside the envelope. - * - *

The CASE WHEN shortcut applies ONLY to axis-aligned rectangular - * query polygons (where the polygon equals its envelope). For any other query - * geometry, bbox-containment in the ENVELOPE does not imply intersection with the - * GEOMETRY (examples, a polygon with a hole, a crescent, etc. contain - * envelope regions outside the polygon), so the exact {@code ST_Intersects} runs for - * every bbox-overlapping row instead. For rectangles: - *

- * - *

Why CASE not OR: Trino's optimizer distributes OR over AND - * ({@code (A AND B AND C AND D) OR X} → {@code (A OR X) AND (B OR X) AND (C OR X) AND (D OR X)}), - * causing ST_Intersects to evaluate up to 4× per row (3.3× slowdown measured). - * CASE WHEN is opaque to that rewrite. - */ - private void writeIntersects(String col, Geometry geom) { - String bboxCol = bboxColName(col); - Envelope env = geom.getEnvelopeInternal(); - String exact = "ST_Intersects(ST_GeomFromBinary(" + quoteIdent(col) + ")," - + " " + geomFromText(geom) + ")"; - if (geom instanceof Polygon p && p.isRectangle()) { - // Rectangle: bbox-overlap (necessary; pushable to file-level pruning) AND - // CASE WHEN bbox-contained (sufficient — the polygon IS its envelope) THEN TRUE - // ELSE exact ST_Intersects. CASE WHEN (not OR) survives Trino's optimizer intact. - // The shortcut rectangle is shrunk by two float ulps per side: the stored - // bbox is float32 rounded to nearest (up to ½ ulp off the true bounds), so - // containment in the UNSHRUNK rectangle could return TRUE for a geometry a - // hair outside it; shrunk containment proves true containment, and the - // boundary shell falls through to the exact test. - Envelope shrunk = new Envelope( - shrinkLo(env.getMinX()), shrinkHi(env.getMaxX()), - shrinkLo(env.getMinY()), shrinkHi(env.getMaxY())); - write("(" + bboxOverlapSql(bboxCol, env) + ") AND " - + "CASE WHEN " + bboxContainedSql(bboxCol, shrunk) + " THEN TRUE" - + " ELSE " + exact + " END"); - } else { - // Non-rectangular query: envelope containment alone doesn't imply intersection - // with the geometry itself, so no row-level shortcut — just the - // pushable bbox-overlap prefilter and the exact test. - write("(" + bboxOverlapSql(bboxCol, env) + ") AND " + exact); - } - } - private static String formatTimestamp(Date date) { SimpleDateFormat fmt = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss z"); fmt.setTimeZone(TimeZone.getTimeZone("UTC")); diff --git a/geomesa-trino/geomesa-trino-datastore/src/test/java/org/locationtech/geomesa/trino/datastore/FilterParityIT.java b/geomesa-trino/geomesa-trino-datastore/src/test/java/org/locationtech/geomesa/trino/datastore/FilterParityIT.java new file mode 100644 index 00000000000..fa8b1350176 --- /dev/null +++ b/geomesa-trino/geomesa-trino-datastore/src/test/java/org/locationtech/geomesa/trino/datastore/FilterParityIT.java @@ -0,0 +1,629 @@ +/*********************************************************************** + * Copyright (c) 2013-2025 General Atomics Integrated Intelligence, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Apache License, Version 2.0 + * which accompanies this distribution and is available at + * https://www.apache.org/licenses/LICENSE-2.0 + ***********************************************************************/ + +package org.locationtech.geomesa.trino.datastore; + +import org.geotools.api.data.DataStore; +import org.geotools.api.data.DataStoreFinder; +import org.geotools.api.data.Query; +import org.geotools.api.data.SimpleFeatureSource; +import org.geotools.api.feature.simple.SimpleFeature; +import org.geotools.api.filter.Filter; +import org.geotools.data.simple.SimpleFeatureIterator; +import org.geotools.filter.text.ecql.ECQL; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DynamicTest; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.TestFactory; +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.Envelope; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.io.WKTReader; +import org.locationtech.jts.operation.distance.DistanceOp; + +import java.time.Instant; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Filter-processing parity suite, ported from the accumulo-datastore module's + * {@code org.locationtech.geomesa.accumulo.filter.FilterTest}: for every CQL + * predicate, the features returned by querying the datastore (which translates + * the filter to Trino SQL) must equal the result of evaluating the same filter + * in memory against a known feature list. + * + *

Oracle design. The accumulo test ingests a small synthetic feature + * set in-test; this datastore is read-only, so the oracle is instead a bounded + * window of a pre-loaded dataset: a small bbox is discovered around the + * data's median location and every feature inside it is fetched once. Each + * ported predicate is then ANDed with the window bbox for the datastore query, + * and evaluated in memory over the window list — {@code datastore(filter AND + * window) == inMemory(filter, windowFeatures)}. The window fetch itself is + * cross-checked against a plain-SQL count through the un-wrapped iceberg + * catalog when it is exposed. + * + *

Distance predicates. DWITHIN/BEYOND are excluded from the plain + * GeoTools-evaluate oracle on purpose: GeoTools evaluates the distance in + * degrees regardless of the declared units (the accumulo suite marks these + * {@code pendingUntilFixed} for the same reason), while this datastore emits an + * exact spherical-meters check. The oracle here computes the great-circle + * distance directly, and features within a small tolerance band of the radius + * are excluded from both sides before comparing, so formula differences at the + * boundary cannot flap the assertion. + * + *

Provisioning. Runs against the local stack (Trino at + * localhost:8080, catalog {@code spatial_iceberg}, schema {@code spatial}); + * each dataset's tests skip when its table is absent. The expected tables are + * ingests of public datasets sharing the layout {@code __fid__, geom (point + * WKB + __geom_bbox__/__geom_z2__ companions), dtg (timestamptz)} plus the + * listed attributes: + *

+ */ +@Tag("integration") +class FilterParityIT { + + /** Window sizing: grow/shrink the discovery bbox until the feature count + * lands in this range — big enough to exercise every predicate, small + * enough to hold in memory as the oracle. */ + private static final int MIN_WINDOW = 800; + private static final int MAX_WINDOW = 60_000; + + /** Distance-predicate tolerance band: features whose spherical distance is + * within max(1.5% of the radius, 10 m) of the radius are excluded from + * both sides of the comparison. */ + private static final double TOLERANCE_FRACTION = 0.015; + private static final double TOLERANCE_FLOOR_METERS = 10.0; + + private static final double EARTH_RADIUS_METERS = 6_371_008.8; + + private static DataStore ds; + + @BeforeAll + static void connect() throws Exception { + try (var socket = new java.net.Socket()) { + socket.connect(new java.net.InetSocketAddress("localhost", 8080), 2000); + } catch (Exception e) { + Assumptions.assumeTrue(false, "Trino not reachable at localhost:8080 — skipping integration tests"); + } + ds = DataStoreFinder.getDataStore(Map.of( + "trino.host", "localhost", + "trino.port", 8080, + "trino.catalog", "spatial_iceberg", + "trino.schema", "spatial" + )); + assertThat(ds).isNotNull(); + } + + @AfterAll + static void disconnect() { + if (ds != null) ds.dispose(); + } + + // ── Dataset entry points ────────────────────────────────────────────────── + + @TestFactory + List tdrive() throws Exception { + Fixture fx = Fixture.load("tdrive", "taxi_id", null); + return cases(fx); + } + + @TestFactory + List geolife() throws Exception { + Fixture fx = Fixture.load("geolife", "altitude_ft", "user_id"); + return cases(fx); + } + + @TestFactory + List ais() throws Exception { + Fixture fx = Fixture.load("ais", "mmsi", "vessel_name"); + return cases(fx); + } + + // ── Fixture: window oracle + derived template values ────────────────────── + + private static final class Fixture { + final String table; + final String geomField; + final List window; + final Envelope env; + /** Template substitutions derived from the window contents. */ + final Map vars = new HashMap<>(); + final String stringAttr; // null when the dataset has no usable string column + + private Fixture(String table, String geomField, List window, + Envelope env, String stringAttr) { + this.table = table; + this.geomField = geomField; + this.window = window; + this.env = env; + this.stringAttr = stringAttr; + } + + static Fixture load(String table, String numericAttr, String stringAttr) throws Exception { + Assumptions.assumeTrue(Arrays.asList(ds.getTypeNames()).contains(table), + "table spatial." + table + " not ingested — skipping (see class javadoc)"); + SimpleFeatureSource src = ds.getFeatureSource(table); + String geomField = src.getSchema().getGeometryDescriptor().getLocalName(); + + // Median location of a sample, as the window center. + Query sample = new Query(table); + sample.setMaxFeatures(2000); + List pts = new ArrayList<>(); + try (SimpleFeatureIterator it = src.getFeatures(sample).features()) { + while (it.hasNext()) { + pts.add(((Geometry) it.next().getDefaultGeometry()).getCoordinate()); + } + } + Assumptions.assumeTrue(pts.size() >= MIN_WINDOW, + "table spatial." + table + " has too few rows for a parity window"); + double cx = median(pts.stream().map(c -> c.x).sorted().toList()); + double cy = median(pts.stream().map(c -> c.y).sorted().toList()); + + // Grow/shrink the window until the count is workable. + double delta = 0.005; + Envelope env = null; + int count = -1; + for (int i = 0; i < 30 && (count < MIN_WINDOW || count > MAX_WINDOW); i++) { + env = new Envelope(cx - delta, cx + delta, cy - delta, cy + delta); + count = src.getCount(new Query(table, ECQL.toFilter(bboxCql(geomField, env)))); + delta = count < MIN_WINDOW ? delta * 2 : delta * 0.7; + } + Assumptions.assumeTrue(count >= MIN_WINDOW && count <= MAX_WINDOW, + "could not find a workable parity window for spatial." + table); + + // The oracle: every feature in the window, fetched once. Its size matching + // the pushed-down SQL count is itself a count-vs-read parity assertion. + List window = new ArrayList<>(count); + try (SimpleFeatureIterator it = + src.getFeatures(new Query(table, ECQL.toFilter(bboxCql(geomField, env)))).features()) { + while (it.hasNext()) { + window.add(it.next()); + } + } + assertThat(window).as("window fetch size vs pushed-down count").hasSize(count); + + Fixture fx = new Fixture(table, geomField, window, env, stringAttr); + fx.derive(numericAttr, stringAttr); + return fx; + } + + /** Derives every template value from the window contents, so the suite + * self-configures against whatever slice of the dataset is loaded. */ + private void derive(String numericAttr, String stringAttr) { + vars.put("G", geomField); + + // Geometry literals. PT is a real feature's coordinate (so equality and + // small-radius distance predicates are non-trivial); R1/R2 are overlapping + // sub-rectangles; TRI is a non-rectangle (exercises the non-rect SQL paths); + // LINE crosses the window diagonally through PT. + Coordinate pt = ((Geometry) window.get(0).getDefaultGeometry()).getCoordinate(); + double w = env.getWidth(), h = env.getHeight(); + double cx = env.centre().x, cy = env.centre().y; + Envelope r1 = new Envelope(cx - 0.35 * w, cx + 0.05 * w, cy - 0.35 * h, cy + 0.05 * h); + Envelope r2 = new Envelope(cx - 0.05 * w, cx + 0.35 * w, cy - 0.05 * h, cy + 0.35 * h); + vars.put("PT", String.format(Locale.ROOT, "POINT (%s %s)", pt.x, pt.y)); + vars.put("R1", polyWkt(r1)); + vars.put("R2", polyWkt(r2)); + vars.put("TRI", String.format(Locale.ROOT, + "POLYGON ((%1$s %3$s, %2$s %3$s, %1$s %4$s, %1$s %3$s))", + r1.getMinX(), r1.getMaxX(), r1.getMinY(), r1.getMaxY())); + vars.put("LINE", String.format(Locale.ROOT, "LINESTRING (%s %s, %s %s)", + env.getMinX(), env.getMinY(), env.getMaxX(), env.getMaxY())); + vars.put("WINDOW", bboxCql(geomField, env)); + + // Temporal boundaries: window dtg quantiles, truncated to whole seconds so + // the SQL literal and the in-memory Date are bit-identical. + List dtgs = window.stream() + .map(f -> (Date) f.getAttribute("dtg")) + .filter(Objects::nonNull) + .sorted() + .toList(); + assertThat(dtgs).as("dtg attribute must be populated").isNotEmpty(); + vars.put("T1", isoSecond(dtgs.get(dtgs.size() / 4))); + vars.put("T2", isoSecond(dtgs.get(dtgs.size() / 2))); + vars.put("T3", isoSecond(dtgs.get(3 * dtgs.size() / 4))); + + // Numeric attribute: the value of a real feature, plus quartile bounds. + vars.put("NATTR", numericAttr); + List nums = window.stream() + .map(f -> (Number) f.getAttribute(numericAttr)) + .filter(Objects::nonNull) + .map(Number::doubleValue) + .sorted() + .toList(); + assertThat(nums).as(numericAttr + " must be populated").isNotEmpty(); + vars.put("NVAL", trimNum(nums.get(nums.size() / 2))); + vars.put("NLO", trimNum(nums.get(nums.size() / 4))); + vars.put("NHI", trimNum(nums.get(3 * nums.size() / 4))); + + // String attribute (when the dataset has one): the most common simple + // value in the window, and a LIKE prefix derived from it. + if (stringAttr != null) { + Map freq = window.stream() + .map(f -> (String) f.getAttribute(stringAttr)) + .filter(v -> v != null && !v.isEmpty() && v.matches("[A-Za-z0-9 _.-]+")) + .collect(Collectors.groupingBy(v -> v, Collectors.counting())); + Assumptions.assumeTrue(!freq.isEmpty(), + stringAttr + " has no plain values in the window — skipping"); + String sval = freq.entrySet().stream() + .max(Map.Entry.comparingByValue()).orElseThrow().getKey(); + vars.put("SATTR", stringAttr); + vars.put("SVAL", sval); + vars.put("SPRE", sval.substring(0, Math.min(3, sval.length()))); + } + + // Feature IDs of real window features (start, middle, end). + vars.put("FID1", window.get(0).getID()); + vars.put("FID2", window.get(window.size() / 2).getID()); + vars.put("FID3", window.get(window.size() - 1).getID()); + } + + String fill(String template) { + String out = template; + for (Map.Entry e : vars.entrySet()) { + out = out.replace("${" + e.getKey() + "}", e.getValue()); + } + assertThat(out).as("unresolved placeholder in: " + out).doesNotContain("${"); + return out; + } + } + + // ── Ported filter cases ─────────────────────────────────────────────────── + + private List cases(Fixture fx) { + List tests = new ArrayList<>(); + tests.add(DynamicTest.dynamicTest("window count matches plain-catalog SQL", + () -> windowCountMatchesPlainSql(fx))); + + for (String cql : standardCases(fx)) { + tests.add(DynamicTest.dynamicTest(cql, () -> runStandard(fx, cql))); + } + for (DistanceCase dc : distanceCases(fx)) { + tests.add(DynamicTest.dynamicTest(dc.cql, () -> runDistance(fx, dc))); + } + return tests; + } + + /** The ported predicate set, adapted from the accumulo-datastore TestFilters + * lists: same categories and operator combinations, with the synthetic + * attr/val/geometry constants replaced by values derived from the window. */ + private static List standardCases(Fixture fx) { + List templates = new ArrayList<>(); + + // goodSpatialPredicates (+ the operators this datastore supports beyond them) + templates.addAll(List.of( + "INTERSECTS(${G}, ${R1})", + "INTERSECTS(${G}, ${TRI})", // non-rectangle: no CASE WHEN shortcut + "OVERLAPS(${G}, ${R1})", + "WITHIN(${G}, ${R1})", + "WITHIN(${G}, ${TRI})", + "CONTAINS(${G}, ${R1})", + "CONTAINS(${G}, ${PT})", + "CROSSES(${G}, ${R1})", + "TOUCHES(${G}, ${R1})", + "EQUALS(${G}, ${PT})", + "BBOX(${G}, ${BB1})", + "DISJOINT(${G}, ${R1})", + // literal-first operand order (the swapped path in visitBinarySpatialOperator) + "INTERSECTS(${R1}, ${G})", + "WITHIN(${R1}, ${G})", + "CONTAINS(${R1}, ${G})" + )); + + // andedSpatialPredicates / oredSpatialPredicates: the full op-pair matrices + List ops = List.of("INTERSECTS", "OVERLAPS", "WITHIN", "DISJOINT", "CROSSES"); + for (String a : ops) { + for (String b : ops) { + if (!a.equals(b)) { + templates.add(a + "(${G}, ${R1}) AND " + b + "(${G}, ${R2})"); + templates.add(a + "(${G}, ${R1}) OR " + b + "(${G}, ${R2})"); + } + } + } + + // simpleNotFilters + templates.addAll(List.of( + "NOT (INTERSECTS(${G}, ${R1}))", + "NOT (WITHIN(${G}, ${TRI}))", + "NOT (BBOX(${G}, ${BB1}))", + "NOT (${NATTR} = ${NVAL})", + "NOT (dtg DURING ${T1}/${T3})", + "NOT (dtg AFTER ${T2})" + )); + + // temporalPredicates + templates.addAll(List.of( + "dtg DURING ${T1}/${T2}", + "dtg DURING ${T2}/${T3}", + "dtg BEFORE ${T2}", + "dtg AFTER ${T2}", + "dtg BETWEEN '${T1}' AND '${T2}'", + "(not dtg after ${T2}) and (not dtg before ${T1})" + )); + + // spatioTemporalPredicates + templates.addAll(List.of( + "INTERSECTS(${G}, ${R1}) AND dtg DURING ${T1}/${T2}", + "WITHIN(${G}, ${R2}) AND dtg AFTER ${T2}" + )); + + // attributePredicates (numeric; every dataset has one) + templates.addAll(List.of( + "${NATTR} = ${NVAL}", + "${NATTR} <> ${NVAL}", + "${NATTR} < ${NHI}", + "${NATTR} > ${NLO}", + "${NATTR} <= ${NHI}", + "${NATTR} >= ${NLO}", + "${NATTR} BETWEEN ${NLO} AND ${NHI}", + "${NATTR} IN (${NLO}, ${NVAL}, ${NHI})" + )); + + // attributePredicates (string, where the dataset has a string column) + if (fx.stringAttr != null) { + templates.addAll(List.of( + "${SATTR} = '${SVAL}'", + "${SATTR} LIKE '${SPRE}%'", + "${SATTR} ILIKE '${SPRE}%'", + "${SATTR} ILIKE '%${SPRE}%'", + "${SATTR} ILIKE 'zzzzzz%'" // known miss, mirroring "ILIKE '1%'" + )); + } + + // attributeAndGeometricPredicates + templates.addAll(List.of( + "${NATTR} = ${NVAL} AND INTERSECTS(${G}, ${R1})", + "${NATTR} > ${NLO} AND WITHIN(${G}, ${TRI})", + "${NATTR} <= ${NHI} AND BBOX(${G}, ${BB1})", + "INTERSECTS(${G}, ${R1}) AND ${NATTR} BETWEEN ${NLO} AND ${NHI}" + )); + if (fx.stringAttr != null) { + templates.addAll(List.of( + "${SATTR} = '${SVAL}' AND INTERSECTS(${G}, ${R1})", + "${SATTR} ILIKE '${SPRE}%' AND WITHIN(${G}, ${R2})" + )); + } + + // andsOrsFilters / oneGeomFilters: nested and/or combinations + templates.addAll(List.of( + "((INTERSECTS(${G}, ${R1}) OR INTERSECTS(${G}, ${R2}))" + + " AND (dtg DURING ${T1}/${T3} OR ${NATTR} = ${NVAL}))", + "((dtg DURING ${T1}/${T3} AND INTERSECTS(${G}, ${R1}))" + + " OR (${NATTR} = ${NVAL} OR dtg DURING ${T2}/${T3}))", + "((${NATTR} = ${NVAL} AND dtg DURING ${T1}/${T2})" + + " AND (INTERSECTS(${G}, ${R2}) OR ${NATTR} > ${NHI} OR INTERSECTS(${G}, ${R1})))", + "(dtg DURING ${T1}/${T2} OR ${NATTR} = ${NVAL} OR dtg DURING ${T2}/${T3})", + "(INTERSECTS(${G}, ${R1}) OR dtg DURING ${T2}/${T3})", + "(INTERSECTS(${G}, ${R1}) AND dtg DURING ${T1}/${T3} AND dtg DURING ${T1}/${T2})" + )); + + // idPredicates + templates.addAll(List.of( + "IN('${FID1}','${FID2}')", + "IN('${FID3}')", + "IN('${FID2}','${FID3}') AND IN('${FID1}')", // contradiction: empty + "IN('${FID1}','${FID2}') AND dtg DURING ${T1}/${T3}", + "dtg DURING ${T1}/${T3} AND IN('${FID1}')", + "WITHIN(${G}, ${R1}) AND IN('${FID1}')", + "IN('${FID1}') AND BBOX(${G}, ${BB1})", + "IN('${FID1}','${FID2}') AND ${NATTR} >= ${NLO} AND WITHIN(${G}, ${WINPOLY})" + )); + + // Resolve the helper placeholders used above. + Envelope r1 = envOf(fx.vars.get("R1")); + fx.vars.put("BB1", String.format(Locale.ROOT, "%s, %s, %s, %s", + r1.getMinX(), r1.getMinY(), r1.getMaxX(), r1.getMaxY())); + fx.vars.put("WINPOLY", polyWkt(fx.env)); + + return templates.stream().map(fx::fill).toList(); + } + + /** DWITHIN/BEYOND, kept structural so the oracle can compute spherical + * distances to the same reference geometry. Radii scale with the window; + * the extended-geometry references exercise the envelope-expansion path. */ + private record DistanceCase(String cql, String refWkt, double meters, boolean beyond) {} + + private static List distanceCases(Fixture fx) { + String pt = fx.vars.get("PT"); + String line = fx.vars.get("LINE"); + String poly = fx.vars.get("R1"); + double windowMeters = haversine(fx.env.getMinY(), fx.env.getMinX(), + fx.env.getMinY(), fx.env.getMaxX()); + double small = Math.max(50, windowMeters / 10); + double medium = windowMeters / 2; + double large = windowMeters * 2; + List cases = new ArrayList<>(); + for (double r : new double[]{1.0, small, medium, large}) { + cases.add(new DistanceCase(dwithinCql(fx, pt, r), pt, r, false)); + } + cases.add(new DistanceCase(dwithinCql(fx, line, small), line, small, false)); + cases.add(new DistanceCase(dwithinCql(fx, poly, small), poly, small, false)); + cases.add(new DistanceCase( + String.format(Locale.ROOT, "BEYOND(%s, %s, %.1f, meters)", fx.geomField, pt, medium), + pt, medium, true)); + cases.add(new DistanceCase( + String.format(Locale.ROOT, "BEYOND(%s, %s, %.1f, meters)", fx.geomField, line, small), + line, small, true)); + return cases; + } + + private static String dwithinCql(Fixture fx, String refWkt, double meters) { + return String.format(Locale.ROOT, "DWITHIN(%s, %s, %.1f, meters)", + fx.geomField, refWkt, meters); + } + + // ── Execution ───────────────────────────────────────────────────────────── + + private static void runStandard(Fixture fx, String cql) throws Exception { + Filter filter = ECQL.toFilter(cql); + Set oracle = fx.window.stream() + .filter(filter::evaluate) + .map(SimpleFeature::getID) + .collect(Collectors.toSet()); + Set actual = queryFids(fx, cql); + assertParity(cql, actual, oracle, fx.window.size()); + } + + private static void runDistance(Fixture fx, DistanceCase dc) throws Exception { + Geometry ref = new WKTReader().read(dc.refWkt()); + double tolerance = Math.max(dc.meters() * TOLERANCE_FRACTION, TOLERANCE_FLOOR_METERS); + + Set oracle = new HashSet<>(); + Set boundary = new HashSet<>(); + for (SimpleFeature f : fx.window) { + double d = sphericalDistance((Geometry) f.getDefaultGeometry(), ref); + if (Math.abs(d - dc.meters()) <= tolerance) { + boundary.add(f.getID()); + } else if (dc.beyond() ? d > dc.meters() : d <= dc.meters()) { + oracle.add(f.getID()); + } + } + // A fid can appear on both sides when duplicate fids exist (multiple rows per + // fid — real in GeoLife) with one row in range and another in the tolerance + // band; drop band-touching fids from BOTH sides so they can't flap the result. + oracle.removeAll(boundary); + Set actual = queryFids(fx, dc.cql()); + actual.removeAll(boundary); + assertParity(dc.cql(), actual, oracle, fx.window.size()); + } + + /** Runs {@code cql AND } through the datastore, returning FIDs. */ + private static Set queryFids(Fixture fx, String cql) throws Exception { + String bounded = "(" + cql + ") AND " + fx.vars.get("WINDOW"); + Set fids = new HashSet<>(); + try (SimpleFeatureIterator it = ds.getFeatureSource(fx.table) + .getFeatures(new Query(fx.table, ECQL.toFilter(bounded))).features()) { + while (it.hasNext()) { + fids.add(it.next().getID()); + } + } + return fids; + } + + private static void assertParity(String cql, Set actual, Set oracle, int windowSize) { + Set missing = new HashSet<>(oracle); + missing.removeAll(actual); + Set extra = new HashSet<>(actual); + extra.removeAll(oracle); + assertThat(actual) + .as("datastore results vs in-memory oracle for [%s] (window=%d, oracle=%d, actual=%d;" + + " missing=%s extra=%s)", + cql, windowSize, oracle.size(), actual.size(), + sample(missing), sample(extra)) + .isEqualTo(oracle); + } + + private static String sample(Set fids) { + return fids.stream().limit(5).collect(Collectors.joining(",", "[", fids.size() > 5 ? ",…]" : "]")); + } + + /** Cross-checks the window feature count against a plain SQL count through the + * un-wrapped iceberg catalog, independently of everything this plugin adds. + * Skips when the plain catalog is not exposed (hardened deployments). */ + private static void windowCountMatchesPlainSql(Fixture fx) throws Exception { + // Bounds MUST be DOUBLE literals: Trino parses a bare 17-digit decimal as + // DECIMAL, and the DECIMAL→DOUBLE coercion can land one ulp away from the + // Java double the window was computed with (observed: 39.067550000000004 + // as DECIMAL coerces one ulp BELOW the Java double, admitting a boundary + // row the window correctly excludes). + String sql = String.format(Locale.ROOT, + "SELECT count(*) FROM iceberg.spatial.%s WHERE" + + " ST_X(ST_GeomFromBinary(%s)) BETWEEN DOUBLE '%s' AND DOUBLE '%s'" + + " AND ST_Y(ST_GeomFromBinary(%s)) BETWEEN DOUBLE '%s' AND DOUBLE '%s'", + fx.table, + fx.geomField, fx.env.getMinX(), fx.env.getMaxX(), + fx.geomField, fx.env.getMinY(), fx.env.getMaxY()); + long plain; + try (var conn = java.sql.DriverManager.getConnection( + "jdbc:trino://localhost:8080/iceberg?user=geomesa"); + var st = conn.createStatement(); + var rs = st.executeQuery(sql)) { + rs.next(); + plain = rs.getLong(1); + } catch (java.sql.SQLException e) { + Assumptions.assumeTrue(false, "plain iceberg catalog not exposed — skipping cross-check"); + return; + } + assertThat(fx.window.size()) + .as("window size vs plain-catalog SQL count (env=%s, table=%s)", fx.env, fx.table) + .isEqualTo((int) plain); + } + + // ── Helpers ─────────────────────────────────────────────────────────────── + + private static double median(List sorted) { + return sorted.get(sorted.size() / 2); + } + + private static String bboxCql(String geomField, Envelope e) { + return String.format(Locale.ROOT, "BBOX(%s, %s, %s, %s, %s)", + geomField, e.getMinX(), e.getMinY(), e.getMaxX(), e.getMaxY()); + } + + private static String polyWkt(Envelope e) { + return String.format(Locale.ROOT, + "POLYGON ((%1$s %3$s, %2$s %3$s, %2$s %4$s, %1$s %4$s, %1$s %3$s))", + e.getMinX(), e.getMaxX(), e.getMinY(), e.getMaxY()); + } + + /** Envelope of a rectangle WKT emitted by {@link #polyWkt}. */ + private static Envelope envOf(String rectWkt) { + try { + return new WKTReader().read(rectWkt).getEnvelopeInternal(); + } catch (org.locationtech.jts.io.ParseException e) { + throw new IllegalArgumentException(rectWkt, e); + } + } + + /** Whole-second ISO instant, so CQL parsing and SQL literals agree exactly. */ + private static String isoSecond(Date d) { + return Instant.ofEpochSecond(d.getTime() / 1000).toString(); + } + + /** Renders a numeric template value, dropping a trailing ".0" for integers. */ + private static String trimNum(double v) { + return v == Math.rint(v) && !Double.isInfinite(v) + ? String.valueOf((long) v) : String.valueOf(v); + } + + /** Great-circle distance (m) between a feature geometry and a reference geometry: + * nearest pair in coordinate space, then haversine between them. Exact for point + * references; a close approximation for lines/polygons (the tolerance band in + * runDistance absorbs the difference). */ + private static double sphericalDistance(Geometry a, Geometry b) { + Coordinate[] nearest = DistanceOp.nearestPoints(a, b); + return haversine(nearest[0].y, nearest[0].x, nearest[1].y, nearest[1].x); + } + + private static double haversine(double lat1, double lon1, double lat2, double lon2) { + double p1 = Math.toRadians(lat1), p2 = Math.toRadians(lat2); + double dp = Math.toRadians(lat2 - lat1), dl = Math.toRadians(lon2 - lon1); + double a = Math.sin(dp / 2) * Math.sin(dp / 2) + + Math.cos(p1) * Math.cos(p2) * Math.sin(dl / 2) * Math.sin(dl / 2); + return 2 * EARTH_RADIUS_METERS * Math.asin(Math.sqrt(a)); + } +} diff --git a/geomesa-trino/geomesa-trino-datastore/src/test/java/org/locationtech/geomesa/trino/datastore/TrinoFilterToSQLTest.java b/geomesa-trino/geomesa-trino-datastore/src/test/java/org/locationtech/geomesa/trino/datastore/TrinoFilterToSQLTest.java index 95534356e4f..6de8265f86c 100644 --- a/geomesa-trino/geomesa-trino-datastore/src/test/java/org/locationtech/geomesa/trino/datastore/TrinoFilterToSQLTest.java +++ b/geomesa-trino/geomesa-trino-datastore/src/test/java/org/locationtech/geomesa/trino/datastore/TrinoFilterToSQLTest.java @@ -16,13 +16,23 @@ import org.geotools.filter.text.ecql.ECQL; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.Point; -import org.locationtech.jts.geom.Polygon; +import org.locationtech.jts.io.WKTReader; import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; +/** + * The translator must emit PURE row-level ST_* predicates: no bbox-struct + * comparisons, no CASE WHEN shortcuts. All bbox/Z2/XZ2 pushdown is derived + * connector-side ({@code SpatialConnectorMetadata.applyFilter}) from exactly the + * {@code ST_*(ST_GeomFromBinary(col), ST_GeometryFromText('…'))} shape asserted + * here, so datastore queries and hand-written Trino SQL get identical plans. + * Result-set semantics are covered end-to-end by the filter-parity suite + * ({@code FilterParityIT}). + */ class TrinoFilterToSQLTest { private TrinoFilterToSQL translator; @@ -34,124 +44,167 @@ void setUp() { ff = CommonFactoryFinder.getFilterFactory(); } + /** The composite-SQL shapes this translator must never emit again. */ + private static void assertPure(String sql) { + assertThat(sql).doesNotContain("_bbox__"); + assertThat(sql).doesNotContain("CASE WHEN"); + assertThat(sql).doesNotContain("THEN TRUE"); + } + + // ── Intersects / BBOX ───────────────────────────────────────────────────── + @Test - void bboxTranslatesToBboxStructColumns() throws Exception { - // BBOX routes through the rectangle-intersects emission: bbox-overlap prefilter - // (pushable), shrunk-contained shortcut, exact ST_Intersects fallback. A bare - // float32 bbox-overlap would admit rows up to ½ ulp outside the envelope (the - // stored bbox is rounded to nearest) — caught by the filter-parity suite. + void bboxTranslatesToPureStIntersectsOfEnvelopeRectangle() throws Exception { + // BBOX(geom, env) ⇔ INTERSECTS(geom, envelope-rectangle). The exact row-level + // test also keeps the predicate correct against float32-rounded stored bboxes + // (a bare bbox-overlap admits rows up to ½ ulp outside the envelope). Filter f = ff.bbox("geom", -80.0, 37.0, -70.0, 45.0, "EPSG:4326"); String sql = translator.encodeToString(f); - assertThat(sql).startsWith( - "(\"__geom_bbox__\".xmax >= -80.0 AND \"__geom_bbox__\".xmin <= -70.0" + - " AND \"__geom_bbox__\".ymax >= 37.0 AND \"__geom_bbox__\".ymin <= 45.0"); - assertThat(sql).contains(") AND CASE WHEN \"__geom_bbox__\".xmin >= "); - assertThat(sql).contains(" THEN TRUE ELSE ST_Intersects(ST_GeomFromBinary(\"geom\"),"); + assertPure(sql); + assertThat(sql).startsWith("ST_Intersects(ST_GeomFromBinary(\"geom\"), ST_GeometryFromText('POLYGON"); + Envelope env = firstGeometryLiteral(sql).getEnvelopeInternal(); + assertThat(env).isEqualTo(new Envelope(-80.0, -70.0, 37.0, 45.0)); } @Test - void intersectsTranslatesToBboxOverlapAndCaseWhenContainedShortcut() throws Exception { - Filter f = ECQL.toFilter( - "INTERSECTS(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))"); - String sql = translator.encodeToString(f); - // Two-part predicate: - // 1. bbox-overlap (pushable) — necessary for ST_Intersects=TRUE; SI's - // tryExtractBboxEnvelope reads this shape and pushes Z2 file-level pruning. - // 2. CASE WHEN bbox-contained — sufficient for ST_Intersects=TRUE; survives - // Trino's optimizer and short-circuits the WKB decode + intersect test. - assertThat(sql).startsWith("(\"__geom_bbox__\".xmax >= -80.0"); - assertThat(sql).contains("\"__geom_bbox__\".xmin <= -70.0"); - assertThat(sql).contains("\"__geom_bbox__\".ymax >= 37.0"); - assertThat(sql).contains("\"__geom_bbox__\".ymin <= 45.0"); - // The contained shortcut uses bounds shrunk two float ulps inside the query - // rectangle (the stored bbox is float32, rounded to nearest). - assertThat(sql).contains(") AND CASE WHEN \"__geom_bbox__\".xmin >= "); - String shortcut = sql.substring(sql.indexOf("CASE WHEN")); - assertThat(extractBound(shortcut, "\"__geom_bbox__\".xmin >= ")).isGreaterThan(-80.0); - assertThat(extractBound(shortcut, "\"__geom_bbox__\".xmax <= ")).isLessThan(-70.0); - assertThat(extractBound(shortcut, "\"__geom_bbox__\".ymin >= ")).isGreaterThan(37.0); - assertThat(extractBound(shortcut, "\"__geom_bbox__\".ymax <= ")).isLessThan(45.0); - assertThat(sql).contains("THEN TRUE"); - assertThat(sql).contains("ELSE ST_Intersects(ST_GeomFromBinary(\"geom\"),"); - assertThat(sql).contains("POLYGON"); - assertThat(sql).endsWith("END"); + void intersectsEmissionIsShapeAndSchemaIndependent() throws Exception { + // Historically rectangles, non-rectangles, and point-schema columns produced + // three different composite shapes. Pure emission is identical for all: the + // connector, not this translator, decides what to derive from the predicate. + String rect = "POLYGON ((-80 37, -70 37, -70 45, -80 45, -80 37))"; + String lShape = "POLYGON ((0 0, 10 0, 10 5, 5 5, 5 10, 0 10, 0 0))"; + for (String poly : new String[]{rect, lShape}) { + String sql = new TrinoFilterToSQL().encodeToString( + ECQL.toFilter("INTERSECTS(geom, " + poly + ")")); + assertPure(sql); + assertThat(sql).isEqualTo( + "ST_Intersects(ST_GeomFromBinary(\"geom\"), ST_GeometryFromText('" + poly + "'))"); + } + // Declaring a Point-typed schema must not change the emission either. + SimpleFeatureTypeBuilder b = new SimpleFeatureTypeBuilder(); + b.setName("test"); + b.add("geom", Point.class); + translator.setFeatureType(b.buildFeatureType()); + String sql = translator.encodeToString(ECQL.toFilter("INTERSECTS(geom, " + rect + ")")); + assertThat(sql).isEqualTo( + "ST_Intersects(ST_GeomFromBinary(\"geom\"), ST_GeometryFromText('" + rect + "'))"); } @Test - void withinRectangleUsesShrunkContainedShortcutWithExactStWithinFallback() throws Exception { - // Rectangular query polygon: bbox-overlap prefilter AND CASE WHEN contained-in- - // SHRUNK-rect THEN TRUE ELSE exact ST_Within. The shortcut rectangle must be - // strictly smaller than the query rectangle: WITHIN is boundary-exclusive, so a - // point ON the rectangle edge must fall through to (and fail) the exact test — - // the filter-parity suite caught boundary-snapped GPS points being wrongly - // included by the old inclusive bbox-contained equivalence. - Filter f = ECQL.toFilter( - "WITHIN(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))"); - String sql = translator.encodeToString(f); - assertThat(sql).startsWith("(\"__geom_bbox__\".xmax >= -80.0"); - assertThat(sql).contains("\"__geom_bbox__\".xmin <= -70.0"); - assertThat(sql).contains(") AND CASE WHEN \"__geom_bbox__\".xmin >= "); - assertThat(sql).contains(" THEN TRUE ELSE ST_Within(ST_GeomFromBinary(\"geom\"),"); - assertThat(sql).endsWith(" END"); - // The shortcut bounds sit strictly inside the query rectangle on every side. - String shortcut = sql.substring(sql.indexOf("CASE WHEN")); - assertThat(extractBound(shortcut, "\"__geom_bbox__\".xmin >= ")).isGreaterThan(-80.0); - assertThat(extractBound(shortcut, "\"__geom_bbox__\".xmax <= ")).isLessThan(-70.0); - assertThat(extractBound(shortcut, "\"__geom_bbox__\".ymin >= ")).isGreaterThan(37.0); - assertThat(extractBound(shortcut, "\"__geom_bbox__\".ymax <= ")).isLessThan(45.0); + void intersectsWithLiteralFirstIsHandled() throws Exception { + // Symmetric operator: literal-first form emits the identical column-first call. + String sql = translator.encodeToString(ECQL.toFilter( + "INTERSECTS(POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)), geom)")); + assertPure(sql); + assertThat(sql).startsWith("ST_Intersects(ST_GeomFromBinary(\"geom\"),"); } + // ── Within / Contains (asymmetric: swapped forms normalize column-first) ── + @Test - void withinNonRectangleTranslatesToBboxOverlapAndStWithin() throws Exception { - // Non-rectangular polygon: must keep ST_Within at row level (bbox⊆env(polygon) - // doesn't prove geom⊆polygon — the polygon is a strict subset of its envelope). - // Still emit bbox-overlap as a leading conjunct so SI can push Z2 partitioning. - Filter f = ECQL.toFilter( - "WITHIN(geom, POLYGON((-80 37, -70 37, -75 45, -80 37)))"); // triangle - String sql = translator.encodeToString(f); - assertThat(sql).startsWith("(\"__geom_bbox__\".xmax >= -80.0"); - assertThat(sql).contains(") AND ST_Within(ST_GeomFromBinary(\"geom\"),"); - assertThat(sql).contains("POLYGON"); + void withinTranslatesToPureStWithin() throws Exception { + String sql = translator.encodeToString(ECQL.toFilter( + "WITHIN(geom, POLYGON((-80 37, -70 37, -75 45, -80 37)))")); // triangle + assertPure(sql); + assertThat(sql).isEqualTo("ST_Within(ST_GeomFromBinary(\"geom\")," + + " ST_GeometryFromText('POLYGON ((-80 37, -70 37, -75 45, -80 37))'))"); } @Test - void dwithinTranslatesToOuterBboxOverlapAndCaseWhenInnerInscribedShortcut() throws Exception { - Filter f = ECQL.toFilter("DWITHIN(geom, POINT(-77.04 38.91), 100000, meters)"); - String sql = translator.encodeToString(f); - // Outer bbox-overlap prefilter (necessary): pushable to file-level pruning. - assertThat(sql).startsWith("(\"__geom_bbox__\".xmax >= "); - assertThat(sql).contains("\"__geom_bbox__\".xmin <= "); - assertThat(sql).contains("\"__geom_bbox__\".ymax >= "); - assertThat(sql).contains("\"__geom_bbox__\".ymin <= "); - // CASE WHEN inner-rectangle-contained shortcut: sufficient for distance ≤ d. - assertThat(sql).contains(") AND CASE WHEN \"__geom_bbox__\".xmin >= "); - assertThat(sql).contains("\"__geom_bbox__\".xmax <= "); - assertThat(sql).contains("\"__geom_bbox__\".ymin >= "); - assertThat(sql).contains("\"__geom_bbox__\".ymax <= "); - assertThat(sql).contains(" THEN TRUE"); - // Exact spherical distance is the ELSE branch — ST_Distance on spherical_geography - // measured in meters, with geom converted via ST_GeomFromBinary. - assertThat(sql).contains("ELSE ST_Distance(to_spherical_geography(ST_GeomFromBinary(\"geom\")),"); - assertThat(sql).contains("to_spherical_geography(ST_GeometryFromText("); + void withinLiteralFirstNormalizesToColumnFirstStContains() throws Exception { + // WITHIN(literal, geom) ⇔ CONTAINS(geom, literal) (OGC complement). Emitting + // the column-first form matters: the connector's recognizer expects + // ST_*(ST_GeomFromBinary(col), literal) and would skip pushdown otherwise. + String sql = translator.encodeToString(ECQL.toFilter( + "WITHIN(POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)), geom)")); + assertPure(sql); + assertThat(sql).startsWith("ST_Contains(ST_GeomFromBinary(\"geom\"),"); + } + + @Test + void containsPropertyFirstTranslatesToPureStContains() throws Exception { + String sql = translator.encodeToString( + ECQL.toFilter("CONTAINS(geom, POINT(-77.04 38.91))")); + assertPure(sql); + assertThat(sql).isEqualTo("ST_Contains(ST_GeomFromBinary(\"geom\")," + + " ST_GeometryFromText('POINT (-77.04 38.91)'))"); + } + + @Test + void containsLiteralFirstNormalizesToColumnFirstStWithin() throws Exception { + // CONTAINS(literal, geom) ⇔ WITHIN(geom, literal). + String sql = translator.encodeToString(ECQL.toFilter( + "CONTAINS(POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)), geom)")); + assertPure(sql); + assertThat(sql).startsWith("ST_Within(ST_GeomFromBinary(\"geom\"),"); + } + + // ── Other binary spatial operators ──────────────────────────────────────── + + @Test + void crossesTouchesOverlapsEqualsTranslateToPureStCalls() throws Exception { + record Case(String cql, String function) {} + var cases = java.util.List.of( + new Case("CROSSES(geom, LINESTRING(-80 37, -70 45))", "ST_Crosses"), + new Case("TOUCHES(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))", "ST_Touches"), + new Case("OVERLAPS(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))", "ST_Overlaps"), + new Case("EQUALS(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))", "ST_Equals")); + for (Case c : cases) { + String sql = new TrinoFilterToSQL().encodeToString(ECQL.toFilter(c.cql())); + assertPure(sql); + assertThat(sql).as(c.cql()) + .startsWith(c.function() + "(ST_GeomFromBinary(\"geom\"), ST_GeometryFromText("); + } + } + + @Test + void disjointTranslatesToPureStDisjoint() throws Exception { + // The connector deliberately derives NO pushdown from st_disjoint (overlap-based + // pruning would drop the answer set) — the pure call is all there is. + String sql = translator.encodeToString(ECQL.toFilter( + "DISJOINT(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))")); + assertPure(sql); + assertThat(sql).startsWith("ST_Disjoint(ST_GeomFromBinary(\"geom\"),"); + } + + // ── DWithin / Beyond ────────────────────────────────────────────────────── + // + // DWithin emits: ST_Intersects(geom, ) AND . The ST_Intersects conjunct is a NECESSARY condition + // (every geometry within d of the reference overlaps the outer envelope) in the + // exact shape the connector derives bbox/Z2 pruning from; the outer-envelope + // geometry math below (poleward-edge scaling, near-pole bands, envelope + // expansion for extended references) is asserted by parsing the emitted WKT. + + @Test + void dwithinEmitsOuterEnvelopeIntersectsAndExactSphericalDistance() throws Exception { + String sql = translator.encodeToString( + ECQL.toFilter("DWITHIN(geom, POINT(-77.04 38.91), 100000, meters)")); + assertPure(sql); + assertThat(sql).startsWith("ST_Intersects(ST_GeomFromBinary(\"geom\"), ST_GeometryFromText('POLYGON"); + assertThat(sql).contains(") AND ST_Distance(to_spherical_geography(ST_GeomFromBinary(\"geom\"))," + + " to_spherical_geography(ST_GeometryFromText("); assertThat(sql).contains("<= 100000"); - assertThat(sql).endsWith(" END"); + // The outer envelope covers the point ± ~1° lat (100 km × 1.1 margin). + Envelope outer = firstGeometryLiteral(sql).getEnvelopeInternal(); + assertThat(outer.getMinY()).isLessThan(38.91 - 0.9); + assertThat(outer.getMaxY()).isGreaterThan(38.91 + 0.9); + assertThat(outer.getMinX()).isLessThan(-77.04 - 0.9); + assertThat(outer.getMaxX()).isGreaterThan(-77.04 + 0.9); } @Test - void dwithinNearPoleUsesFullLongitudeBandAndSkipsInscribedShortcut() throws Exception { + void dwithinNearPoleUsesFullLongitudeBand() throws Exception { // lat 87 ≥ NEAR_POLE_LAT (85): the flat cos(lat) longitude scaling degenerates - // (→ division-by-zero at the pole, and a within-d region that wraps all longitudes). - // The outer prefilter must span every longitude so no matching row is dropped, and the - // inscribed-rectangle TRUE shortcut is skipped in favor of the exact distance check. - Filter f = ECQL.toFilter("DWITHIN(geom, POINT(10 87), 100000, meters)"); - String sql = translator.encodeToString(f); - // Full-longitude outer band: xmax >= -180 ... xmin <= 180 (no bounded, row-dropping span). - assertThat(sql).contains("\"__geom_bbox__\".xmax >= -180"); - assertThat(sql).contains("\"__geom_bbox__\".xmin <= 180"); - // No inscribed-rectangle shortcut near the pole — no false-positive TRUE branch. - assertThat(sql).doesNotContain("CASE WHEN"); - assertThat(sql).doesNotContain("THEN TRUE"); - // Exact spherical distance is ANDed directly as the sole row-level check. + // (→ division-by-zero at the pole, and a within-d region that wraps all + // longitudes). The outer envelope must span every longitude so the pruning + // conjunct never drops a matching row. + String sql = translator.encodeToString( + ECQL.toFilter("DWITHIN(geom, POINT(10 87), 100000, meters)")); + assertPure(sql); + Envelope outer = firstGeometryLiteral(sql).getEnvelopeInternal(); + assertThat(outer.getMinX()).isEqualTo(-180.0); + assertThat(outer.getMaxX()).isEqualTo(180.0); assertThat(sql).contains(") AND ST_Distance(to_spherical_geography(ST_GeomFromBinary(\"geom\")),"); assertThat(sql).contains("<= 100000"); } @@ -163,13 +216,11 @@ void dwithinHighLatitudeOuterBoxCoversTrueRegion() throws Exception { // Sizing the outer box with cos(center) yields a ±19.8° half-span while points // within d near the band edge reach ~±26.1° — silently excluded. The fix sizes // with cos(poleward edge); assert the emitted half-span covers the true extreme. - Filter f = ECQL.toFilter("DWITHIN(geom, POINT(10 60), 1000000, meters)"); - String sql = new TrinoFilterToSQL().encodeToString(f); - double minX = extractBound(sql, "\"__geom_bbox__\".xmax >= "); - double maxX = extractBound(sql, "\"__geom_bbox__\".xmin <= "); - // True requirement (spherical): d / (111,320 m/deg × cos(69.9°)) ≈ 26.1° - assertThat(10.0 - minX).as("west half-span").isGreaterThan(26.2); - assertThat(maxX - 10.0).as("east half-span").isGreaterThan(26.2); + String sql = new TrinoFilterToSQL().encodeToString( + ECQL.toFilter("DWITHIN(geom, POINT(10 60), 1000000, meters)")); + Envelope outer = firstGeometryLiteral(sql).getEnvelopeInternal(); + assertThat(10.0 - outer.getMinX()).as("west half-span").isGreaterThan(26.2); + assertThat(outer.getMaxX() - 10.0).as("east half-span").isGreaterThan(26.2); } @Test @@ -177,102 +228,68 @@ void dwithinBandReachingPoleUsesFullLongitudeBand() throws Exception { // Centered at 80° with d = 1000 km the band reaches ~89.9° — effectively at the // pole — so the region wraps all longitudes even though the CENTER is below the // near-pole gate. The gate must be evaluated at the band edge. - Filter f = ECQL.toFilter("DWITHIN(geom, POINT(10 80), 1000000, meters)"); - String sql = new TrinoFilterToSQL().encodeToString(f); - assertThat(sql).contains("\"__geom_bbox__\".xmax >= -180"); - assertThat(sql).contains("\"__geom_bbox__\".xmin <= 180"); - assertThat(sql).doesNotContain("CASE WHEN"); - } - - @Test - void dwithinInnerRectangleCornersNeverExceedDistance() throws Exception { - // The THEN TRUE shortcut is sound only if every point of the inner rectangle is - // within d of the reference. Sized with cos(center) the equatorward corners land - // beyond d at high latitude + large radius (lat 70°, d=1000 km → ~1.03 d); the - // fix sizes with cos(equatorward edge). Verify all four corners by haversine. - double lat = 70.0, lon = 10.0, d = 1_000_000; - Filter f = ECQL.toFilter("DWITHIN(geom, POINT(10 70), 1000000, meters)"); - String sql = new TrinoFilterToSQL().encodeToString(f); - // Inner rectangle bounds live in the CASE WHEN clause (xmin >= / xmax <= form). - String inner = sql.substring(sql.indexOf("CASE WHEN")); - double xMin = extractBound(inner, "\"__geom_bbox__\".xmin >= "); - double xMax = extractBound(inner, "\"__geom_bbox__\".xmax <= "); - double yMin = extractBound(inner, "\"__geom_bbox__\".ymin >= "); - double yMax = extractBound(inner, "\"__geom_bbox__\".ymax <= "); - for (double cy : new double[]{yMin, yMax}) { - for (double cx : new double[]{xMin, xMax}) { - assertThat(haversineMeters(lat, lon, cy, cx)) - .as("corner (%s, %s)", cx, cy) - .isLessThan(d); - } - } + String sql = new TrinoFilterToSQL().encodeToString( + ECQL.toFilter("DWITHIN(geom, POINT(10 80), 1000000, meters)")); + Envelope outer = firstGeometryLiteral(sql).getEnvelopeInternal(); + assertThat(outer.getMinX()).isEqualTo(-180.0); + assertThat(outer.getMaxX()).isEqualTo(180.0); } @Test - void dwithinLinestringOuterBoxCoversFarEndsAndSkipsInscribedShortcut() throws Exception { + void dwithinLinestringOuterBoxCoversFarEnds() throws Exception { // For an extended reference the within-d region surrounds the WHOLE geometry. A - // radius-d box around the envelope centroid (the old behavior) excludes rows near - // the far ends: for this ~470 km linestring and d = 1 km, a point 550 m from the - // (48, 27) endpoint sits ~230 km from the centroid — silently pruned. The outer - // box must be the reference envelope EXPANDED by d. - Filter f = ECQL.toFilter("DWITHIN(geom, LINESTRING(45 23, 48 27), 1000, meters)"); - String sql = translator.encodeToString(f); - double minX = extractBound(sql, "\"__geom_bbox__\".xmax >= "); - double maxX = extractBound(sql, "\"__geom_bbox__\".xmin <= "); - double minY = extractBound(sql, "\"__geom_bbox__\".ymax >= "); - double maxY = extractBound(sql, "\"__geom_bbox__\".ymin <= "); - assertThat(minX).as("west bound covers west endpoint + d").isLessThan(45.0); - assertThat(maxX).as("east bound covers east endpoint + d").isGreaterThan(48.0049); - assertThat(minY).as("south bound covers south endpoint + d").isLessThan(23.0); - assertThat(maxY).as("north bound covers north endpoint + d").isGreaterThan(27.0049); - // No inscribed-rectangle TRUE shortcut: it would sit on the envelope centroid, - // which need not lie ON the geometry, so containment proves nothing. - assertThat(sql).doesNotContain("CASE WHEN"); - // Exact spherical distance is the sole row check. Trino's spherical ST_Distance - // only accepts points, so the extended reference goes through the planar - // nearest-points pair first. + // radius-d box around the envelope centroid excludes rows near the far ends: + // for this ~470 km linestring and d = 1 km, a point 550 m from the (48, 27) + // endpoint sits ~230 km from the centroid — silently pruned. The outer box must + // be the reference envelope EXPANDED by d. + String sql = translator.encodeToString( + ECQL.toFilter("DWITHIN(geom, LINESTRING(45 23, 48 27), 1000, meters)")); + assertPure(sql); + Envelope outer = firstGeometryLiteral(sql).getEnvelopeInternal(); + assertThat(outer.getMinX()).as("west bound covers west endpoint + d").isLessThan(45.0); + assertThat(outer.getMaxX()).as("east bound covers east endpoint + d").isGreaterThan(48.0049); + assertThat(outer.getMinY()).as("south bound covers south endpoint + d").isLessThan(23.0); + assertThat(outer.getMaxY()).as("north bound covers north endpoint + d").isGreaterThan(27.0049); + // Trino's spherical ST_Distance only accepts points, so the extended reference + // goes through the planar nearest-points pair first. assertThat(sql).contains(") AND ST_Distance(to_spherical_geography(" + "geometry_nearest_points(ST_GeomFromBinary(\"geom\"), ST_GeometryFromText("); assertThat(sql).contains("LINESTRING (45 23, 48 27)"); } @Test - void dwithinPolygonReferenceUsesEnvelopeExpandedOuterBoxWithoutShortcut() throws Exception { - Filter f = ECQL.toFilter( - "DWITHIN(geom, POLYGON((45 23, 48 23, 48 27, 45 27, 45 23)), 1000, meters)"); - String sql = translator.encodeToString(f); - double minX = extractBound(sql, "\"__geom_bbox__\".xmax >= "); - double maxX = extractBound(sql, "\"__geom_bbox__\".xmin <= "); - assertThat(minX).isLessThan(45.0); - assertThat(maxX).isGreaterThan(48.0); - assertThat(sql).doesNotContain("CASE WHEN"); + void dwithinPolygonReferenceUsesEnvelopeExpandedOuterBox() throws Exception { + String sql = translator.encodeToString(ECQL.toFilter( + "DWITHIN(geom, POLYGON((45 23, 48 23, 48 27, 45 27, 45 23)), 1000, meters)")); + assertPure(sql); + Envelope outer = firstGeometryLiteral(sql).getEnvelopeInternal(); + assertThat(outer.getMinX()).isLessThan(45.0); + assertThat(outer.getMaxX()).isGreaterThan(48.0); assertThat(sql).contains("<= 1000"); } - /** First numeric bound following {@code marker} in the SQL. */ - private static double extractBound(String sql, String marker) { - int i = sql.indexOf(marker); - assertThat(i).as("marker present: " + marker).isGreaterThanOrEqualTo(0); - int start = i + marker.length(); - int end = start; - while (end < sql.length() && (Character.isDigit(sql.charAt(end)) - || sql.charAt(end) == '-' || sql.charAt(end) == '.' - || sql.charAt(end) == 'E')) { - end++; - } - return Double.parseDouble(sql.substring(start, end)); + @Test + void dwithinWithLiteralFirstIsHandled() throws Exception { + String sql = translator.encodeToString( + ECQL.toFilter("DWITHIN(POINT(-77.04 38.91), geom, 1000, meters)")); + assertPure(sql); + assertThat(sql).startsWith("ST_Intersects(ST_GeomFromBinary(\"geom\"),"); + assertThat(sql).contains("ST_Distance(to_spherical_geography(ST_GeomFromBinary(\"geom\")),"); } - /** Great-circle distance on the WGS84 mean sphere. */ - private static double haversineMeters(double lat1, double lon1, double lat2, double lon2) { - double r = 6_371_008.8; - double p1 = Math.toRadians(lat1), p2 = Math.toRadians(lat2); - double dp = Math.toRadians(lat2 - lat1), dl = Math.toRadians(lon2 - lon1); - double a = Math.sin(dp / 2) * Math.sin(dp / 2) - + Math.cos(p1) * Math.cos(p2) * Math.sin(dl / 2) * Math.sin(dl / 2); - return 2 * r * Math.asin(Math.sqrt(a)); + @Test + void beyondTranslatesToDistanceGreaterThanWithoutPrefilter() throws Exception { + // Beyond is DWithin's complement: exact spherical distance > d, and like + // Disjoint no prefilter (the matching rows are OUTSIDE the neighborhood). + String sql = translator.encodeToString( + ECQL.toFilter("BEYOND(geom, POINT(-77.04 38.91), 100000, meters)")); + assertPure(sql); + assertThat(sql).startsWith("ST_Distance(to_spherical_geography(ST_GeomFromBinary(\"geom\")),"); + assertThat(sql).contains("> 100000"); } + // ── Temporal ────────────────────────────────────────────────────────────── + @Test void temporalComparisonLiteralsAreTimestampTyped() throws Exception { // BEFORE/AFTER (and BETWEEN etc.) route Date literals through writeLiteral — @@ -296,6 +313,8 @@ void duringTranslatesToTimestampRange() throws Exception { assertThat(sql).contains("\"dtg\" < TIMESTAMP '2024-01-01 00:00:00 UTC'"); } + // ── Composition / identifiers / feature ids ─────────────────────────────── + @Test void columnIdentifiersAreQuotedInEmittedSql() throws Exception { Filter spatial = ECQL.toFilter( @@ -336,209 +355,18 @@ void fidInTranslatesToFidColumnIn() throws Exception { assertThat(sql).contains("'def-456'"); } - // ── ST_Intersects rectangle + point fast path ───────────────────────────── - // - // For axis-aligned rectangle R and Point data: - // bbox(point) = point, so bbox-overlap(point, R) ⇔ point ∈ R ⇔ ST_Intersects. - // The CASE WHEN bbox-contained fallback in the general shortcut is dead code - // here — it always returns TRUE on the same rows bbox-overlap passes. Emit - // bbox-overlap alone and skip the per-row CASE evaluation. - // - // Both conditions are required: rectangle (so bbox-overlap is sufficient, - // not just necessary) AND point geometry (so bbox-overlap is necessary AND - // sufficient, not just necessary). - - @Test - void intersectsRectangleOnPointDataKeepsExactFallback() throws Exception { - // There is deliberately NO bbox-overlap-only fast path for point columns any - // more: bbox-overlap(point-bbox, rect) ⇔ intersects only holds in infinite - // precision — the stored bbox is float32 (rounded to nearest), so a point up - // to ½ ulp outside the rectangle can pass the inclusive overlap test. The - // filter-parity suite caught exactly that on real GPS data. Point columns take - // the same shrunk-shortcut + exact-ST_Intersects shape as everything else. - SimpleFeatureTypeBuilder b = new SimpleFeatureTypeBuilder(); - b.setName("test"); - b.add("geom", Point.class); - translator.setFeatureType(b.buildFeatureType()); - - Filter f = ECQL.toFilter( - "INTERSECTS(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))"); - String sql = translator.encodeToString(f); - - assertThat(sql).startsWith("(\"__geom_bbox__\".xmax >= -80.0"); - assertThat(sql).contains(" THEN TRUE ELSE ST_Intersects(ST_GeomFromBinary(\"geom\"),"); - } - - @Test - void intersectsRectangleOnPolygonDataFallsBackToCaseWhenShortcut() throws Exception { - // Non-point geometry column: bbox(g) is a strict superset of g (e.g., a - // diagonal line's bbox can overlap a query rectangle the line misses). - // bbox-overlap-only is unsound; keep the CASE WHEN shortcut with row-level - // ST_Intersects fallback. - SimpleFeatureTypeBuilder b = new SimpleFeatureTypeBuilder(); - b.setName("test"); - b.add("geom", Polygon.class); - translator.setFeatureType(b.buildFeatureType()); - - Filter f = ECQL.toFilter( - "INTERSECTS(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))"); - String sql = translator.encodeToString(f); - assertThat(sql).contains("CASE WHEN"); - assertThat(sql).contains("ELSE ST_Intersects(ST_GeomFromBinary(\"geom\"),"); - } - - @Test - void intersectsNonRectangleEmitsExactStIntersectsWithoutShortcut() throws Exception { - // L-shaped query polygon: a row whose bbox sits inside the ENVELOPE but in the - // L's notch does NOT intersect the polygon, so the bbox-contained THEN TRUE - // shortcut is unsound for any non-rectangular query geometry. Expect just the - // pushable bbox-overlap prefilter AND the exact row-level ST_Intersects. - SimpleFeatureTypeBuilder b = new SimpleFeatureTypeBuilder(); - b.setName("test"); - b.add("geom", Point.class); - translator.setFeatureType(b.buildFeatureType()); - - Filter f = ECQL.toFilter( - "INTERSECTS(geom, POLYGON((0 0, 10 0, 10 5, 5 5, 5 10, 0 10, 0 0)))"); - String sql = translator.encodeToString(f); - assertThat(sql).doesNotContain("CASE WHEN"); - assertThat(sql).doesNotContain("THEN TRUE"); - assertThat(sql).contains(") AND ST_Intersects(ST_GeomFromBinary(\"geom\"),"); - } - - // ── Operand order: literal-first spatial filters ────────────────────────── - // - // CQL permits the geometry literal on either side (e.g. INTERSECTS(POLYGON, geom)). - // Intersects/DWithin are symmetric; Within is not — WITHIN(literal, geom) asks - // whether the literal is contained in the row geometry. - - @Test - void intersectsWithLiteralFirstIsHandled() throws Exception { - Filter f = ECQL.toFilter( - "INTERSECTS(POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)), geom)"); - String sql = translator.encodeToString(f); - // Same predicate as the property-first form: rectangle query → bbox-overlap - // AND CASE WHEN shortcut on the geom column's companions. - assertThat(sql).startsWith("(\"__geom_bbox__\".xmax >= -80.0"); - assertThat(sql).contains("CASE WHEN"); - assertThat(sql).contains("ELSE ST_Intersects(ST_GeomFromBinary(\"geom\"),"); - } - - @Test - void dwithinWithLiteralFirstIsHandled() throws Exception { - Filter f = ECQL.toFilter("DWITHIN(POINT(-77.04 38.91), geom, 1000, meters)"); - String sql = translator.encodeToString(f); - assertThat(sql).startsWith("(\"__geom_bbox__\".xmax >= "); - assertThat(sql).contains("ST_Distance(to_spherical_geography(ST_GeomFromBinary(\"geom\")),"); - } - - @Test - void withinWithLiteralFirstTestsLiteralContainedInRowGeometry() throws Exception { - // WITHIN(literal, geom): the literal within the row geometry — the REVERSE - // containment. Expect ST_Within(literal, geom) plus the bbox-COVERS prefilter - // (the row bbox must contain the literal's envelope). - Filter f = ECQL.toFilter( - "WITHIN(POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)), geom)"); - String sql = translator.encodeToString(f); - assertThat(sql).contains("\"__geom_bbox__\".xmin <= -80.0"); - assertThat(sql).contains("\"__geom_bbox__\".xmax >= -70.0"); - assertThat(sql).contains("\"__geom_bbox__\".ymin <= 37.0"); - assertThat(sql).contains("\"__geom_bbox__\".ymax >= 45.0"); - assertThat(sql).contains("AND ST_Within(ST_GeometryFromText("); - assertThat(sql).contains("ST_GeomFromBinary(\"geom\"))"); - } - - @Test - void intersectsRectangleWithoutFeatureTypeFallsBackToCaseWhenShortcut() throws Exception { - // GeoTools allows FilterToSQL without a FeatureType. If we can't prove - // the column is Point, we must keep the safe general shortcut form. - Filter f = ECQL.toFilter( - "INTERSECTS(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))"); - String sql = translator.encodeToString(f); - assertThat(sql).contains("CASE WHEN"); - assertThat(sql).contains("ELSE ST_Intersects(ST_GeomFromBinary(\"geom\"),"); - } - - // ── Other binary spatial operators ──────────────────────────────────────── - - @Test - void crossesTouchesOverlapsEqualsGetBboxPrefilterAndExactTest() throws Exception { - // Each of these implies a non-empty intersection, so the pushable - // bbox-overlap prefilter is a valid necessary condition; the exact ST_* - // runs at row level with no CASE WHEN shortcut. - record Case(String cql, String function) {} - var cases = java.util.List.of( - new Case("CROSSES(geom, LINESTRING(-80 37, -70 45))", "ST_Crosses"), - new Case("TOUCHES(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))", "ST_Touches"), - new Case("OVERLAPS(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))", "ST_Overlaps"), - new Case("EQUALS(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))", "ST_Equals")); - for (Case c : cases) { - String sql = new TrinoFilterToSQL().encodeToString(ECQL.toFilter(c.cql())); - assertThat(sql).as(c.cql()).startsWith("(\"__geom_bbox__\".xmax >= -80.0"); - assertThat(sql).as(c.cql()).contains(") AND " + c.function() + "(ST_GeomFromBinary(\"geom\"),"); - assertThat(sql).as(c.cql()).doesNotContain("CASE WHEN"); - } - } - - @Test - void disjointHasNoBboxPrefilter() throws Exception { - // Disjoint's result set is the complement of the overlap region — a bbox - // prefilter would prune exactly the rows that satisfy the predicate. - Filter f = ECQL.toFilter( - "DISJOINT(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))"); - String sql = translator.encodeToString(f); - assertThat(sql).startsWith("ST_Disjoint(ST_GeomFromBinary(\"geom\"),"); - assertThat(sql).doesNotContain("__geom_bbox__"); - } + // ── Helpers ─────────────────────────────────────────────────────────────── - @Test - void beyondTranslatesToDistanceGreaterThanWithoutPrefilter() throws Exception { - // Beyond is DWithin's complement: exact spherical distance > d, and like - // Disjoint no bbox prefilter (the matching rows are OUTSIDE the neighborhood). - Filter f = ECQL.toFilter("BEYOND(geom, POINT(-77.04 38.91), 100000, meters)"); - String sql = translator.encodeToString(f); - assertThat(sql).startsWith("ST_Distance(to_spherical_geography(ST_GeomFromBinary(\"geom\")),"); - assertThat(sql).contains("> 100000"); - assertThat(sql).doesNotContain("__geom_bbox__"); - } - - @Test - void containsPropertyFirstGetsBboxCoversPrefilterAndExactTest() throws Exception { - // CONTAINS(geom, literal): the row geometry contains the literal, so the row - // bbox must COVER the literal's envelope (necessary, pushable) + exact test. - Filter f = ECQL.toFilter("CONTAINS(geom, POINT(-77.04 38.91))"); - String sql = translator.encodeToString(f); - // Bounds are float32-aligned in the admitting direction: the stored bbox is - // nearest-rounded float32, so raw double bounds can drop covering rows whose - // stored values rounded across them (see TrinoFilterToSQL.bboxOverlapSql). - assertThat(sql).contains("\"__geom_bbox__\".xmin <= -77.03999328613281"); - assertThat(sql).contains("\"__geom_bbox__\".xmax >= -77.04000091552734"); - assertThat(sql).contains("\"__geom_bbox__\".ymin <= 38.910003662109375"); - assertThat(sql).contains("\"__geom_bbox__\".ymax >= 38.90999984741211"); - assertThat(sql).contains("AND ST_Contains(ST_GeomFromBinary(\"geom\"),"); - } - - @Test - void containsLiteralFirstIsWithinReversed() throws Exception { - // CONTAINS(literal, geom) ⇔ WITHIN(geom, literal): a rectangular literal takes - // the Within rectangle path — shrunk-contained shortcut with an exact ST_Within - // fallback (never ST_Contains, and never a bare bbox equivalence). - Filter f = ECQL.toFilter( - "CONTAINS(POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)), geom)"); - String sql = translator.encodeToString(f); - assertThat(sql).contains(") AND CASE WHEN \"__geom_bbox__\".xmin >= "); - assertThat(sql).contains(" THEN TRUE ELSE ST_Within(ST_GeomFromBinary(\"geom\"),"); - assertThat(sql).doesNotContain("ST_Contains"); - } - - @Test - void attributeAndIntersectsJoinedWithAnd() throws Exception { - Filter f = ECQL.toFilter( - "active = TRUE AND value > 50.0 AND " + - "INTERSECTS(geom, POLYGON((-80 37, -70 37, -70 45, -80 45, -80 37)))"); - String sql = translator.encodeToString(f); - assertThat(sql).contains("active"); - assertThat(sql).contains("value"); - assertThat(sql).contains("ST_Intersects(ST_GeomFromBinary(\"geom\"),"); + /** The first {@code ST_GeometryFromText('…')} literal in the SQL, parsed. For the + * DWithin emission this is the outer-envelope rectangle (the reference geometry + * appears later, inside the distance expression). */ + private static org.locationtech.jts.geom.Geometry firstGeometryLiteral(String sql) throws Exception { + String marker = "ST_GeometryFromText('"; + int start = sql.indexOf(marker); + assertThat(start).as("geometry literal present").isGreaterThanOrEqualTo(0); + start += marker.length(); + int end = sql.indexOf("')", start); + assertThat(end).as("geometry literal terminated").isGreaterThan(start); + return new WKTReader().read(sql.substring(start, end)); } } From b974caa68eccba7103b2695f86f7ca7b8b29c1c8 Mon Sep 17 00:00:00 2001 From: Chris Dobbins Date: Tue, 21 Jul 2026 20:04:05 +0000 Subject: [PATCH 2/5] GEOMESA-3582 Added connector-configurable BBOX-containment short-circuiting logic to the plugin connector, which shortcuts the more expensive WKB decode during row-filtering, provides additional performance improvement for coarsely-pruned datasets. --- .../connector/BboxFilteringPageSource.java | 237 ++++++++++++++++++ .../iceberg/connector/SpatialConnector.java | 49 +++- .../connector/SpatialConnectorFactory.java | 38 ++- .../connector/SpatialConnectorMetadata.java | 230 ++++++++++++++--- .../connector/SpatialPageSourceProvider.java | 159 ++++++++++++ .../iceberg/connector/SpatialTableHandle.java | 80 ++++++ .../spatial/BboxShortCircuitParityIT.java | 119 +++++++++ .../BboxFilteringPageSourceTest.java | 225 +++++++++++++++++ .../connector/BboxShortCircuitTest.java | 88 +++++++ ...rMetadataShortCircuitPreservationTest.java | 104 ++++++++ ...patialConnectorSplitManagerGatingTest.java | 83 ++++++ 11 files changed, 1363 insertions(+), 49 deletions(-) create mode 100644 geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSource.java create mode 100644 geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialPageSourceProvider.java create mode 100644 geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialTableHandle.java create mode 100644 geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/BboxShortCircuitParityIT.java create mode 100644 geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSourceTest.java create mode 100644 geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxShortCircuitTest.java create mode 100644 geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadataShortCircuitPreservationTest.java create mode 100644 geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorSplitManagerGatingTest.java diff --git a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSource.java b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSource.java new file mode 100644 index 00000000000..46a93ee94bb --- /dev/null +++ b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSource.java @@ -0,0 +1,237 @@ +/*********************************************************************** + * Copyright (c) 2013-2025 General Atomics Integrated Intelligence, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Apache License, Version 2.0 + * which accompanies this distribution and is available at + * https://www.apache.org/licenses/LICENSE-2.0 + ***********************************************************************/ + +package org.locationtech.geomesa.trino.spatial.iceberg.connector; + +import io.airlift.slice.Slice; +import io.trino.spi.Page; +import io.trino.spi.block.Block; +import io.trino.spi.connector.ConnectorPageSource; +import io.trino.spi.connector.SourcePage; +import io.trino.spi.metrics.Metrics; +import io.trino.spi.type.RealType; +import io.trino.spi.type.VarbinaryType; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.io.WKBReader; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.List; +import java.util.OptionalLong; +import java.util.concurrent.CompletableFuture; +import java.util.function.ObjLongConsumer; + +/** + * Wraps the Iceberg page source with a sound, pre-decode bbox filter over a geometry's + * {@code __X_bbox__} sub-fields, which the connector injected as real domains and which ride to + * the worker in the table handle's unenforced predicate. + * Row Filtering: + *
    + *
  • bbox outside the outer (float32-outward-rounded) box ⇒ reject;
    • + *
  • bbox inside the inner (float32-inward-rounded) box ⇒ accept + * with no WKB decode — the row's geometry is provably inside the query rectangle;
    • + *
  • otherwise (the thin boundary shell, or a null bbox) ⇒ decode the geometry WKB and run + * the exact {@code intersects} test against the query rectangle.
    • + *
+ * The outward/inward float32 rounding of the two boxes guarantees every row that could be + * misclassified by the nearest-rounded stored bbox falls through to the exact test, so the + * result is identical to the engine's exact predicate. Bbox-short-circuit mode is only ever + * engaged for a rectangle {@code ST_Intersects} on a geometry column whose SFT subtype is + * {@code Point} (see {@link SpatialConnectorMetadata}). + * + *

Laziness. Only the cheap bbox columns are materialized to classify rows; the + * geometry column is fetched only when a shell row is present in a page, and decoded only at the + * shell positions. Accept and reject rows never touch the WKB. The bbox sub-field columns this + * page source added to the physical read (beyond what the query projected) are stripped from the + * returned page. + */ +final class BboxFilteringPageSource implements ConnectorPageSource { + + private static final Logger LOG = LoggerFactory.getLogger(BboxFilteringPageSource.class); + + /** One bbox-box bound: the row's real value in {@code channel} must lie within {@code [low, high]}. */ + record BboxBound(int channel, float low, float high) {} + + /** Bbox-short-circuit configuration. {@code outerBounds} are the reject bounds (ANY violated by a + * non-null bbox ⇒ reject); {@code innerBounds} are the containment bounds (ALL must hold ⇒ accept); + * {@code geomChannel} is the physical channel of the geometry WKB column; {@code queryRect} is the + * exact query rectangle for the shell test. */ + record ShortCircuitConfig(List outerBounds, + List innerBounds, + int geomChannel, + Geometry queryRect) {} + + private final ConnectorPageSource delegate; + private final ShortCircuitConfig config; + + /** Number of channels the query actually requested; any beyond this were added to read the + * bbox sub-fields and the geometry, and must be hidden from the engine. */ + private final int outputChannelCount; + private final boolean stripAddedChannels; + + BboxFilteringPageSource(ConnectorPageSource delegate, + int outputChannelCount, + boolean stripAddedChannels, + ShortCircuitConfig config) { + this.delegate = delegate; + this.outputChannelCount = outputChannelCount; + this.stripAddedChannels = stripAddedChannels; + this.config = config; + } + + @Override + public SourcePage getNextSourcePage() { + SourcePage page = delegate.getNextSourcePage(); + if (page == null) { + return null; + } + int positions = page.getPositionCount(); + + List outerBounds = config.outerBounds(); + List innerBounds = config.innerBounds(); + Block[] outerBlocks = materialize(page, outerBounds); + Block[] innerBlocks = materialize(page, innerBounds); + Block geomBlock = null; // geometry column + reader fetched lazily, only if a shell row appears + WKBReader reader = null; + + int[] retained = new int[positions]; + int kept = 0; + for (int p = 0; p < positions; p++) { + if (rejected(p, outerBlocks, outerBounds)) { + continue; // provably outside the envelope → drop + } + if (accepted(p, innerBlocks, innerBounds)) { + retained[kept++] = p; + continue; // provably inside the envelope → accept with no WKB decode + } + // Boundary shell (or null bbox): decode WKB and run the exact test. + if (geomBlock == null) { + geomBlock = page.getBlock(config.geomChannel()); + reader = new WKBReader(); + } + if (shellMatches(geomBlock, p, reader, config.queryRect())) { + retained[kept++] = p; + } + } + + if (kept < positions) { + page.selectPositions(retained, 0, kept); + } + return stripAddedChannels ? new ChannelPrefixSourcePage(page, outputChannelCount) : page; + } + + private static Block[] materialize(SourcePage page, List bounds) { + Block[] blocks = new Block[bounds.size()]; + for (int i = 0; i < bounds.size(); i++) { + blocks[i] = page.getBlock(bounds.get(i).channel()); + } + return blocks; + } + + /** True iff the row is provably outside the query envelope — any outer bound is violated by a + * non-null bbox value. A null bbox value cannot prove non-overlap, so it never rejects. */ + private static boolean rejected(int position, Block[] blocks, List bounds) { + for (int i = 0; i < bounds.size(); i++) { + Block b = blocks[i]; + if (b.isNull(position)) { + continue; + } + float v = RealType.REAL.getFloat(b, position); + BboxBound bound = bounds.get(i); + if (v < bound.low() || v > bound.high()) { + return true; + } + } + return false; + } + + /** True iff the row's bbox is provably inside the (inward-rounded) query rectangle — every inner + * containment bound holds. A null bbox value cannot prove containment, so it is NOT accepted + * (it falls through to the exact test). */ + private static boolean accepted(int position, Block[] blocks, List bounds) { + for (int i = 0; i < bounds.size(); i++) { + Block b = blocks[i]; + if (b.isNull(position)) { + return false; + } + float v = RealType.REAL.getFloat(b, position); + BboxBound bound = bounds.get(i); + if (v < bound.low() || v > bound.high()) { + return false; + } + } + return true; + } + + /** Exact test for a boundary-shell row: decode the geometry WKB and test intersection against + * the query rectangle. A null or undecodable geometry cannot match, so it is dropped (this + * mirrors the engine excluding a row whose {@code ST_GeomFromBinary} yields null). */ + private boolean shellMatches(Block geomBlock, int position, WKBReader reader, Geometry queryRect) { + if (geomBlock.isNull(position)) { + return false; + } + Slice slice = VarbinaryType.VARBINARY.getSlice(geomBlock, position); + try { + Geometry g = reader.read(slice.getBytes()); + return g != null && !g.isEmpty() && g.intersects(queryRect); + } catch (Exception e) { + LOG.warn("Dropping row with undecodable geometry WKB during bbox bbox-short-circuit: {}", e.toString()); + return false; + } + } + + /** + * Pure delegation + */ + @Override public long getCompletedBytes() { return delegate.getCompletedBytes(); } + @Override public OptionalLong getCompletedPositions() { return delegate.getCompletedPositions(); } + @Override public long getReadTimeNanos() { return delegate.getReadTimeNanos(); } + @Override public boolean isFinished() { return delegate.isFinished(); } + @Override public long getMemoryUsage() { return delegate.getMemoryUsage(); } + @Override public void close() throws IOException { delegate.close(); } + @Override public CompletableFuture isBlocked() { return delegate.isBlocked(); } + @Override public Metrics getMetrics() { return delegate.getMetrics(); } + + /** A {@link SourcePage} view exposing only the first {@code channelCount} channels of a + * delegate, hiding the bbox/geometry columns this page source added to the physical read. */ + private static final class ChannelPrefixSourcePage implements SourcePage { + private final SourcePage delegate; + private final int channelCount; + + ChannelPrefixSourcePage(SourcePage delegate, int channelCount) { + this.delegate = delegate; + this.channelCount = channelCount; + } + + @Override public int getPositionCount() { return delegate.getPositionCount(); } + @Override public long getSizeInBytes() { return delegate.getSizeInBytes(); } + @Override public long getRetainedSizeInBytes() { return delegate.getRetainedSizeInBytes(); } + @Override public void retainedBytesForEachPart(ObjLongConsumer consumer) { + delegate.retainedBytesForEachPart(consumer); + } + @Override public int getChannelCount() { return channelCount; } + @Override public Block getBlock(int channel) { + if (channel >= channelCount) { + throw new IndexOutOfBoundsException("channel " + channel + " >= " + channelCount); + } + return delegate.getBlock(channel); + } + @Override public Page getPage() { + int[] channels = new int[channelCount]; + for (int i = 0; i < channelCount; i++) { + channels[i] = i; + } + return delegate.getColumns(channels); + } + @Override public Page getColumns(int[] channels) { return delegate.getColumns(channels); } + @Override public void selectPositions(int[] positions, int offset, int size) { + delegate.selectPositions(positions, offset, size); + } + } +} diff --git a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnector.java b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnector.java index e45e4f80f8b..58f814221fb 100644 --- a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnector.java +++ b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnector.java @@ -34,6 +34,7 @@ public class SpatialConnector implements Connector { private final Connector delegate; private final GeoMesaColumnCatalog geomCatalog; private final ConnectorAccessControl accessControl; + private final boolean bboxShortCircuit; /** * Wraps a delegate connector with no Trino-layer visibility enforcement. @@ -41,7 +42,7 @@ public class SpatialConnector implements Connector { * @param delegate the underlying iceberg connector */ public SpatialConnector(Connector delegate) { - this(delegate, null, null); + this(delegate, null, null, true); } /** @@ -55,10 +56,26 @@ public SpatialConnector(Connector delegate) { * access control delegates to Iceberg (no extra filter). */ public SpatialConnector(Connector delegate, String catalogName, AuthorizationResolver resolver) { + this(delegate, catalogName, resolver, true); + } + + /** + * Wraps a delegate connector, optionally installing Trino-layer row-visibility enforcement + * and the page-source bbox cheap-reject. + * + * @param delegate the underlying iceberg connector + * @param catalogName the Trino catalog name; may be null when no resolver. + * @param resolver identity→auths resolver; null disables Trino-layer enforcement. + * @param bboxShortCircuit when true, wrap the page source with the bbox cheap-reject + * ({@link SpatialPageSourceProvider}). + */ + public SpatialConnector(Connector delegate, String catalogName, AuthorizationResolver resolver, + boolean bboxShortCircuit) { this.delegate = delegate; this.geomCatalog = new GeoMesaColumnCatalog(); this.accessControl = resolver == null ? null : new VisibilityAccessControl(catalogName, geomCatalog, resolver); + this.bboxShortCircuit = bboxShortCircuit; } /** @@ -105,7 +122,10 @@ public void rollback(ConnectorTransactionHandle transactionHandle) { public ConnectorMetadata getMetadata(ConnectorSession session, ConnectorTransactionHandle transactionHandle) { return new SpatialConnectorMetadata( - delegate.getMetadata(session, transactionHandle), geomCatalog); + delegate.getMetadata(session, transactionHandle), + geomCatalog, + bboxShortCircuit + ); } /** @@ -115,7 +135,22 @@ public ConnectorMetadata getMetadata(ConnectorSession session, */ @Override public ConnectorSplitManager getSplitManager() { - return delegate.getSplitManager(); + ConnectorSplitManager splitManager = delegate.getSplitManager(); + if (!bboxShortCircuit) { + return splitManager; + } + // BBOX Short-Circuit: unwrap a SpatialTableHandle before Iceberg's split manager sees it; + // the pushed bbox/z2 domains ride in the unwrapped handle's predicate, so file/manifest + // pruning is unaffected. + return new ConnectorSplitManager() { + @Override + public ConnectorSplitSource getSplits(ConnectorTransactionHandle transaction, + ConnectorSession session, ConnectorTableHandle table, + DynamicFilter dynamicFilter, Constraint constraint) { + return splitManager.getSplits(transaction, session, SpatialTableHandle.unwrap(table), + dynamicFilter, constraint); + } + }; } /** @@ -125,7 +160,8 @@ public ConnectorSplitManager getSplitManager() { */ @Override public ConnectorPageSourceProvider getPageSourceProvider() { - return delegate.getPageSourceProvider(); + ConnectorPageSourceProvider provider = delegate.getPageSourceProvider(); + return bboxShortCircuit ? new SpatialPageSourceProvider(provider) : provider; } /** @@ -135,7 +171,10 @@ public ConnectorPageSourceProvider getPageSourceProvider() { */ @Override public ConnectorPageSourceProviderFactory getPageSourceProviderFactory() { - return delegate.getPageSourceProviderFactory(); + ConnectorPageSourceProviderFactory factory = delegate.getPageSourceProviderFactory(); + return bboxShortCircuit + ? () -> new SpatialPageSourceProvider(factory.createPageSourceProvider()) + : factory; } /** diff --git a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorFactory.java b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorFactory.java index a0539faad5c..70b98004548 100644 --- a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorFactory.java +++ b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorFactory.java @@ -40,6 +40,19 @@ public class SpatialConnectorFactory implements ConnectorFactory { private static final String AUTH_RESOLVER = SECURITY_PREFIX + "auth-resolver"; private static final String AUTH_MAPPING = SECURITY_PREFIX + "auth-mapping-file"; + + /** Enables connector-side bbox filtering (see {@link BboxFilteringPageSource}). + * - For a rectangle {@code ST_Intersects} on a Z2/point geometry column the connector claims + * the predicate as enforced and the page source filters: + * a) rows whose bbox is inside the rectangle are accepted (skips decoding their geometry WKB), + * b) rows outside are rejected + * c) an exact test runs only on the envelope boundary. + * - ON by default; + * - Disable via {@code geomesa.spatial.bbox-page-filter=false}. + */ + private static final String SPATIAL_PREFIX = "geomesa.spatial."; + private static final String BBOX_PAGE_FILTER = SPATIAL_PREFIX + "bbox-page-filter"; + /** Default constructor; instances are created by {@link SpatialIcebergPlugin}. */ public SpatialConnectorFactory() {} @@ -65,16 +78,17 @@ public String getName() { public Connector create(String catalogName, Map config, ConnectorContext context) { AuthorizationResolver resolver = buildResolver(config); + boolean bboxShortCircuit = Boolean.parseBoolean(config.getOrDefault(BBOX_PAGE_FILTER, "true")); - // Iceberg uses strict config validation; strip our keys so it doesn't - // reject them as unused. + // Iceberg uses strict config validation; strip our keys so it doesn't reject them as unused. Map icebergConfig = new LinkedHashMap<>(); - config.forEach((k, v) -> { if (!k.startsWith(SECURITY_PREFIX)) icebergConfig.put(k, v); }); + config.forEach((k, v) -> { + if (!k.startsWith(SECURITY_PREFIX) && !k.startsWith(SPATIAL_PREFIX)) icebergConfig.put(k, v); + }); - ConnectorFactory icebergFactory = - new IcebergPlugin().getConnectorFactories().iterator().next(); + ConnectorFactory icebergFactory = new IcebergPlugin().getConnectorFactories().iterator().next(); Connector icebergConnector = icebergFactory.create(catalogName, icebergConfig, context); - return new SpatialConnector(icebergConnector, catalogName, resolver); + return new SpatialConnector(icebergConnector, catalogName, resolver, bboxShortCircuit); } /** Builds the identity→auths resolver from catalog config, or null when no @@ -89,19 +103,17 @@ private static AuthorizationResolver buildResolver(Map config) { if ("file".equals(kind)) { String path = config.get(AUTH_MAPPING); if (path == null) { - throw new IllegalArgumentException( - AUTH_RESOLVER + "=file requires " + AUTH_MAPPING); + throw new IllegalArgumentException(AUTH_RESOLVER + "=file requires " + AUTH_MAPPING); } return new FileAuthorizationResolver(Path.of(path)); } - // External implementation named by fully-qualified class with a - // (Map) constructor. + // External implementation named by fully-qualified class with a (Map) constructor. try { - return (AuthorizationResolver) Class.forName(kind) - .getConstructor(Map.class).newInstance(config); + return (AuthorizationResolver) Class.forName(kind).getConstructor(Map.class).newInstance(config); } catch (ReflectiveOperationException e) { throw new IllegalArgumentException( - "Cannot load AuthorizationResolver '" + kind + "' named by " + AUTH_RESOLVER, e); + "Cannot load AuthorizationResolver '" + kind + "' named by " + AUTH_RESOLVER, e + ); } } } diff --git a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java index e3390483752..69f84bd4750 100644 --- a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java +++ b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java @@ -11,6 +11,8 @@ import io.airlift.slice.Slice; import io.airlift.slice.Slices; import io.trino.geospatial.serde.JtsGeometrySerde; +import io.trino.plugin.iceberg.IcebergColumnHandle; +import io.trino.plugin.iceberg.IcebergTableHandle; import io.trino.spi.connector.*; import io.trino.spi.expression.Call; import io.trino.spi.expression.ConnectorExpression; @@ -23,6 +25,7 @@ import static io.trino.spi.expression.StandardFunctions.GREATER_THAN_OR_EQUAL_OPERATOR_FUNCTION_NAME; import static io.trino.spi.expression.StandardFunctions.LESS_THAN_OR_EQUAL_OPERATOR_FUNCTION_NAME; import static io.trino.spi.expression.StandardFunctions.OR_FUNCTION_NAME; +import static io.trino.spi.type.StandardTypes.GEOMETRY; import io.trino.spi.predicate.*; import io.trino.spi.security.TrinoPrincipal; import io.trino.spi.statistics.TableStatistics; @@ -34,6 +37,7 @@ import org.locationtech.geomesa.trino.spatial.iceberg.BboxHandles; import org.locationtech.geomesa.trino.spatial.iceberg.GeoMesaColumnCatalog; import org.locationtech.geomesa.trino.spatial.GeometryColumn; +import org.locationtech.geomesa.trino.spatial.SpatialIndexKind; import org.locationtech.geomesa.trino.spatial.iceberg.SpatialPartitionHandle; import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.Geometry; @@ -82,9 +86,12 @@ public class SpatialConnectorMetadata implements ConnectorMetadata { // (see envelopeOf). Keyed by concrete Class so each lookup runs at most once. private static final Map, Method> GEOM_ENVELOPE_METHOD = new ConcurrentHashMap<>(); private static final Map, Method[]> ENVELOPE_BOUND_METHODS = new ConcurrentHashMap<>(); + private static final Map, Method> GEOM_ISRECTANGLE_METHOD = new ConcurrentHashMap<>(); private final ConnectorMetadata delegate; private final GeoMesaColumnCatalog geomCatalog; + /** When true, claim eligible rectangle ST_Intersects predicates enforced. */ + private final boolean bboxShortCircuit; /** Result of locating a spatial constraint in a constraint expression: the * query envelope(s), the spatial function's name (lowercased ASCII; {@code @@ -110,8 +117,15 @@ record BboxPatternMatch(Envelope envelope, String geomName) {} */ public SpatialConnectorMetadata(ConnectorMetadata delegate, GeoMesaColumnCatalog geomCatalog) { + this(delegate, geomCatalog, false); + } + + public SpatialConnectorMetadata(ConnectorMetadata delegate, + GeoMesaColumnCatalog geomCatalog, + boolean bboxShortCircuit) { this.delegate = delegate; this.geomCatalog = geomCatalog; + this.bboxShortCircuit = bboxShortCircuit; } /** Resolve the per-geom-column descriptor map for the given table handle. @@ -134,6 +148,12 @@ private Map geomsFor(ConnectorSession session, public Optional> applyFilter( ConnectorSession session, ConnectorTableHandle handle, Constraint constraint) { + // Only bboxShortCircuit ever wraps the handle- we already claimed the ST_ as enforced + // and wrapped this handle on a prior pass. + if (handle instanceof SpatialTableHandle) { + return Optional.empty(); + } + // Walk the filter expression for ALL ST_* spatial calls (not just the first). // Per-geom routing: each ST_* on column X uses X's bbox + partition companions. List matches = findAllSpatialMatches(constraint.getExpression()); @@ -158,18 +178,13 @@ public Optional> applyFilter( List injectedPartitions = new ArrayList<>(); for (SpatialMatch match : matches) { - GeometryColumn g = geoms.get(match.geomName()); - if (g == null) continue; // ST_* on a non-geom column → leave in residual + GeometryColumn geom = geoms.get(match.geomName()); + if (geom == null) continue; - // The four bbox sub-field domains are ANDed by the engine, so a - // multi-envelope (OR) match can only push their combined bounds. - // Partition ranges live in a single Domain and are pushed per - // envelope, preserving the union's selectivity. Envelope env = new Envelope(match.envelopes().get(0)); match.envelopes().forEach(env::expandToInclude); - // Bbox sub-field domains for per-file Parquet-stat pruning. - g.bbox().ifPresent(bbox -> { + geom.bbox().ifPresent(bbox -> { // Skip re-injection if the planner round-tripped this already. if (constraint.getSummary().getDomains().map(d -> d.containsKey(bbox.xmax())).orElse(false)) { return; @@ -182,7 +197,7 @@ public Optional> applyFilter( }); // Spatial-partition pushdown for manifest-list pruning. - g.partition().ifPresent(sp -> { + geom.partition().ifPresent(sp -> { List ranges = buildPartitionRanges(sp, match.envelopes()); if (!ranges.isEmpty()) { domains.merge(sp.column(), @@ -219,13 +234,50 @@ public Optional> applyFilter( TupleDomain cleanedRemaining = stripInjectedDomains(dr.getRemainingFilter(), injectedBboxes, injectedPartitions); - // orElse replicates the SPI default ("no claim" keeps the original); the - // ST_* predicate must stay row-level — the injected domains only prune. + ConnectorExpression remainingExpr = dr.getRemainingExpression().orElse(constraint.getExpression()); + ConnectorTableHandle resultHandle = dr.getHandle(); + + // BBOX Short-Circuit: A single rectangle ST_Intersects on a Z2 (point) geometry column. + // + // Claim the ST_ predicate ENFORCED (drop it) and carry the exact query rectangle to + // the worker in a SpatialTableHandle, where BboxFilteringPageSource does the exact filtering + // — accepting rows whose bbox is inside the rectangle WITHOUT decoding their geometry WKB. + // Conditions: + // (a) accept via the envelope requires query == its envelope, + // (b) the shell exact test (JTS intersects) matches Trino's ST_Intersects + // exactly for points; + // (c) z2 partitioning ⟺ point data. + // Every other predicate keeps the engine's exact residual. + if (bboxShortCircuit && matches.size() == 1 && resultHandle instanceof IcebergTableHandle ith) { + SpatialMatch m = matches.get(0); + GeometryColumn g = geoms.get(m.geomName()); + if ("st_intersects".equals(m.functionName()) + && g != null + && g.partition().map(p -> p.kind() == SpatialIndexKind.Z2).orElse(false) + && queryIsRectangle(constraint.getExpression())) { + IcebergColumnHandle geomHandle = geomColumnHandle(session, handle, m.geomName()); + if (geomHandle != null && g.bbox().isPresent()) { + BboxHandles bh = g.bbox().get(); + Envelope rect = m.envelopes().get(0); + resultHandle = new SpatialTableHandle(ith, geomHandle, + List.of(bh.xmin(), bh.ymin(), bh.xmax(), bh.ymax()), + rect.getMinX(), rect.getMinY(), rect.getMaxX(), rect.getMaxY()); + + // Drop ONLY the ST_Intersects call — the page source now enforces it. + // Other conjuncts (critically is_visible row-filter) MUST stay in the + // residual for the engine to apply. + remainingExpr = dropSpatialCall(remainingExpr); + LOG.atDebug() + .setMessage("bbox-short-circuit: claimed rectangle ST_Intersects enforced on {} (geom '{}')") + .addArgument(() -> delegate.getTableName(session, handle)) + .addArgument(m::geomName) + .log(); + } + } + } + return Optional.of(new ConstraintApplicationResult<>( - dr.getHandle(), - cleanedRemaining, - dr.getRemainingExpression().orElse(constraint.getExpression()), - dr.isPrecalculateStatistics())); + resultHandle, cleanedRemaining, remainingExpr, dr.isPrecalculateStatistics())); } /** @@ -425,6 +477,101 @@ private static Method lookupMethod(Class c, String name) { } } + /** True iff the query geometry of the single {@code st_intersects} call is an axis-aligned + * rectangle — the soundness precondition for envelope-based bbox-short-circuit (the query must equal + * its own envelope). Reflects {@code isRectangle()} across the plugin-classloader boundary, + * mirroring {@link #envelopeOf}. Fail-safe: false on any uncertainty, so a missed case simply + * falls back to the engine's exact residual, never a wrong result. */ + private boolean queryIsRectangle(ConnectorExpression expr) { + Call call = findSpatialCall(expr); + if (call == null) { + return false; + } + for (ConnectorExpression arg : call.getArguments()) { + if (arg instanceof Constant c && GEOMETRY.equals(c.getType().getBaseName())) { + return isRectangleGeom(c.getValue()); + } + if (arg instanceof Call wkt) { + String fn = wkt.getFunctionName().getName().toLowerCase(Locale.ROOT); + if ((fn.equals("st_geometryfromtext") || fn.equals("st_geomfromtext")) + && !wkt.getArguments().isEmpty() + && wkt.getArguments().get(0) instanceof Constant wc + && wc.getValue() instanceof Slice s) { + try { + return new WKTReader().read(s.toStringUtf8()).isRectangle(); + } catch (Exception e) { + return false; + } + } + } + } + return false; + } + + /** Returns the expression with the {@code st_intersects} call replaced by TRUE, + * preserving every other conjunct — most importantly the {@code is_visible} row-filter, + * which the engine must still apply. Only the spatial predicate is claimed enforced. */ + static ConnectorExpression dropSpatialCall(ConnectorExpression expr) { + if (expr instanceof Call call) { + if ("st_intersects".equals(call.getFunctionName().getName().toLowerCase(Locale.ROOT))) { + return Constant.TRUE; + } + if (AND_FUNCTION_NAME.equals(call.getFunctionName())) { + List args = new ArrayList<>(); + for (ConnectorExpression a : call.getArguments()) { + args.add(dropSpatialCall(a)); + } + return new Call(call.getType(), call.getFunctionName(), args); + } + } + return expr; + } + + /** Finds the first {@code st_intersects} call in the expression, descending through ANDs. */ + private static Call findSpatialCall(ConnectorExpression expr) { + if (!(expr instanceof Call call)) { + return null; + } + if ("st_intersects".equals(call.getFunctionName().getName().toLowerCase(Locale.ROOT))) { + return call; + } + if (AND_FUNCTION_NAME.equals(call.getFunctionName())) { + for (ConnectorExpression arg : call.getArguments()) { + Call found = findSpatialCall(arg); + if (found != null) { + return found; + } + } + } + return null; + } + + /** Reflect {@code isRectangle()} on a folded (foreign-classloader) Geometry constant, or read a + * serialized {@link Slice} via the bundled serde. Fail-safe: false on any error. */ + private static boolean isRectangleGeom(Object value) { + if (value == null) { + return false; + } + try { + if (value instanceof Slice slice) { + return JtsGeometrySerde.deserialize(slice).isRectangle(); + } + Method m = GEOM_ISRECTANGLE_METHOD.computeIfAbsent( + value.getClass(), c -> lookupMethod(c, "isRectangle")); + return (boolean) m.invoke(value); + } catch (Throwable e) { + return false; + } + } + + /** The geometry column's Iceberg handle (so the worker can read its WKB for the shell test), or + * null if it can't be resolved as an {@link IcebergColumnHandle}. */ + private IcebergColumnHandle geomColumnHandle(ConnectorSession session, + ConnectorTableHandle handle, String geomName) { + ColumnHandle ch = delegate.getColumnHandles(session, handle).get(geomName); + return ch instanceof IcebergColumnHandle ich ? ich : null; + } + /** Like {@link #tryExtractEnvelope} but also returns the function name and the * geom-column name extracted from the spatial-arg position. Convenience entry * point retained for test fixtures and as the seam for a future Trino-aware @@ -731,7 +878,7 @@ public ConnectorTableHandle getTableHandle(ConnectorSession session, SchemaTable @Override public ConnectorTableMetadata getTableMetadata(ConnectorSession session, ConnectorTableHandle table) { - return delegate.getTableMetadata(session, table); + return delegate.getTableMetadata(session, SpatialTableHandle.unwrap(table)); } /** @@ -743,7 +890,7 @@ public ConnectorTableMetadata getTableMetadata(ConnectorSession session, */ @Override public SchemaTableName getTableName(ConnectorSession session, ConnectorTableHandle table) { - return delegate.getTableName(session, table); + return delegate.getTableName(session, SpatialTableHandle.unwrap(table)); } /** @@ -756,7 +903,7 @@ public SchemaTableName getTableName(ConnectorSession session, ConnectorTableHand @Override public ConnectorTableProperties getTableProperties(ConnectorSession session, ConnectorTableHandle table) { - return delegate.getTableProperties(session, table); + return delegate.getTableProperties(session, SpatialTableHandle.unwrap(table)); } /** @@ -768,7 +915,7 @@ public ConnectorTableProperties getTableProperties(ConnectorSession session, */ @Override public Optional getInfo(ConnectorSession session, ConnectorTableHandle table) { - return delegate.getInfo(session, table); + return delegate.getInfo(session, SpatialTableHandle.unwrap(table)); } /** @@ -820,12 +967,12 @@ public Map getRelationTypes(ConnectorSession sess @Override public Map getColumnHandles(ConnectorSession session, ConnectorTableHandle tableHandle) { - Map handles = delegate.getColumnHandles(session, tableHandle); + Map handles = delegate.getColumnHandles(session, SpatialTableHandle.unwrap(tableHandle)); // Record the table's visibility column at analysis time so the Trino-layer // VisibilityAccessControl (which only receives a SchemaTableName, no session) // can read it warm. Best-effort: never let bookkeeping break column resolution. try { - geomCatalog.recordVisibilityColumn(delegate.getTableName(session, tableHandle), + geomCatalog.recordVisibilityColumn(delegate.getTableName(session, SpatialTableHandle.unwrap(tableHandle)), handles.keySet()); } catch (RuntimeException e) { LOG.debug("Could not record visibility column: {}", e.getMessage()); @@ -845,7 +992,7 @@ public Map getColumnHandles(ConnectorSession session, public ColumnMetadata getColumnMetadata(ConnectorSession session, ConnectorTableHandle tableHandle, ColumnHandle columnHandle) { - return delegate.getColumnMetadata(session, tableHandle, columnHandle); + return delegate.getColumnMetadata(session, SpatialTableHandle.unwrap(tableHandle), columnHandle); } /** @@ -916,7 +1063,12 @@ public Iterator streamRelationComments(ConnectorSession public Optional> applyProjection( ConnectorSession session, ConnectorTableHandle handle, List projections, Map assignments) { - return delegate.applyProjection(session, handle, projections, assignments); + // Preserve the cheap-accept wrapper: the delegate's result would carry the unwrapped Iceberg + // handle, silently replacing our SpatialTableHandle and disabling the page-source filter. + if (handle instanceof SpatialTableHandle) { + return Optional.empty(); + } + return delegate.applyProjection(session, SpatialTableHandle.unwrap(handle), projections, assignments); } /** @@ -930,7 +1082,10 @@ public Optional> applyProjecti @Override public Optional> applyLimit( ConnectorSession session, ConnectorTableHandle handle, long limit) { - return delegate.applyLimit(session, handle, limit); + if (handle instanceof SpatialTableHandle) { + return Optional.empty(); // preserve the cheap-accept wrapper (see applyProjection) + } + return delegate.applyLimit(session, SpatialTableHandle.unwrap(handle), limit); } /** @@ -943,6 +1098,13 @@ public Optional> applyLimit( * {@code record_count} values in the manifest list — no splits, no scan, * roughly 50ms instead of minutes on multi-billion-row tables. * + *

But that manifest fast-path counts {@code record_count}, ignoring any row-level predicate — + * so for a cheap-accept scan (a {@link SpatialTableHandle}, whose {@code ST_Intersects} we claimed + * enforced and now filter in the page source) it would return the whole file's row count, not the + * intersecting count. We therefore decline the pushdown for a wrapped handle: returning empty + * both preserves the wrapper and forces a real scan-and-filter. Non-spatial aggregates still take + * the fast path. + * * @param session the connector session * @param handle the table handle * @param aggregates the aggregate functions @@ -955,7 +1117,10 @@ public Optional> applyAggrega ConnectorSession session, ConnectorTableHandle handle, List aggregates, Map assignments, List> groupingSets) { - return delegate.applyAggregation(session, handle, aggregates, assignments, groupingSets); + if (handle instanceof SpatialTableHandle) { + return Optional.empty(); + } + return delegate.applyAggregation(session, SpatialTableHandle.unwrap(handle), aggregates, assignments, groupingSets); } /** @@ -972,7 +1137,10 @@ public Optional> applyAggrega public Optional> applyTopN( ConnectorSession session, ConnectorTableHandle handle, long topNCount, List sortItems, Map assignments) { - return delegate.applyTopN(session, handle, topNCount, sortItems, assignments); + if (handle instanceof SpatialTableHandle) { + return Optional.empty(); // preserve the cheap-accept wrapper (see applyProjection) + } + return delegate.applyTopN(session, SpatialTableHandle.unwrap(handle), topNCount, sortItems, assignments); } /** @@ -985,7 +1153,7 @@ public Optional> applyTopN( @Override public TableStatistics getTableStatistics(ConnectorSession session, ConnectorTableHandle tableHandle) { - return delegate.getTableStatistics(session, tableHandle); + return delegate.getTableStatistics(session, SpatialTableHandle.unwrap(tableHandle)); } /** @@ -996,7 +1164,7 @@ public TableStatistics getTableStatistics(ConnectorSession session, */ @Override public void validateScan(ConnectorSession session, ConnectorTableHandle handle) { - delegate.validateScan(session, handle); + delegate.validateScan(session, SpatialTableHandle.unwrap(handle)); } /** @@ -1009,7 +1177,7 @@ public void validateScan(ConnectorSession session, ConnectorTableHandle handle) @Override public boolean allowSplittingReadIntoMultipleSubQueries(ConnectorSession session, ConnectorTableHandle tableHandle) { - return delegate.allowSplittingReadIntoMultipleSubQueries(session, tableHandle); + return delegate.allowSplittingReadIntoMultipleSubQueries(session, SpatialTableHandle.unwrap(tableHandle)); } // Views @@ -1201,7 +1369,7 @@ public void setSchemaAuthorization(ConnectorSession session, String schemaName, */ @Override public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle) { - SchemaTableName name = delegate.getTableName(session, tableHandle); + SchemaTableName name = delegate.getTableName(session, SpatialTableHandle.unwrap(tableHandle)); delegate.dropTable(session, tableHandle); geomCatalog.invalidate(name); } @@ -1216,7 +1384,7 @@ public void dropTable(ConnectorSession session, ConnectorTableHandle tableHandle @Override public void renameTable(ConnectorSession session, ConnectorTableHandle tableHandle, SchemaTableName newTableName) { - SchemaTableName oldName = delegate.getTableName(session, tableHandle); + SchemaTableName oldName = delegate.getTableName(session, SpatialTableHandle.unwrap(tableHandle)); delegate.renameTable(session, tableHandle, newTableName); geomCatalog.invalidate(oldName); geomCatalog.invalidate(newTableName); diff --git a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialPageSourceProvider.java b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialPageSourceProvider.java new file mode 100644 index 00000000000..d7305080004 --- /dev/null +++ b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialPageSourceProvider.java @@ -0,0 +1,159 @@ +/*********************************************************************** + * Copyright (c) 2013-2025 General Atomics Integrated Intelligence, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Apache License, Version 2.0 + * which accompanies this distribution and is available at + * https://www.apache.org/licenses/LICENSE-2.0 + ***********************************************************************/ + +package org.locationtech.geomesa.trino.spatial.iceberg.connector; + +import io.trino.spi.connector.ColumnHandle; +import io.trino.spi.connector.ConnectorPageSource; +import io.trino.spi.connector.ConnectorPageSourceProvider; +import io.trino.spi.connector.ConnectorSession; +import io.trino.spi.connector.ConnectorSplit; +import io.trino.spi.connector.ConnectorTableCredentials; +import io.trino.spi.connector.ConnectorTableHandle; +import io.trino.spi.connector.ConnectorTransactionHandle; +import io.trino.spi.connector.DynamicFilter; +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.GeometryFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +import org.locationtech.geomesa.trino.spatial.iceberg.connector.BboxFilteringPageSource.BboxBound; +import org.locationtech.geomesa.trino.spatial.iceberg.connector.BboxFilteringPageSource.ShortCircuitConfig; + +import static org.locationtech.geomesa.trino.spatial.iceberg.connector.SpatialTableHandle.XMAX; +import static org.locationtech.geomesa.trino.spatial.iceberg.connector.SpatialTableHandle.XMIN; +import static org.locationtech.geomesa.trino.spatial.iceberg.connector.SpatialTableHandle.YMAX; +import static org.locationtech.geomesa.trino.spatial.iceberg.connector.SpatialTableHandle.YMIN; + +/** + * Wraps the Iceberg page-source provider to apply the connector's BBOX short-circuit in + * {@link BboxFilteringPageSource}: when the handle is a {@link SpatialTableHandle} (the connector + * claimed a rectangle {@code ST_Intersects} on a point column as enforced), this reads the bbox + * sub-fields + geometry, builds outer (reject) and inner (accept) boxes from the exact rectangle, + * and does the authoritative reject/accept/exact filtering. + * + *

For any other handle it delegates unchanged — zero overhead. The handle is always unwrapped to + * its underlying Iceberg table handle before the delegate reads it; a missed unwrap would surface as + * a loud {@code ClassCastException}, never a wrong result. + */ +final class SpatialPageSourceProvider implements ConnectorPageSourceProvider { + + private static final Logger LOG = LoggerFactory.getLogger(SpatialPageSourceProvider.class); + private static final GeometryFactory GEOMETRY_FACTORY = new GeometryFactory(); + + private final ConnectorPageSourceProvider delegate; + + SpatialPageSourceProvider(ConnectorPageSourceProvider delegate) { + this.delegate = delegate; + } + + @Override + public ConnectorPageSource createPageSource( + ConnectorTransactionHandle transaction, ConnectorSession session, + ConnectorSplit split, ConnectorTableHandle table, + Optional credentials, + List columns, DynamicFilter dynamicFilter) { + Plan plan = plan(table, columns); + ConnectorPageSource ps = delegate.createPageSource( + transaction, session, split, SpatialTableHandle.unwrap(table), credentials, + plan.columns(), dynamicFilter); + return plan.wrap(ps); + } + + @Override + public ConnectorPageSource createPageSource( + ConnectorTransactionHandle transaction, ConnectorSession session, + ConnectorSplit split, ConnectorTableHandle table, + List columns, DynamicFilter dynamicFilter) { + Plan plan = plan(table, columns); + ConnectorPageSource ps = delegate.createPageSource( + transaction, session, split, SpatialTableHandle.unwrap(table), plan.columns(), dynamicFilter); + return plan.wrap(ps); + } + + /** The physical columns to read (query columns, plus bbox sub-fields — and, in bbox-short-circuit + * mode, the geometry — appended to evaluate the filter) and how to wrap the page source. */ + private record Plan(List columns, int outputChannelCount, + boolean stripAddedChannels, ShortCircuitConfig shortCircuitConfig) { + ConnectorPageSource wrap(ConnectorPageSource ps) { + return shortCircuitConfig == null ? ps + : new BboxFilteringPageSource(ps, outputChannelCount, stripAddedChannels, shortCircuitConfig); + } + } + + private Plan plan(ConnectorTableHandle table, List columns) { + if (table instanceof SpatialTableHandle sth) { + LOG.debug("bbox short-circuit ENGAGED (page-source filtering active)"); + return planAccept(sth, columns); + } + // If a rectangle ST_Intersects on points was claimed enforced but the handle arrives here as a + // plain Iceberg handle, the filter silently no-ops and the engine sees every row — log which. + LOG.debug("bbox short-circuit NOT engaged; passthrough for handle {}", table.getClass().getName()); + return new Plan(columns, columns.size(), false, null); + } + + /** Bbox-short-circuit: authoritative reject/accept/exact. Outer (reject) and inner (accept) boxes are + * built from the exact query rectangle with directional 2-ulp float rounding so a row that the + * nearest-rounded stored bbox could misclassify always falls through to the exact test. */ + private Plan planAccept(SpatialTableHandle tableHandle, List columns) { + List augmented = new ArrayList<>(columns); + int[] channels = new int[4]; + for (int i = 0; i < 4; i++) { + channels[i] = channelFor(augmented, tableHandle.bboxLeaves().get(i)); + } + int geomCh = channelFor(augmented, tableHandle.geomColumn()); + + double minX = tableHandle.rectMinX(), + minY = tableHandle.rectMinY(), + maxX = tableHandle.rectMaxX(), + maxY = tableHandle.rectMaxY(); + + // Reject box (necessary overlap condition): expanded outward. + List outer = List.of( + new BboxBound(channels[XMAX], outLow(minX), Float.POSITIVE_INFINITY), + new BboxBound(channels[XMIN], Float.NEGATIVE_INFINITY, outHigh(maxX)), + new BboxBound(channels[YMAX], outLow(minY), Float.POSITIVE_INFINITY), + new BboxBound(channels[YMIN], Float.NEGATIVE_INFINITY, outHigh(maxY))); + // Accept box (sufficient containment condition): shrunk inward — no WKB decode needed. + List inner = List.of( + new BboxBound(channels[XMIN], inLow(minX), Float.POSITIVE_INFINITY), + new BboxBound(channels[XMAX], Float.NEGATIVE_INFINITY, inHigh(maxX)), + new BboxBound(channels[YMIN], inLow(minY), Float.POSITIVE_INFINITY), + new BboxBound(channels[YMAX], Float.NEGATIVE_INFINITY, inHigh(maxY))); + + Geometry queryRect = GEOMETRY_FACTORY.createPolygon(new Coordinate[]{ + new Coordinate(minX, minY), new Coordinate(maxX, minY), new Coordinate(maxX, maxY), + new Coordinate(minX, maxY), new Coordinate(minX, minY)}); + + boolean stripped = augmented.size() > columns.size(); + return new Plan(augmented, columns.size(), stripped, + new ShortCircuitConfig(outer, inner, geomCh, queryRect)); + } + + private static int channelFor(List columns, ColumnHandle handle) { + int i = columns.indexOf(handle); + if (i >= 0) { + return i; + } + columns.add(handle); + return columns.size() - 1; + } + + // Directional 2-ulp float rounding: outward for the reject box (never drop a true match), + // inward for the accept box (never accept a true non-match). Two ulps clears the ½-ulp error + // between a nearest-rounded stored float32 bbox and the true double geometry bound. + static float outLow(double d) { return Math.nextDown(Math.nextDown((float) d)); } + static float outHigh(double d) { return Math.nextUp(Math.nextUp((float) d)); } + static float inLow(double d) { return Math.nextUp(Math.nextUp((float) d)); } + static float inHigh(double d) { return Math.nextDown(Math.nextDown((float) d)); } +} diff --git a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialTableHandle.java b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialTableHandle.java new file mode 100644 index 00000000000..3a73f9c1ac9 --- /dev/null +++ b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialTableHandle.java @@ -0,0 +1,80 @@ +/*********************************************************************** + * Copyright (c) 2013-2025 General Atomics Integrated Intelligence, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Apache License, Version 2.0 + * which accompanies this distribution and is available at + * https://www.apache.org/licenses/LICENSE-2.0 + ***********************************************************************/ + +package org.locationtech.geomesa.trino.spatial.iceberg.connector; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import io.trino.plugin.iceberg.IcebergColumnHandle; +import io.trino.plugin.iceberg.IcebergTableHandle; +import io.trino.spi.connector.ConnectorTableHandle; + +import java.util.List; + +/** + * A table handle produced by {@link SpatialConnectorMetadata#applyFilter} only when the + * connector has claimed a rectangle {@code ST_Intersects} on a {@code Point} geometry column as + * fully enforced — i.e. it will do the exact filtering itself in {@link BboxFilteringPageSource} + * rather than leave it to the engine. Wraps {@link IcebergTableHandle} and carries what + * the worker needs that the pushed float32 bbox domains cannot convey: + * + *

    + *
  • {@code geomColumn} — the geometry WKB column to materialize for the boundary-shell exact test;
    • + *
  • the exact query rectangle {@code [minX,maxX] × [minY,maxY]} (double precision) — + * used to build the inward-rounded accept box and to run the shell {@code intersects} test. + * The float32 domains on the handle are outward-rounded and lossy, so the exact rectangle + * must ride here.
    • + *
+ * + *

All Trino/Iceberg interaction unwraps this back to {@link #delegate()} first; + * a missed unwrap surfaces as a loud {@code ClassCastException}, never a silent wrong result. + */ +public record SpatialTableHandle( + IcebergTableHandle delegate, + IcebergColumnHandle geomColumn, + List bboxLeaves, + double rectMinX, + double rectMinY, + double rectMaxX, + double rectMaxY +) implements ConnectorTableHandle { + + /** Order of {@link #bboxLeaves}: xmin, ymin, xmax, ymax. */ + public static final int XMIN = 0, YMIN = 1, XMAX = 2, YMAX = 3; + + @JsonCreator + public SpatialTableHandle( + @JsonProperty("delegate") IcebergTableHandle delegate, + @JsonProperty("geomColumn") IcebergColumnHandle geomColumn, + @JsonProperty("bboxLeaves") List bboxLeaves, + @JsonProperty("rectMinX") double rectMinX, + @JsonProperty("rectMinY") double rectMinY, + @JsonProperty("rectMaxX") double rectMaxX, + @JsonProperty("rectMaxY") double rectMaxY) { + this.delegate = delegate; + this.geomColumn = geomColumn; + this.bboxLeaves = bboxLeaves; + this.rectMinX = rectMinX; + this.rectMinY = rectMinY; + this.rectMaxX = rectMaxX; + this.rectMaxY = rectMaxY; + } + + @JsonProperty public IcebergTableHandle delegate() { return delegate; } + @JsonProperty public IcebergColumnHandle geomColumn() { return geomColumn; } + @JsonProperty public List bboxLeaves() { return bboxLeaves; } + @JsonProperty public double rectMinX() { return rectMinX; } + @JsonProperty public double rectMinY() { return rectMinY; } + @JsonProperty public double rectMaxX() { return rectMaxX; } + @JsonProperty public double rectMaxY() { return rectMaxY; } + + /** Unwrap to the Iceberg handle if wrapped, else return the handle unchanged. */ + static ConnectorTableHandle unwrap(ConnectorTableHandle handle) { + return handle instanceof SpatialTableHandle w ? w.delegate() : handle; + } +} diff --git a/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/BboxShortCircuitParityIT.java b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/BboxShortCircuitParityIT.java new file mode 100644 index 00000000000..f88f8504534 --- /dev/null +++ b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/BboxShortCircuitParityIT.java @@ -0,0 +1,119 @@ +/*********************************************************************** + * Copyright (c) 2013-2025 General Atomics Integrated Intelligence, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Apache License, Version 2.0 + * which accompanies this distribution and is available at + * https://www.apache.org/licenses/LICENSE-2.0 + ***********************************************************************/ + +package org.locationtech.geomesa.trino.spatial; + +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * The ship gate for connector-side bbox-short-circuit: with {@code geomesa.spatial.bbox-page-filter=true}, + * the connector claims rectangle {@code ST_Intersects} on point columns enforced and filters + * in {@code BboxFilteringPageSource} instead of the engine — so there is no exact-predicate safety + * net, and only a corpus check against the engine's own {@code ST_Intersects} proves it correct. + * + *

Two invariants, on the {@code spatial.observations} demo table (points, z2 → bbox-short-circuit + * eligible, and carrying {@code __vis__}): + *

    + *
  1. Spatial parity — for every rectangle in the corpus (including whole-world, + * empty, and mid-extent boxes whose edges straddle data and exercise the exact shell test), + * the count via {@code spatial_iceberg} as a full-auth identity equals the count via the plain + * {@code iceberg} connector (which evaluates the exact {@code ST_Intersects} with no + * visibility). Any divergence means bbox-short-circuit dropped or admitted a row the engine didn't.
    2. + *
  2. Visibility preserved — a partial-auth identity sees strictly fewer rows than + * a full-auth identity, proving the {@code is_visible} row-filter still applies under + * bbox-short-circuit. (Regression guard: the first cut replaced the whole residual with TRUE and + * silently bypassed visibility — full-auth parity alone would not have caught it.)
    3. + *
+ * + *

Requires a running Trino at localhost:8080 with the flag ON; skips otherwise. Relies on the + * local file auth-mapping (user {@code geomesa} → full ladder; {@code public} → partial tier). + */ +@Tag("integration") +class BboxShortCircuitParityIT { + + private static final String SPATIAL = "jdbc:trino://localhost:8080/spatial_iceberg"; + private static final String PLAIN = "jdbc:trino://localhost:8080/iceberg"; + + /** Full-auth identity (sees every visibility tier the fixture uses). */ + private static final String FULL_USER = "geomesa"; + /** Partial-auth identity (a strict subset of the tiers). */ + private static final String PARTIAL_USER = "public"; + + private static boolean ready; + + /** Rectangles spanning the whole world down to sub-degree boxes at varied positions; the exact + * result is unknown but must be identical between the two engines for every one. */ + private static final List BOXES = List.of( + "POLYGON ((-180 -90, 180 -90, 180 90, -180 90, -180 -90))", // whole world → all rows + "POLYGON ((-180 -90, -170 -90, -170 -80, -180 -80, -180 -90))", // far corner → likely empty + "POLYGON ((-10 -10, 10 -10, 10 10, -10 10, -10 -10))", + "POLYGON ((0 0, 40 0, 40 40, 0 40, 0 0))", + "POLYGON ((-100 20, -80 20, -80 40, -100 40, -100 20))", + "POLYGON ((-77 38, -77 40, -76 40, -76 38, -77 38))", + "POLYGON ((-0.5 -0.5, 0.5 -0.5, 0.5 0.5, -0.5 0.5, -0.5 -0.5))", + "POLYGON ((30 30, 30.25 30, 30.25 30.25, 30 30.25, 30 30))"); + + @BeforeAll + static void setup() { + try (var socket = new java.net.Socket()) { + socket.connect(new java.net.InetSocketAddress("localhost", 8080), 2000); + } catch (Exception e) { + Assumptions.assumeTrue(false, "Trino not reachable at localhost:8080 — skipping"); + } + ready = TestFixtures.ensureTable("observations"); + Assumptions.assumeTrue(ready, "spatial.observations unavailable — skipping"); + } + + private static long count(String catalogUrl, String user, String box) throws SQLException { + String sql = "SELECT count(*) FROM spatial.observations" + + " WHERE ST_Intersects(ST_GeometryFromText('" + box + "'), ST_GeomFromBinary(geom))"; + try (Connection c = DriverManager.getConnection(catalogUrl + "?user=" + user); + Statement s = c.createStatement(); + ResultSet rs = s.executeQuery(sql)) { + rs.next(); + return rs.getLong(1); + } + } + + @Test + void bboxShortCircuitMatchesEngineExactAcrossBoxes() throws SQLException { + for (String box : BOXES) { + long viaConnector = count(SPATIAL, FULL_USER, box); // bbox-short-circuit, all rows visible + long viaEngine = count(PLAIN, "admin", box); // exact ST_Intersects, no visibility + assertThat(viaConnector) + .as("bbox-short-circuit count must equal the engine's exact ST_Intersects for %s", box) + .isEqualTo(viaEngine); + } + } + + @Test + void visibilityStillEnforcedUnderBboxShortCircuit() throws SQLException { + String all = BOXES.get(0); + long full = count(SPATIAL, FULL_USER, all); + long partial = count(SPATIAL, PARTIAL_USER, all); + assertThat(full) + .as("full-auth identity must see the fixture's restricted rows") + .isGreaterThan(0); + assertThat(partial) + .as("partial-auth identity must see strictly fewer rows — is_visible must survive " + + "bbox-short-circuit (regression guard for the visibility-bypass bug)") + .isLessThan(full); + } +} diff --git a/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSourceTest.java b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSourceTest.java new file mode 100644 index 00000000000..4df2d016bf2 --- /dev/null +++ b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSourceTest.java @@ -0,0 +1,225 @@ +/*********************************************************************** + * Copyright (c) 2013-2025 General Atomics Integrated Intelligence, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Apache License, Version 2.0 + * which accompanies this distribution and is available at + * https://www.apache.org/licenses/LICENSE-2.0 + ***********************************************************************/ + +package org.locationtech.geomesa.trino.spatial.iceberg.connector; + +import io.airlift.slice.Slices; +import io.trino.spi.Page; +import io.trino.spi.block.Block; +import io.trino.spi.block.BlockBuilder; +import io.trino.spi.block.IntArrayBlock; +import io.trino.spi.connector.ConnectorPageSource; +import io.trino.spi.connector.SourcePage; +import io.trino.spi.type.VarbinaryType; +import org.junit.jupiter.api.Test; +import org.locationtech.geomesa.trino.spatial.iceberg.connector.BboxFilteringPageSource.BboxBound; +import org.locationtech.geomesa.trino.spatial.iceberg.connector.BboxFilteringPageSource.ShortCircuitConfig; +import org.locationtech.jts.geom.Coordinate; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.GeometryFactory; +import org.locationtech.jts.io.WKBWriter; + +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.OptionalLong; +import java.util.function.ObjLongConsumer; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Direct behavioural test of {@link BboxFilteringPageSource}: feed a page of bbox sub-field columns + * (and geometry WKB) through a stub delegate and assert exactly which positions the reject / accept / + * exact-shell classification keeps. This is the coverage the corpus/parity IT can't give offline — + * and the guard against a refactor silently turning the filter into a pass-through (every row kept). + * + *

Geometry subtype is {@code Point}, so each row's bbox is degenerate: {@code xmin==xmax==x} and + * {@code ymin==ymax==y}. Bounds are built exactly as {@link SpatialPageSourceProvider#planAccept} + * does, via its directional-rounding helpers, over the query rectangle {@code [0,10] × [0,20]}. + * Physical channel layout: {@code 0=xmin, 1=ymin, 2=xmax, 3=ymax, 4=geom}. + */ +class BboxFilteringPageSourceTest { + + private static final GeometryFactory GF = new GeometryFactory(); + private static final int XMIN = 0, YMIN = 1, XMAX = 2, YMAX = 3, GEOM = 4; + + /** Reject/accept boxes over [0,10]×[0,20], built the same way the provider builds them. */ + private static ShortCircuitConfig config() { + double minX = 0, minY = 0, maxX = 10, maxY = 20; + List outer = List.of( + new BboxBound(XMAX, SpatialPageSourceProvider.outLow(minX), Float.POSITIVE_INFINITY), + new BboxBound(XMIN, Float.NEGATIVE_INFINITY, SpatialPageSourceProvider.outHigh(maxX)), + new BboxBound(YMAX, SpatialPageSourceProvider.outLow(minY), Float.POSITIVE_INFINITY), + new BboxBound(YMIN, Float.NEGATIVE_INFINITY, SpatialPageSourceProvider.outHigh(maxY))); + List inner = List.of( + new BboxBound(XMIN, SpatialPageSourceProvider.inLow(minX), Float.POSITIVE_INFINITY), + new BboxBound(XMAX, Float.NEGATIVE_INFINITY, SpatialPageSourceProvider.inHigh(maxX)), + new BboxBound(YMIN, SpatialPageSourceProvider.inLow(minY), Float.POSITIVE_INFINITY), + new BboxBound(YMAX, Float.NEGATIVE_INFINITY, SpatialPageSourceProvider.inHigh(maxY))); + Geometry rect = GF.createPolygon(new Coordinate[]{ + new Coordinate(minX, minY), new Coordinate(maxX, minY), new Coordinate(maxX, maxY), + new Coordinate(minX, maxY), new Coordinate(minX, minY)}); + return new ShortCircuitConfig(outer, inner, GEOM, rect); + } + + /** A page of point rows: bbox leaves from the coordinates, geometry from {@code geoms} + * (null ⇒ null WKB). {@code xs.length} must equal {@code ys.length} and {@code geoms.length}. */ + private static SourcePage pointPage(float[] xs, float[] ys, Geometry[] geoms) { + return new StubSourcePage(xs.length, + realBlock(xs), realBlock(ys), realBlock(xs), realBlock(ys), geomBlock(geoms)); + } + + /** A page whose bbox leaves are all SQL NULL (forcing the exact-shell path), with the given geoms. */ + private static SourcePage nullBboxPage(Geometry[] geoms) { + Block nulls = nullRealBlock(geoms.length); + return new StubSourcePage(geoms.length, nulls, nulls, nulls, nulls, geomBlock(geoms)); + } + + private static int filteredPositions(SourcePage page, ShortCircuitConfig config, + int outputChannelCount, boolean strip) { + var src = new BboxFilteringPageSource(new OneShotPageSource(page), outputChannelCount, strip, config); + SourcePage out = src.getNextSourcePage(); + return out.getPositionCount(); + } + + @Test + void rejectsPointsWhollyOutsideEnvelope() { + // (-5,10) left of box, (50,10) right, (5,30) above — none can intersect [0,10]×[0,20]. + SourcePage page = pointPage(new float[]{-5, 50, 5}, new float[]{10, 10, 30}, + new Geometry[]{point(-5, 10), point(50, 10), point(5, 30)}); + assertThat(filteredPositions(page, config(), 5, false)).isEqualTo(0); + } + + @Test + void acceptsPointsWhollyInsideEnvelopeWithoutDecodingGeometry() { + // All strictly inside the inward-rounded accept box. Geometry blocks are NULL: if the accept + // path wrongly fell through to the exact test it would decode null and drop them → 0, failing. + SourcePage page = pointPage(new float[]{5, 2, 8}, new float[]{10, 5, 15}, + new Geometry[]{null, null, null}); + assertThat(filteredPositions(page, config(), 5, false)).isEqualTo(3); + } + + @Test + void keepsOnlyIntersectingPointsInMixedPage() { + // in, out, in, out, in → 3 kept. The regression guard: a pass-through filter returns 5. + SourcePage page = pointPage( + new float[]{5, -5, 2, 50, 8}, + new float[]{10, 10, 5, 10, 15}, + new Geometry[]{point(5, 10), point(-5, 10), point(2, 5), point(50, 10), point(8, 15)}); + assertThat(filteredPositions(page, config(), 5, false)).isEqualTo(3); + } + + @Test + void countStarConfigurationStripsChannelsAndReportsFilteredCount() { + // Mirrors the production count(*) path: no query columns, so outputChannelCount = 0 and the + // added bbox/geom channels are stripped. The reported position count must be the FILTERED + // count, not the raw input — this is the exact shape the flag-on EXPLAIN showed passing 4.44B. + SourcePage page = pointPage( + new float[]{5, -5, 2, 50, 8}, + new float[]{10, 10, 5, 10, 15}, + new Geometry[]{point(5, 10), point(-5, 10), point(2, 5), point(50, 10), point(8, 15)}); + var src = new BboxFilteringPageSource(new OneShotPageSource(page), 0, true, config()); + SourcePage out = src.getNextSourcePage(); + assertThat(out.getPositionCount()).as("filtered position count").isEqualTo(3); + assertThat(out.getChannelCount()).as("added channels hidden from engine").isEqualTo(0); + } + + @Test + void nullBboxRowFallsThroughToExactGeometryTest() { + // Null bbox can prove neither reject nor accept, so each row decodes its WKB and runs the + // exact intersection: the (5,10) point matches, the (50,10) point does not. + SourcePage page = nullBboxPage(new Geometry[]{point(5, 10), point(50, 10)}); + assertThat(filteredPositions(page, config(), 5, false)).isEqualTo(1); + } + + // ---- fixtures --------------------------------------------------------------------------------- + + private static Geometry point(double x, double y) { + return GF.createPoint(new Coordinate(x, y)); + } + + /** REAL block (float32) from the given values, no nulls. */ + private static Block realBlock(float... values) { + int[] bits = new int[values.length]; + for (int i = 0; i < values.length; i++) { + bits[i] = Float.floatToIntBits(values[i]); + } + return new IntArrayBlock(values.length, Optional.empty(), bits); + } + + /** REAL block whose every position is SQL NULL. */ + private static Block nullRealBlock(int positions) { + boolean[] isNull = new boolean[positions]; + Arrays.fill(isNull, true); + return new IntArrayBlock(positions, Optional.of(isNull), new int[positions]); + } + + /** VARBINARY block of WKB-encoded geometries (null ⇒ SQL NULL). */ + private static Block geomBlock(Geometry[] geoms) { + WKBWriter writer = new WKBWriter(); + BlockBuilder builder = VarbinaryType.VARBINARY.createBlockBuilder(null, geoms.length); + for (Geometry g : geoms) { + if (g == null) { + builder.appendNull(); + } else { + VarbinaryType.VARBINARY.writeSlice(builder, Slices.wrappedBuffer(writer.write(g))); + } + } + return builder.build(); + } + + /** Minimal in-memory {@link SourcePage}; {@link #selectPositions} narrows it in place so the + * post-filter {@link #getPositionCount()} reflects what the page source retained. */ + private static final class StubSourcePage implements SourcePage { + private Block[] blocks; + private int positionCount; + + StubSourcePage(int positionCount, Block... blocks) { + this.positionCount = positionCount; + this.blocks = blocks; + } + + @Override public int getPositionCount() { return positionCount; } + @Override public long getSizeInBytes() { return 0; } + @Override public long getRetainedSizeInBytes() { return 0; } + @Override public void retainedBytesForEachPart(ObjLongConsumer consumer) {} + @Override public int getChannelCount() { return blocks.length; } + @Override public Block getBlock(int channel) { return blocks[channel]; } + @Override public Page getPage() { return new Page(positionCount, blocks); } + + @Override + public void selectPositions(int[] positions, int offset, int size) { + Block[] narrowed = new Block[blocks.length]; + for (int i = 0; i < blocks.length; i++) { + narrowed[i] = blocks[i].getPositions(positions, offset, size); + } + blocks = narrowed; + positionCount = size; + } + } + + /** Delegate that yields one page then signals exhaustion. */ + private static final class OneShotPageSource implements ConnectorPageSource { + private SourcePage page; + + OneShotPageSource(SourcePage page) { this.page = page; } + + @Override public SourcePage getNextSourcePage() { + SourcePage next = page; + page = null; + return next; + } + + @Override public boolean isFinished() { return page == null; } + @Override public long getCompletedBytes() { return 0; } + @Override public long getReadTimeNanos() { return 0; } + @Override public long getMemoryUsage() { return 0; } + @Override public OptionalLong getCompletedPositions() { return OptionalLong.empty(); } + @Override public void close() {} + } +} diff --git a/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxShortCircuitTest.java b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxShortCircuitTest.java new file mode 100644 index 00000000000..78f55164def --- /dev/null +++ b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxShortCircuitTest.java @@ -0,0 +1,88 @@ +/*********************************************************************** + * Copyright (c) 2013-2025 General Atomics Integrated Intelligence, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Apache License, Version 2.0 + * which accompanies this distribution and is available at + * https://www.apache.org/licenses/LICENSE-2.0 + ***********************************************************************/ + +package org.locationtech.geomesa.trino.spatial.iceberg.connector; + +import io.trino.spi.expression.Call; +import io.trino.spi.expression.ConnectorExpression; +import io.trino.spi.expression.Constant; +import io.trino.spi.expression.FunctionName; +import io.trino.spi.type.BooleanType; +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static io.trino.spi.expression.StandardFunctions.AND_FUNCTION_NAME; +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Pure-logic guards for the connector-side bbox-short-circuit: + *
    + *
  • {@code dropSpatialCall} must claim only {@code ST_Intersects} enforced — every + * other conjunct, above all the {@code is_visible} row-filter, must survive. (Regression guard + * for the visibility-bypass bug where the whole residual was replaced with TRUE.)
    • + *
  • the reject/accept box bounds must round in the correct direction (outer outward, inner + * inward) so soundness holds against the ½-ulp error of the nearest-rounded float32 bbox.
    • + *
+ */ +class BboxShortCircuitTest { + + private static Call call(String name, ConnectorExpression... args) { + return new Call(BooleanType.BOOLEAN, new FunctionName(name), List.of(args)); + } + + private static Call and(ConnectorExpression... args) { + return new Call(BooleanType.BOOLEAN, AND_FUNCTION_NAME, List.of(args)); + } + + @Test + void dropSpatialCallReplacesOnlyStIntersects_keepingIsVisible() { + Call isVisible = call("is_visible"); + Call stIntersects = call("st_intersects"); + ConnectorExpression result = + SpatialConnectorMetadata.dropSpatialCall(and(isVisible, stIntersects)); + + assertThat(result).isInstanceOf(Call.class); + Call andResult = (Call) result; + assertThat(andResult.getFunctionName()).isEqualTo(AND_FUNCTION_NAME); + assertThat(andResult.getArguments()).hasSize(2); + assertThat(andResult.getArguments().get(0)).isEqualTo(isVisible); + assertThat(andResult.getArguments().get(1)).isEqualTo(Constant.TRUE); + } + + @Test + void dropSpatialCallOnBarePredicateBecomesTrue() { + assertThat(SpatialConnectorMetadata.dropSpatialCall(call("st_intersects"))) + .isEqualTo(Constant.TRUE); + } + + @Test + void dropSpatialCallLeavesNonSpatialConjunctsUntouched() { + Call isVisible = call("is_visible"); + Call other = call("some_other_predicate"); + ConnectorExpression result = SpatialConnectorMetadata.dropSpatialCall(and(isVisible, other)); + // No st_intersects present → nothing is dropped; the tree is preserved. + assertThat(((Call) result).getArguments()).containsExactly(isVisible, other); + } + + @Test + void boundsRoundOutwardForRejectInwardForAccept() { + for (double d : new double[]{-77.0, -76.0, 38.0, 40.0, 0.0, -179.999, 179.999, 123.456}) { + float f = (float) d; + // Reject box is expanded outward → never drops a true match. + assertThat(SpatialPageSourceProvider.outLow(d)).isLessThan(f); + assertThat(SpatialPageSourceProvider.outHigh(d)).isGreaterThan(f); + // Accept box is shrunk inward → never accepts a true non-match. + assertThat(SpatialPageSourceProvider.inLow(d)).isGreaterThan(f); + assertThat(SpatialPageSourceProvider.inHigh(d)).isLessThan(f); + // Accept box is strictly inside the reject box (boundary rows fall to the exact test). + assertThat(SpatialPageSourceProvider.inLow(d)).isGreaterThan(SpatialPageSourceProvider.outLow(d)); + assertThat(SpatialPageSourceProvider.inHigh(d)).isLessThan(SpatialPageSourceProvider.outHigh(d)); + } + } +} diff --git a/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadataShortCircuitPreservationTest.java b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadataShortCircuitPreservationTest.java new file mode 100644 index 00000000000..72036acfbf3 --- /dev/null +++ b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadataShortCircuitPreservationTest.java @@ -0,0 +1,104 @@ +/*********************************************************************** + * Copyright (c) 2013-2025 General Atomics Integrated Intelligence, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Apache License, Version 2.0 + * which accompanies this distribution and is available at + * https://www.apache.org/licenses/LICENSE-2.0 + ***********************************************************************/ + +package org.locationtech.geomesa.trino.spatial.iceberg.connector; + +import io.trino.spi.connector.AggregateFunction; +import io.trino.spi.connector.AggregationApplicationResult; +import io.trino.spi.connector.ColumnHandle; +import io.trino.spi.connector.ConnectorMetadata; +import io.trino.spi.connector.ConnectorSession; +import io.trino.spi.connector.ConnectorTableHandle; +import io.trino.spi.connector.LimitApplicationResult; +import io.trino.spi.connector.ProjectionApplicationResult; +import io.trino.spi.connector.SortItem; +import io.trino.spi.connector.TopNApplicationResult; +import io.trino.spi.expression.ConnectorExpression; +import org.junit.jupiter.api.Test; +import org.locationtech.geomesa.trino.spatial.iceberg.GeoMesaColumnCatalog; + +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Once {@code applyFilter} claims a rectangle {@code ST_Intersects} enforced and wraps the handle in + * a {@link SpatialTableHandle}, the later push-down callbacks must NOT hand that handle to the + * Iceberg delegate: the delegate's result carries the unwrapped Iceberg handle, which the + * engine would adopt as the scan handle — dropping the wrapper, so {@code SpatialPageSourceProvider} + * sees a plain Iceberg handle, skips the filter, and every row survives (a {@code count(*)} returns + * the whole table). {@code applyAggregation} is the worst offender: Iceberg answers {@code count(*)} + * from manifest {@code record_count}, ignoring the spatial predicate entirely. + * + *

Each callback must therefore decline (return empty) for a wrapped handle, preserving it for the + * page source. The stub delegate throws if reached, so a missing guard fails the test rather than + * silently regressing correctness. + */ +class SpatialConnectorMetadataShortCircuitPreservationTest { + + /** A wrapped handle; the guards test {@code instanceof} before touching any field, so the inner + * Iceberg handle can be null. */ + private static final SpatialTableHandle WRAPPED = + new SpatialTableHandle(null, null, List.of(), 0, 0, 0, 0); + + private final SpatialConnectorMetadata metadata = + new SpatialConnectorMetadata(new FailIfCalledDelegate(), new GeoMesaColumnCatalog(), true); + + @Test + void applyProjectionDeclinesForWrappedHandle() { + assertThat(metadata.applyProjection(null, WRAPPED, List.of(), Map.of())).isEmpty(); + } + + @Test + void applyLimitDeclinesForWrappedHandle() { + assertThat(metadata.applyLimit(null, WRAPPED, 10)).isEmpty(); + } + + @Test + void applyAggregationDeclinesForWrappedHandle() { + assertThat(metadata.applyAggregation(null, WRAPPED, List.of(), Map.of(), List.of())).isEmpty(); + } + + @Test + void applyTopNDeclinesForWrappedHandle() { + assertThat(metadata.applyTopN(null, WRAPPED, 10, List.of(), Map.of())).isEmpty(); + } + + /** Delegate whose push-down callbacks blow up if invoked — proving the guard short-circuited. */ + private static final class FailIfCalledDelegate implements ConnectorMetadata { + @Override + public Optional> applyProjection( + ConnectorSession session, ConnectorTableHandle handle, + List projections, Map assignments) { + throw new AssertionError("delegate.applyProjection must not be called for a SpatialTableHandle"); + } + + @Override + public Optional> applyLimit( + ConnectorSession session, ConnectorTableHandle handle, long limit) { + throw new AssertionError("delegate.applyLimit must not be called for a SpatialTableHandle"); + } + + @Override + public Optional> applyAggregation( + ConnectorSession session, ConnectorTableHandle handle, + List aggregates, Map assignments, + List> groupingSets) { + throw new AssertionError("delegate.applyAggregation must not be called for a SpatialTableHandle"); + } + + @Override + public Optional> applyTopN( + ConnectorSession session, ConnectorTableHandle handle, + long topNCount, List sortItems, Map assignments) { + throw new AssertionError("delegate.applyTopN must not be called for a SpatialTableHandle"); + } + } +} diff --git a/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorSplitManagerGatingTest.java b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorSplitManagerGatingTest.java new file mode 100644 index 00000000000..0c9c4ac369b --- /dev/null +++ b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorSplitManagerGatingTest.java @@ -0,0 +1,83 @@ +/*********************************************************************** + * Copyright (c) 2013-2025 General Atomics Integrated Intelligence, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Apache License, Version 2.0 + * which accompanies this distribution and is available at + * https://www.apache.org/licenses/LICENSE-2.0 + ***********************************************************************/ + +package org.locationtech.geomesa.trino.spatial.iceberg.connector; + +import io.trino.spi.connector.Connector; +import io.trino.spi.connector.ConnectorMetadata; +import io.trino.spi.connector.ConnectorSession; +import io.trino.spi.connector.ConnectorSplitManager; +import io.trino.spi.connector.ConnectorSplitSource; +import io.trino.spi.connector.ConnectorTableHandle; +import io.trino.spi.connector.ConnectorTransactionHandle; +import io.trino.spi.connector.Constraint; +import io.trino.spi.connector.DynamicFilter; +import io.trino.spi.transaction.IsolationLevel; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Regression guard for the split-manager gate. Only bbox-page-filter ON ever wraps a + * handle in {@link SpatialTableHandle}, so only then may {@link SpatialConnector#getSplitManager()} + * interpose a wrapper to unwrap it. When the flag is off it MUST hand back Iceberg's own split + * manager verbatim. See {@code SpatialConnector.getSplitManager}. + */ +class SpatialConnectorSplitManagerGatingTest { + + /** Sentinel standing in for Iceberg's real split manager, so we can assert identity. */ + private static final ConnectorSplitManager DELEGATE_SPLIT_MANAGER = new ConnectorSplitManager() { + @Override + public ConnectorSplitSource getSplits(ConnectorTransactionHandle transaction, + ConnectorSession session, ConnectorTableHandle table, + DynamicFilter dynamicFilter, Constraint constraint) { + return null; + } + }; + + private static Connector delegateConnector() { + return new Connector() { + @Override + public ConnectorTransactionHandle beginTransaction(IsolationLevel isolationLevel, + boolean readOnly, boolean autoCommit) { + return null; + } + + @Override + public ConnectorMetadata getMetadata(ConnectorSession session, + ConnectorTransactionHandle transactionHandle) { + return null; + } + + @Override + public ConnectorSplitManager getSplitManager() { + return DELEGATE_SPLIT_MANAGER; + } + + @Override + public void shutdown() {} + }; + } + + @Test + void flagOffReturnsIcebergSplitManagerVerbatim() { + SpatialConnector connector = new SpatialConnector(delegateConnector(), null, null, false); + assertThat(connector.getSplitManager()) + .as("flag off must not interpose any wrapper — Iceberg pruning depends on its own " + + "split manager being handed back unchanged") + .isSameAs(DELEGATE_SPLIT_MANAGER); + } + + @Test + void flagOnWrapsSoSpatialTableHandleCanBeUnwrapped() { + SpatialConnector connector = new SpatialConnector(delegateConnector(), null, null, true); + assertThat(connector.getSplitManager()) + .as("flag on wraps to unwrap the bbox-short-circuit SpatialTableHandle before Iceberg") + .isNotSameAs(DELEGATE_SPLIT_MANAGER); + } +} From d5dc81975cbc927dfe8aa97aba095fe36735e95d Mon Sep 17 00:00:00 2001 From: Chris Dobbins Date: Wed, 22 Jul 2026 01:40:05 +0000 Subject: [PATCH 3/5] Additionally added a pre-reject filter for coordinates outside of the bounding box coordinates of the envelope, which boosts non-point, non-rectangular geometry performance --- .../connector/BboxFilteringPageSource.java | 88 +++++---- .../connector/SpatialConnectorMetadata.java | 33 +++- .../connector/SpatialPageSourceProvider.java | 184 ++++++++++++++---- .../BboxFilteringPageSourceTest.java | 91 +++++---- 4 files changed, 278 insertions(+), 118 deletions(-) diff --git a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSource.java b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSource.java index 46a93ee94bb..db17a54f8ab 100644 --- a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSource.java +++ b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSource.java @@ -29,27 +29,37 @@ /** * Wraps the Iceberg page source with a sound, pre-decode bbox filter over a geometry's - * {@code __X_bbox__} sub-fields, which the connector injected as real domains and which ride to - * the worker in the table handle's unenforced predicate. - * Row Filtering: + * {@code __X_bbox__} sub-fields, which the connector injected as REAL domains and which ride to the + * worker in the table handle's unenforced predicate. Two modes, selected by whether an + * {@link AcceptConfig} is supplied: + * + *

    + *
  • Reject-only ({@code accept == null}) — drops rows whose bbox cannot overlap + * the query envelope and hands the survivors to the engine's exact {@code ST_*} residual. A + * sound pre-filter for any spatial predicate the connector could not claim enforced + * (non-rectangle query, non-point column, {@code ST_Contains}, …): it saves the WKB decode on + * the rejected rows without changing the result.
    • + *
  • Short-circuit ({@code accept != null}) — the connector claimed a rectangle + * {@code ST_Intersects} on a point column enforced, so this is the authoritative filter: *
      - *
    • bbox outside the outer (float32-outward-rounded) box ⇒ reject;
      • - *
    • bbox inside the inner (float32-inward-rounded) box ⇒ accept - * with no WKB decode — the row's geometry is provably inside the query rectangle;
      • - *
    • otherwise (the thin boundary shell, or a null bbox) ⇒ decode the geometry WKB and run - * the exact {@code intersects} test against the query rectangle.
      • + *
    • bbox outside the reject box (float32-outward-rounded) ⇒ reject;
      • + *
    • bbox inside the accept box (float32-inward-rounded) ⇒ accept with no + * WKB decode — the geometry is provably inside the query rectangle;
      • + *
    • otherwise (thin boundary shell, or a null bbox) ⇒ decode the WKB and run the + * exact {@code intersects} test.
      • *
    - * The outward/inward float32 rounding of the two boxes guarantees every row that could be - * misclassified by the nearest-rounded stored bbox falls through to the exact test, so the - * result is identical to the engine's exact predicate. Bbox-short-circuit mode is only ever - * engaged for a rectangle {@code ST_Intersects} on a geometry column whose SFT subtype is - * {@code Point} (see {@link SpatialConnectorMetadata}). + * The outward/inward rounding guarantees any row a nearest-rounded float32 bbox could + * misclassify falls through to the exact test, so the result equals the engine's exact + * predicate.
    • + *
+ * + *

Both modes share the same reject pass over {@link #rejectBounds}; short-circuit adds the + * accept/shell classification on the survivors. * *

Laziness. Only the cheap bbox columns are materialized to classify rows; the - * geometry column is fetched only when a shell row is present in a page, and decoded only at the - * shell positions. Accept and reject rows never touch the WKB. The bbox sub-field columns this - * page source added to the physical read (beyond what the query projected) are stripped from the - * returned page. + * geometry column is fetched only when a shell row appears in a page, and decoded only at the shell + * positions. Accept and reject rows never touch the WKB. Any bbox/geometry columns this page source + * added to the physical read (beyond what the query projected) are stripped from the returned page. */ final class BboxFilteringPageSource implements ConnectorPageSource { @@ -58,31 +68,33 @@ final class BboxFilteringPageSource implements ConnectorPageSource { /** One bbox-box bound: the row's real value in {@code channel} must lie within {@code [low, high]}. */ record BboxBound(int channel, float low, float high) {} - /** Bbox-short-circuit configuration. {@code outerBounds} are the reject bounds (ANY violated by a - * non-null bbox ⇒ reject); {@code innerBounds} are the containment bounds (ALL must hold ⇒ accept); - * {@code geomChannel} is the physical channel of the geometry WKB column; {@code queryRect} is the - * exact query rectangle for the shell test. */ - record ShortCircuitConfig(List outerBounds, - List innerBounds, - int geomChannel, - Geometry queryRect) {} + /** Short-circuit accept configuration; {@code null} ⇒ reject-only mode. {@code innerBounds} + * are the containment bounds (ALL must hold ⇒ accept with no WKB decode); {@code geomChannel} is + * the physical channel of the geometry WKB column; {@code queryRect} is the exact query rectangle + * for the boundary-shell test. */ + record AcceptConfig(List innerBounds, int geomChannel, Geometry queryRect) {} private final ConnectorPageSource delegate; - private final ShortCircuitConfig config; + /** Reject box: a row whose non-null bbox violates any bound is provably outside the envelope. */ + private final List rejectBounds; + /** Accept/exact config in short-circuit mode; {@code null} ⇒ reject-only pre-filter. */ + private final AcceptConfig accept; /** Number of channels the query actually requested; any beyond this were added to read the - * bbox sub-fields and the geometry, and must be hidden from the engine. */ + * bbox sub-fields (and, in short-circuit mode, the geometry) and must be hidden from the engine. */ private final int outputChannelCount; private final boolean stripAddedChannels; BboxFilteringPageSource(ConnectorPageSource delegate, int outputChannelCount, boolean stripAddedChannels, - ShortCircuitConfig config) { + List rejectBounds, + AcceptConfig accept) { this.delegate = delegate; this.outputChannelCount = outputChannelCount; this.stripAddedChannels = stripAddedChannels; - this.config = config; + this.rejectBounds = rejectBounds; + this.accept = accept; } @Override @@ -93,29 +105,31 @@ public SourcePage getNextSourcePage() { } int positions = page.getPositionCount(); - List outerBounds = config.outerBounds(); - List innerBounds = config.innerBounds(); - Block[] outerBlocks = materialize(page, outerBounds); - Block[] innerBlocks = materialize(page, innerBounds); + Block[] rejectBlocks = materialize(page, rejectBounds); + Block[] innerBlocks = accept == null ? null : materialize(page, accept.innerBounds()); Block geomBlock = null; // geometry column + reader fetched lazily, only if a shell row appears WKBReader reader = null; int[] retained = new int[positions]; int kept = 0; for (int p = 0; p < positions; p++) { - if (rejected(p, outerBlocks, outerBounds)) { + if (rejected(p, rejectBlocks, rejectBounds)) { continue; // provably outside the envelope → drop } - if (accepted(p, innerBlocks, innerBounds)) { + if (accept == null) { + retained[kept++] = p; + continue; // reject-only: the engine's exact ST_ still runs on the survivors + } + if (accepted(p, innerBlocks, accept.innerBounds())) { retained[kept++] = p; continue; // provably inside the envelope → accept with no WKB decode } // Boundary shell (or null bbox): decode WKB and run the exact test. if (geomBlock == null) { - geomBlock = page.getBlock(config.geomChannel()); + geomBlock = page.getBlock(accept.geomChannel()); reader = new WKBReader(); } - if (shellMatches(geomBlock, p, reader, config.queryRect())) { + if (shellMatches(geomBlock, p, reader, accept.queryRect())) { retained[kept++] = p; } } diff --git a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java index 69f84bd4750..36d969babb6 100644 --- a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java +++ b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java @@ -184,15 +184,27 @@ public Optional> applyFilter( Envelope env = new Envelope(match.envelopes().get(0)); match.envelopes().forEach(env::expandToInclude); // Bbox sub-field domains for per-file Parquet-stat pruning. + boolean point = geom.partition().map(p -> p.kind() == SpatialIndexKind.Z2).orElse(false); geom.bbox().ifPresent(bbox -> { - // Skip re-injection if the planner round-tripped this already. - if (constraint.getSummary().getDomains().map(d -> d.containsKey(bbox.xmax())).orElse(false)) { + // Skip re-injection if the planner round-tripped this already (xmin is injected in + // both the point and non-point forms below). + if (constraint.getSummary().getDomains().map(d -> d.containsKey(bbox.xmin())).orElse(false)) { return; } - domains.merge(bbox.xmax(), realGreaterThanOrEqual(pruneSafeLowerBound(env.getMinX())), Domain::intersect); - domains.merge(bbox.xmin(), realLessThanOrEqual(pruneSafeUpperBound(env.getMaxX())), Domain::intersect); - domains.merge(bbox.ymax(), realGreaterThanOrEqual(pruneSafeLowerBound(env.getMinY())), Domain::intersect); - domains.merge(bbox.ymin(), realLessThanOrEqual(pruneSafeUpperBound(env.getMaxY())), Domain::intersect); + if (point) { + // Point data (Z2 ⟺ sft subtype Point): the stored bbox is degenerate — + // xmin==xmax==x, ymin==ymax==y — so box overlap is a point-in-range test. One + // two-sided domain per axis on the lower sub-field gives the same file pruning + // while halving both the stat predicates and the columns the reject-only page + // source later reads (it reconstructs the reject box from these domains). + domains.merge(bbox.xmin(), realBetween(pruneSafeLowerBound(env.getMinX()), pruneSafeUpperBound(env.getMaxX())), Domain::intersect); + domains.merge(bbox.ymin(), realBetween(pruneSafeLowerBound(env.getMinY()), pruneSafeUpperBound(env.getMaxY())), Domain::intersect); + } else { + domains.merge(bbox.xmax(), realGreaterThanOrEqual(pruneSafeLowerBound(env.getMinX())), Domain::intersect); + domains.merge(bbox.xmin(), realLessThanOrEqual(pruneSafeUpperBound(env.getMaxX())), Domain::intersect); + domains.merge(bbox.ymax(), realGreaterThanOrEqual(pruneSafeLowerBound(env.getMinY())), Domain::intersect); + domains.merge(bbox.ymin(), realLessThanOrEqual(pruneSafeUpperBound(env.getMaxY())), Domain::intersect); + } injectedBboxes.add(bbox); }); @@ -367,6 +379,15 @@ private static Domain realLessThanOrEqual(float value) { List.of(Range.lessThanOrEqual(RealType.REAL, bits))), false); } + /** Builds a REAL-typed Domain for "low <= column <= high" (used for point data, where the + * degenerate bbox collapses each axis to a single two-sided range). */ + private static Domain realBetween(float low, float high) { + return Domain.create(SortedRangeSet.copyOf(RealType.REAL, List.of( + Range.range(RealType.REAL, + (long) Float.floatToIntBits(low), true, + (long) Float.floatToIntBits(high), true))), false); + } + /** * Function names whose row-side argument is a geometry and whose query-side * argument is a geometry literal — the shape this connector can extract an diff --git a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialPageSourceProvider.java b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialPageSourceProvider.java index d7305080004..365f5471d11 100644 --- a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialPageSourceProvider.java +++ b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialPageSourceProvider.java @@ -8,6 +8,8 @@ package org.locationtech.geomesa.trino.spatial.iceberg.connector; +import io.trino.plugin.iceberg.IcebergColumnHandle; +import io.trino.plugin.iceberg.IcebergTableHandle; import io.trino.spi.connector.ColumnHandle; import io.trino.spi.connector.ConnectorPageSource; import io.trino.spi.connector.ConnectorPageSourceProvider; @@ -17,6 +19,11 @@ import io.trino.spi.connector.ConnectorTableHandle; import io.trino.spi.connector.ConnectorTransactionHandle; import io.trino.spi.connector.DynamicFilter; +import io.trino.spi.predicate.Domain; +import io.trino.spi.predicate.Range; +import io.trino.spi.predicate.SortedRangeSet; +import io.trino.spi.predicate.TupleDomain; +import io.trino.spi.type.RealType; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryFactory; @@ -25,31 +32,40 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Optional; +import org.locationtech.geomesa.trino.spatial.iceberg.connector.BboxFilteringPageSource.AcceptConfig; import org.locationtech.geomesa.trino.spatial.iceberg.connector.BboxFilteringPageSource.BboxBound; -import org.locationtech.geomesa.trino.spatial.iceberg.connector.BboxFilteringPageSource.ShortCircuitConfig; -import static org.locationtech.geomesa.trino.spatial.iceberg.connector.SpatialTableHandle.XMAX; import static org.locationtech.geomesa.trino.spatial.iceberg.connector.SpatialTableHandle.XMIN; -import static org.locationtech.geomesa.trino.spatial.iceberg.connector.SpatialTableHandle.YMAX; import static org.locationtech.geomesa.trino.spatial.iceberg.connector.SpatialTableHandle.YMIN; /** - * Wraps the Iceberg page-source provider to apply the connector's BBOX short-circuit in - * {@link BboxFilteringPageSource}: when the handle is a {@link SpatialTableHandle} (the connector - * claimed a rectangle {@code ST_Intersects} on a point column as enforced), this reads the bbox - * sub-fields + geometry, builds outer (reject) and inner (accept) boxes from the exact rectangle, - * and does the authoritative reject/accept/exact filtering. + * Wraps the Iceberg page-source provider to apply the connector's bbox filtering in + * {@link BboxFilteringPageSource}. Two engagements, both driven by the bbox reject box: * - *

For any other handle it delegates unchanged — zero overhead. The handle is always unwrapped to - * its underlying Iceberg table handle before the delegate reads it; a missed unwrap would surface as - * a loud {@code ClassCastException}, never a wrong result. + *

    + *
  • Short-circuit — the handle is a {@link SpatialTableHandle} (the connector + * claimed a rectangle {@code ST_Intersects} on a point column enforced). Reads the bbox + * sub-fields + geometry and does the authoritative reject/accept/exact filtering; the reject + * box is built from the exact rectangle with outward float rounding.
    • + *
  • Reject-only — a plain {@link IcebergTableHandle} whose unenforced predicate + * carries the connector-injected REAL bbox domains (any spatial predicate that could not be + * claimed enforced: non-rectangle query, non-point column, {@code ST_Contains}, …). Rebuilds + * the same reject box from those domains and drops rows that cannot overlap before the + * engine's exact {@code ST_*} residual decodes their WKB.
    • + *
+ * + *

Otherwise (non-spatial query, or no pushed bbox domains) it delegates unchanged — zero overhead. + * The handle is always unwrapped to its underlying Iceberg handle before the delegate reads it; a + * missed unwrap would surface as a loud {@code ClassCastException}, never a wrong result. */ final class SpatialPageSourceProvider implements ConnectorPageSourceProvider { private static final Logger LOG = LoggerFactory.getLogger(SpatialPageSourceProvider.class); private static final GeometryFactory GEOMETRY_FACTORY = new GeometryFactory(); + private static final String BBOX_SUFFIX = "_bbox__"; private final ConnectorPageSourceProvider delegate; @@ -81,36 +97,43 @@ public ConnectorPageSource createPageSource( return plan.wrap(ps); } - /** The physical columns to read (query columns, plus bbox sub-fields — and, in bbox-short-circuit - * mode, the geometry — appended to evaluate the filter) and how to wrap the page source. */ - private record Plan(List columns, int outputChannelCount, - boolean stripAddedChannels, ShortCircuitConfig shortCircuitConfig) { + /** The physical columns to read (query columns, plus the bbox sub-fields — and, in short-circuit + * mode, the geometry — appended to evaluate the filter), the reject box, and (short-circuit only) + * the accept/exact config. Empty reject box ⇒ nothing to filter, delegate passes through. */ + private record Plan(List columns, int outputChannelCount, boolean stripAddedChannels, + List rejectBounds, AcceptConfig accept) { ConnectorPageSource wrap(ConnectorPageSource ps) { - return shortCircuitConfig == null ? ps - : new BboxFilteringPageSource(ps, outputChannelCount, stripAddedChannels, shortCircuitConfig); + return rejectBounds.isEmpty() ? ps + : new BboxFilteringPageSource(ps, outputChannelCount, stripAddedChannels, rejectBounds, accept); } } private Plan plan(ConnectorTableHandle table, List columns) { if (table instanceof SpatialTableHandle sth) { - LOG.debug("bbox short-circuit ENGAGED (page-source filtering active)"); + LOG.debug("bbox short-circuit ENGAGED (reject/accept/exact)"); return planAccept(sth, columns); } - // If a rectangle ST_Intersects on points was claimed enforced but the handle arrives here as a - // plain Iceberg handle, the filter silently no-ops and the engine sees every row — log which. - LOG.debug("bbox short-circuit NOT engaged; passthrough for handle {}", table.getClass().getName()); - return new Plan(columns, columns.size(), false, null); + if (table instanceof IcebergTableHandle ith) { + return planReject(ith, columns); + } + return passthrough(columns); + } + + private static Plan passthrough(List columns) { + return new Plan(columns, columns.size(), false, List.of(), null); } - /** Bbox-short-circuit: authoritative reject/accept/exact. Outer (reject) and inner (accept) boxes are - * built from the exact query rectangle with directional 2-ulp float rounding so a row that the - * nearest-rounded stored bbox could misclassify always falls through to the exact test. */ + /** Short-circuit: authoritative reject/accept/exact for POINT data — Z2 ⟺ sft subtype + * {@code Point}, the only case {@link SpatialConnectorMetadata} wraps in a + * {@link SpatialTableHandle}. For a point the stored bbox is degenerate ({@code xmin==xmax==x}, + * {@code ymin==ymax==y}), so we read only the two lower sub-fields as the point's {@code (x, y)} — + * half the bbox columns scanned and half the per-row compares. Box overlap collapses to a + * point-in-range test, built from the exact query rectangle with directional 2-ulp float rounding + * so any point a nearest-rounded float32 could misclassify falls through to the exact shell test. */ private Plan planAccept(SpatialTableHandle tableHandle, List columns) { List augmented = new ArrayList<>(columns); - int[] channels = new int[4]; - for (int i = 0; i < 4; i++) { - channels[i] = channelFor(augmented, tableHandle.bboxLeaves().get(i)); - } + int xCh = channelFor(augmented, tableHandle.bboxLeaves().get(XMIN)); // xmin == xmax == x + int yCh = channelFor(augmented, tableHandle.bboxLeaves().get(YMIN)); // ymin == ymax == y int geomCh = channelFor(augmented, tableHandle.geomColumn()); double minX = tableHandle.rectMinX(), @@ -118,26 +141,57 @@ private Plan planAccept(SpatialTableHandle tableHandle, List colum maxX = tableHandle.rectMaxX(), maxY = tableHandle.rectMaxY(); - // Reject box (necessary overlap condition): expanded outward. - List outer = List.of( - new BboxBound(channels[XMAX], outLow(minX), Float.POSITIVE_INFINITY), - new BboxBound(channels[XMIN], Float.NEGATIVE_INFINITY, outHigh(maxX)), - new BboxBound(channels[YMAX], outLow(minY), Float.POSITIVE_INFINITY), - new BboxBound(channels[YMIN], Float.NEGATIVE_INFINITY, outHigh(maxY))); - // Accept box (sufficient containment condition): shrunk inward — no WKB decode needed. + // Reject box (necessary condition): the point lies outside the outward-rounded rectangle. + List reject = List.of( + new BboxBound(xCh, outLow(minX), outHigh(maxX)), + new BboxBound(yCh, outLow(minY), outHigh(maxY))); + // Accept box (sufficient condition): the point lies inside the inward-rounded rectangle — keep + // with no WKB decode. List inner = List.of( - new BboxBound(channels[XMIN], inLow(minX), Float.POSITIVE_INFINITY), - new BboxBound(channels[XMAX], Float.NEGATIVE_INFINITY, inHigh(maxX)), - new BboxBound(channels[YMIN], inLow(minY), Float.POSITIVE_INFINITY), - new BboxBound(channels[YMAX], Float.NEGATIVE_INFINITY, inHigh(maxY))); + new BboxBound(xCh, inLow(minX), inHigh(maxX)), + new BboxBound(yCh, inLow(minY), inHigh(maxY))); Geometry queryRect = GEOMETRY_FACTORY.createPolygon(new Coordinate[]{ new Coordinate(minX, minY), new Coordinate(maxX, minY), new Coordinate(maxX, maxY), new Coordinate(minX, maxY), new Coordinate(minX, minY)}); boolean stripped = augmented.size() > columns.size(); - return new Plan(augmented, columns.size(), stripped, - new ShortCircuitConfig(outer, inner, geomCh, queryRect)); + return new Plan(augmented, columns.size(), stripped, reject, new AcceptConfig(inner, geomCh, queryRect)); + } + + /** Reject-only pre-filter: rebuild the reject box from the connector-injected REAL bbox domains in + * the unenforced predicate (the same overlap condition {@link #planAccept} derives from the exact + * rectangle) and drop rows that cannot overlap; the engine's exact {@code ST_*} residual runs on + * the survivors. No accept/exact config — the engine, not this page source, is authoritative. */ + private Plan planReject(IcebergTableHandle ith, List columns) { + TupleDomain predicate = ith.getUnenforcedPredicate(); + if (predicate.isNone() || predicate.getDomains().isEmpty()) { + return passthrough(columns); + } + List augmented = new ArrayList<>(columns); + List reject = new ArrayList<>(); + for (Map.Entry e : predicate.getDomains().get().entrySet()) { + IcebergColumnHandle handle = e.getKey(); + if (!isBboxSubField(handle)) { + continue; + } + // Reject-only only pays when it pre-empts a WKB decode. If the geometry the bbox guards + // isn't being read (a bbox-only query), the engine already applies the bbox predicate + // itself, so this pass would be pure overhead — decline it. + if (!geomColumnRead(handle, columns)) { + return passthrough(columns); + } + float[] span = spanBounds(e.getValue()); + if (span != null) { + reject.add(new BboxBound(channelFor(augmented, handle), span[0], span[1])); + } + } + if (reject.isEmpty()) { + return passthrough(columns); + } + LOG.debug("bbox reject-only pre-filter on {} bound(s)", reject.size()); + boolean stripped = augmented.size() > columns.size(); + return new Plan(augmented, columns.size(), stripped, reject, null); } private static int channelFor(List columns, ColumnHandle handle) { @@ -149,6 +203,52 @@ private static int channelFor(List columns, ColumnHandle handle) { return columns.size() - 1; } + /** A REAL sub-field of a {@code __X_bbox__} struct column. */ + private static boolean isBboxSubField(IcebergColumnHandle handle) { + return !handle.isBaseColumn() + && handle.getType() instanceof RealType + && handle.getBaseColumnIdentity().getName().endsWith(BBOX_SUFFIX); + } + + /** True iff the geometry column guarded by this bbox sub-field is among the read columns — i.e. + * there is a downstream WKB decode for reject-only to pre-empt. The geometry column {@code X} is + * the base of the {@code __X_bbox__} struct. */ + private static boolean geomColumnRead(IcebergColumnHandle bboxSubField, List columns) { + String bboxName = bboxSubField.getBaseColumnIdentity().getName(); // __X_bbox__ + String geomName = bboxName.substring(0, bboxName.length() - BBOX_SUFFIX.length()); // __X + if (geomName.startsWith("__")) { + geomName = geomName.substring(2); // X + } + for (ColumnHandle c : columns) { + if (c instanceof IcebergColumnHandle ich + && ich.isBaseColumn() + && ich.getBaseColumnIdentity().getName().equals(geomName)) { + return true; + } + } + return false; + } + + /** {@code [low, high]} float bounds of the domain's span (±∞ when unbounded), or null when the + * domain is not an ordered range set or imposes no bound. */ + private static float[] spanBounds(Domain domain) { + if (!(domain.getValues() instanceof SortedRangeSet ranges) || ranges.getRangeCount() == 0) { + return null; + } + Range span = ranges.getSpan(); + float low = span.isLowUnbounded() ? Float.NEGATIVE_INFINITY : realBits(span.getLowValue()); + float high = span.isHighUnbounded() ? Float.POSITIVE_INFINITY : realBits(span.getHighValue()); + if (low == Float.NEGATIVE_INFINITY && high == Float.POSITIVE_INFINITY) { + return null; + } + return new float[]{low, high}; + } + + /** REAL predicate values are the float's int bits widened to long. */ + private static float realBits(Optional value) { + return Float.intBitsToFloat(((Long) value.orElseThrow()).intValue()); + } + // Directional 2-ulp float rounding: outward for the reject box (never drop a true match), // inward for the accept box (never accept a true non-match). Two ulps clears the ½-ulp error // between a nearest-rounded stored float32 bbox and the true double geometry bound. diff --git a/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSourceTest.java b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSourceTest.java index 4df2d016bf2..8200e1374d9 100644 --- a/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSourceTest.java +++ b/geomesa-trino/geomesa-trino-plugin/src/test/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/BboxFilteringPageSourceTest.java @@ -17,8 +17,8 @@ import io.trino.spi.connector.SourcePage; import io.trino.spi.type.VarbinaryType; import org.junit.jupiter.api.Test; +import org.locationtech.geomesa.trino.spatial.iceberg.connector.BboxFilteringPageSource.AcceptConfig; import org.locationtech.geomesa.trino.spatial.iceberg.connector.BboxFilteringPageSource.BboxBound; -import org.locationtech.geomesa.trino.spatial.iceberg.connector.BboxFilteringPageSource.ShortCircuitConfig; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Geometry; import org.locationtech.jts.geom.GeometryFactory; @@ -38,61 +38,64 @@ * exact-shell classification keeps. This is the coverage the corpus/parity IT can't give offline — * and the guard against a refactor silently turning the filter into a pass-through (every row kept). * - *

Geometry subtype is {@code Point}, so each row's bbox is degenerate: {@code xmin==xmax==x} and - * {@code ymin==ymax==y}. Bounds are built exactly as {@link SpatialPageSourceProvider#planAccept} - * does, via its directional-rounding helpers, over the query rectangle {@code [0,10] × [0,20]}. - * Physical channel layout: {@code 0=xmin, 1=ymin, 2=xmax, 3=ymax, 4=geom}. + *

Geometry subtype is {@code Point}, so each row's bbox is degenerate ({@code xmin==xmax==x}, + * {@code ymin==ymax==y}) and {@code planAccept} reads only the two lower sub-fields as the point's + * {@code (x, y)}. Bounds are built exactly as {@link SpatialPageSourceProvider#planAccept} does, via + * its directional-rounding helpers, over the query rectangle {@code [0,10] × [0,20]}. Physical + * channel layout: {@code 0=x, 1=y, 2=geom}. */ class BboxFilteringPageSourceTest { private static final GeometryFactory GF = new GeometryFactory(); - private static final int XMIN = 0, YMIN = 1, XMAX = 2, YMAX = 3, GEOM = 4; + private static final int X = 0, Y = 1, GEOM = 2; - /** Reject/accept boxes over [0,10]×[0,20], built the same way the provider builds them. */ - private static ShortCircuitConfig config() { + /** Reject box over [0,10]×[0,20]: point outside the outward-rounded rectangle (as planAccept builds). */ + private static List rejectBox() { + double minX = 0, minY = 0, maxX = 10, maxY = 20; + return List.of( + new BboxBound(X, SpatialPageSourceProvider.outLow(minX), SpatialPageSourceProvider.outHigh(maxX)), + new BboxBound(Y, SpatialPageSourceProvider.outLow(minY), SpatialPageSourceProvider.outHigh(maxY))); + } + + /** Accept/exact config over the same rectangle (short-circuit mode; {@code null} ⇒ reject-only). */ + private static AcceptConfig acceptConfig() { double minX = 0, minY = 0, maxX = 10, maxY = 20; - List outer = List.of( - new BboxBound(XMAX, SpatialPageSourceProvider.outLow(minX), Float.POSITIVE_INFINITY), - new BboxBound(XMIN, Float.NEGATIVE_INFINITY, SpatialPageSourceProvider.outHigh(maxX)), - new BboxBound(YMAX, SpatialPageSourceProvider.outLow(minY), Float.POSITIVE_INFINITY), - new BboxBound(YMIN, Float.NEGATIVE_INFINITY, SpatialPageSourceProvider.outHigh(maxY))); List inner = List.of( - new BboxBound(XMIN, SpatialPageSourceProvider.inLow(minX), Float.POSITIVE_INFINITY), - new BboxBound(XMAX, Float.NEGATIVE_INFINITY, SpatialPageSourceProvider.inHigh(maxX)), - new BboxBound(YMIN, SpatialPageSourceProvider.inLow(minY), Float.POSITIVE_INFINITY), - new BboxBound(YMAX, Float.NEGATIVE_INFINITY, SpatialPageSourceProvider.inHigh(maxY))); + new BboxBound(X, SpatialPageSourceProvider.inLow(minX), SpatialPageSourceProvider.inHigh(maxX)), + new BboxBound(Y, SpatialPageSourceProvider.inLow(minY), SpatialPageSourceProvider.inHigh(maxY))); Geometry rect = GF.createPolygon(new Coordinate[]{ new Coordinate(minX, minY), new Coordinate(maxX, minY), new Coordinate(maxX, maxY), new Coordinate(minX, maxY), new Coordinate(minX, minY)}); - return new ShortCircuitConfig(outer, inner, GEOM, rect); + return new AcceptConfig(inner, GEOM, rect); } - /** A page of point rows: bbox leaves from the coordinates, geometry from {@code geoms} - * (null ⇒ null WKB). {@code xs.length} must equal {@code ys.length} and {@code geoms.length}. */ + /** A page of point rows: the degenerate bbox is a single (x, y) pair per row, geometry from + * {@code geoms} (null ⇒ null WKB). {@code xs.length} must equal {@code ys.length} and {@code geoms.length}. */ private static SourcePage pointPage(float[] xs, float[] ys, Geometry[] geoms) { - return new StubSourcePage(xs.length, - realBlock(xs), realBlock(ys), realBlock(xs), realBlock(ys), geomBlock(geoms)); + return new StubSourcePage(xs.length, realBlock(xs), realBlock(ys), geomBlock(geoms)); } - /** A page whose bbox leaves are all SQL NULL (forcing the exact-shell path), with the given geoms. */ + /** A page whose (x, y) are SQL NULL (forcing the exact-shell path), with the given geoms. */ private static SourcePage nullBboxPage(Geometry[] geoms) { Block nulls = nullRealBlock(geoms.length); - return new StubSourcePage(geoms.length, nulls, nulls, nulls, nulls, geomBlock(geoms)); + return new StubSourcePage(geoms.length, nulls, nulls, geomBlock(geoms)); } - private static int filteredPositions(SourcePage page, ShortCircuitConfig config, + private static int filteredPositions(SourcePage page, List rejectBounds, AcceptConfig accept, int outputChannelCount, boolean strip) { - var src = new BboxFilteringPageSource(new OneShotPageSource(page), outputChannelCount, strip, config); - SourcePage out = src.getNextSourcePage(); - return out.getPositionCount(); + var src = new BboxFilteringPageSource( + new OneShotPageSource(page), outputChannelCount, strip, rejectBounds, accept); + return src.getNextSourcePage().getPositionCount(); } + // ---- short-circuit mode (accept != null): authoritative reject / accept / exact ------------- + @Test void rejectsPointsWhollyOutsideEnvelope() { // (-5,10) left of box, (50,10) right, (5,30) above — none can intersect [0,10]×[0,20]. SourcePage page = pointPage(new float[]{-5, 50, 5}, new float[]{10, 10, 30}, new Geometry[]{point(-5, 10), point(50, 10), point(5, 30)}); - assertThat(filteredPositions(page, config(), 5, false)).isEqualTo(0); + assertThat(filteredPositions(page, rejectBox(), acceptConfig(), 5, false)).isEqualTo(0); } @Test @@ -101,7 +104,7 @@ void acceptsPointsWhollyInsideEnvelopeWithoutDecodingGeometry() { // path wrongly fell through to the exact test it would decode null and drop them → 0, failing. SourcePage page = pointPage(new float[]{5, 2, 8}, new float[]{10, 5, 15}, new Geometry[]{null, null, null}); - assertThat(filteredPositions(page, config(), 5, false)).isEqualTo(3); + assertThat(filteredPositions(page, rejectBox(), acceptConfig(), 5, false)).isEqualTo(3); } @Test @@ -111,7 +114,7 @@ void keepsOnlyIntersectingPointsInMixedPage() { new float[]{5, -5, 2, 50, 8}, new float[]{10, 10, 5, 10, 15}, new Geometry[]{point(5, 10), point(-5, 10), point(2, 5), point(50, 10), point(8, 15)}); - assertThat(filteredPositions(page, config(), 5, false)).isEqualTo(3); + assertThat(filteredPositions(page, rejectBox(), acceptConfig(), 5, false)).isEqualTo(3); } @Test @@ -123,7 +126,7 @@ void countStarConfigurationStripsChannelsAndReportsFilteredCount() { new float[]{5, -5, 2, 50, 8}, new float[]{10, 10, 5, 10, 15}, new Geometry[]{point(5, 10), point(-5, 10), point(2, 5), point(50, 10), point(8, 15)}); - var src = new BboxFilteringPageSource(new OneShotPageSource(page), 0, true, config()); + var src = new BboxFilteringPageSource(new OneShotPageSource(page), 0, true, rejectBox(), acceptConfig()); SourcePage out = src.getNextSourcePage(); assertThat(out.getPositionCount()).as("filtered position count").isEqualTo(3); assertThat(out.getChannelCount()).as("added channels hidden from engine").isEqualTo(0); @@ -134,7 +137,29 @@ void nullBboxRowFallsThroughToExactGeometryTest() { // Null bbox can prove neither reject nor accept, so each row decodes its WKB and runs the // exact intersection: the (5,10) point matches, the (50,10) point does not. SourcePage page = nullBboxPage(new Geometry[]{point(5, 10), point(50, 10)}); - assertThat(filteredPositions(page, config(), 5, false)).isEqualTo(1); + assertThat(filteredPositions(page, rejectBox(), acceptConfig(), 5, false)).isEqualTo(1); + } + + // ---- reject-only mode (accept == null): drop non-overlapping, keep the rest for the engine --- + + @Test + void rejectOnlyDropsOutsidePointsAndKeepsTheRest() { + // Same mixed page; reject-only drops the two clearly-outside points and keeps the other three + // for the engine's exact ST_ (no accept/shell classification here). + SourcePage page = pointPage( + new float[]{5, -5, 2, 50, 8}, + new float[]{10, 10, 5, 10, 15}, + new Geometry[]{point(5, 10), point(-5, 10), point(2, 5), point(50, 10), point(8, 15)}); + assertThat(filteredPositions(page, rejectBox(), null, 5, false)).isEqualTo(3); + } + + @Test + void rejectOnlyKeepsNullBboxRowsWithoutDecoding() { + // A null bbox is never provably outside, so reject-only keeps BOTH rows for the engine WITHOUT + // decoding — including the (50,10) point the engine will later drop. Contrast with + // nullBboxRowFallsThroughToExactGeometryTest, where short-circuit mode decodes and keeps 1. + SourcePage page = nullBboxPage(new Geometry[]{point(5, 10), point(50, 10)}); + assertThat(filteredPositions(page, rejectBox(), null, 5, false)).isEqualTo(2); } // ---- fixtures --------------------------------------------------------------------------------- From df8976afed527ae1c8f8a0ce71e74d207a0f813c Mon Sep 17 00:00:00 2001 From: Chris Dobbins Date: Wed, 22 Jul 2026 03:34:31 +0000 Subject: [PATCH 4/5] Added configuration/flag documentation --- docs/user/trino/configuration.rst | 74 ++++++++++++++++++++ docs/user/trino/design.rst | 111 +++++++++++++++++------------- docs/user/trino/index.rst | 15 ++-- docs/user/trino/install.rst | 6 +- docs/user/trino/usage.rst | 106 ++++++++++++++-------------- 5 files changed, 202 insertions(+), 110 deletions(-) create mode 100644 docs/user/trino/configuration.rst diff --git a/docs/user/trino/configuration.rst b/docs/user/trino/configuration.rst new file mode 100644 index 00000000000..f35d3a6149b --- /dev/null +++ b/docs/user/trino/configuration.rst @@ -0,0 +1,74 @@ +.. _trino_configuration: + +Trino Connector Configuration +============================= + +The ``spatial_iceberg`` connector is configured through **catalog properties** in its +``.properties`` file (see :ref:`trino_install`). Any property that is not in a +``geomesa.*`` namespace is passed through unchanged to the stock Iceberg connector the +plugin wraps, so all of Iceberg's own catalog properties are available as-is. The +GeoMesa-specific properties below are consumed by the plugin and stripped before the +config reaches the Iceberg delegate. + +.. list-table:: + :header-rows: 1 + :widths: 32 12 12 44 + + * - Catalog property + - Default + - Required + - Description + * - ``geomesa.spatial.bbox-page-filter`` + - ``true`` + - no + - Enables connector-side, pre-decode bbox filtering in the page source (see below). + * - ``geomesa.security.*`` + - — + - no + - Trino-layer row-visibility enforcement; see :ref:`trino_security`. + +.. _trino_bbox_page_filter: + +Bounding-box page filtering +--------------------------- + +``geomesa.spatial.bbox-page-filter`` controls whether the connector wraps the Iceberg +page source with a bounding-box filter that runs **before** each row's geometry WKB is +decoded. The filter reads only the cheap ``___bbox__`` sub-field columns (which the +connector already injects as REAL domains for per-file stat pruning, and which ride to the +worker in the table handle's unenforced predicate) and operates in one of two modes, +chosen per query: + +* **Reject pre-filter** (all spatial predicates) — drops any row whose bbox *cannot* + overlap the query envelope, then hands the survivors to the engine's exact ``ST_*`` + residual. This is a sound, non-destructive pre-filter: only rows failing a *necessary* + overlap condition are removed, so no matching row is ever dropped and the engine's exact + predicate still runs on every survivor. It saves the WKB decode on the rejected rows for + any spatial predicate — including non-rectangular geometries, ``ST_Contains``, and the + outer envelope of a ``DWITHIN`` distance query. + +* **Short-circuit** (rectangle ``ST_Intersects`` on a Z2/point geometry column) — here the + connector can claim the predicate *enforced*, so the page source is the authoritative + filter: rows whose bbox lies fully inside the query rectangle are **accepted** with no + WKB decode, rows outside are **rejected**, and only rows on the thin envelope boundary + fall through to an exact decode-and-test. For point geometry columns the stored bbox is + degenerate (``xmin == xmax``, ``ymin == ymax``), so the test reads two coordinate columns + and collapses to a point-in-range check. + +Both modes use directional float32 rounding — the reject box is rounded outward and the +accept box inward — so any row a nearest-rounded float32 bbox could misclassify falls +through to the exact test, and the result is identical to the engine's exact predicate. +Non-spatial queries carry no bbox domains and delegate to Iceberg unchanged. + +When to disable +~~~~~~~~~~~~~~~ + +The filter's value is inversely proportional to spatial partition quality. On a +well-``truncate``-partitioned Z2/XZ2 table, Layer 1 and Layer 2 pruning +(see :ref:`trino_design`) already eliminate most non-matching rows before the page source +runs, so the extra bbox-column read can cost more than the decodes it saves — this is most +noticeable for ``DWITHIN``, where the geometry must be materialized for the exact distance +test regardless. On a coarsely-partitioned table the row-level rejection catches what file +pruning misses and is a clear win. Set ``geomesa.spatial.bbox-page-filter=false`` to turn +it off; the connector then relies on partition and per-file stat pruning alone and the +engine's exact ``ST_*`` predicate decodes every surviving row. diff --git a/docs/user/trino/design.rst b/docs/user/trino/design.rst index 52c7312d12a..37a4780bf00 100644 --- a/docs/user/trino/design.rst +++ b/docs/user/trino/design.rst @@ -17,7 +17,7 @@ Architecture │ applyFilter() → bbox + Z2 TupleDomain │ │ │ │ CQL clients ──► geomesa-trino-datastore ──► TrinoFilterToSQL │ - │ (CQL filter ──► bbox-overlap + CASE WHEN shortcut SQL) │ + │ (CQL filter ──► pure row-level ST_* SQL, column-first) │ └──────────────┬──────────────────────────────────────┬──────────────────┘ │ │ ┌───────▼──────────┐ ┌────────▼─────────────────┐ @@ -91,59 +91,72 @@ set — files in non-overlapping Z2 cells have non-overlapping bbox stats, so benchmark deltas between the two connectors read ~0% on rectangular queries: both land at the same file count via different mechanisms. -Layer 3: Row-Level CASE WHEN bbox-Contained Shortcut -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -For surviving rows, the GeoMesa data store's ``TrinoFilterToSQL`` emits SQL that -short-circuits the expensive geometry test when the row's bbox is fully inside -the query envelope. The form differs by spatial filter type: +Layer 3: Connector-Side Page-Source bbox Filter +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The data store emits only row-level ``ST_*`` predicates (no ``CASE WHEN``), so the +row-level saving that shortcut used to provide is recovered *inside the connector*, where +it applies uniformly to data store queries and hand-written Trino SQL alike. When +``geomesa.spatial.bbox-page-filter`` is enabled (the default), ``SpatialPageSourceProvider`` +wraps the Iceberg page source with ``BboxFilteringPageSource``. It reads only the cheap +``___bbox__`` sub-field columns (the REAL domains injected in Layer 2, carried to the +worker in the table handle's unenforced predicate) and filters rows **before** the geometry +WKB is decoded, in one of two modes chosen per query: + +* **Reject pre-filter** — for any spatial predicate, drops rows whose bbox cannot overlap + the query envelope and passes the survivors to the engine's exact ``ST_*`` residual. A + sound, non-destructive pre-filter: only rows failing a *necessary* overlap condition are + dropped, so no matching row is removed and the exact predicate still runs on every + survivor. Its win is the WKB decode it avoids on rejected rows. +* **Short-circuit** — for a rectangle ``ST_Intersects`` on a Z2/point geometry column the + bbox proves the result exactly, so ``applyFilter()`` claims the predicate enforced + (replacing only the ``st_intersects`` node with ``TRUE`` — never ``is_visible`` or any + other conjunct) and the page source becomes authoritative: rows whose bbox is inside are + **accepted with no decode**, rows outside are rejected, and only the thin + envelope-boundary shell is decoded and tested exactly. For point columns the stored bbox + is degenerate (``xmin == xmax``, ``ymin == ymax``), so the classifier reads two coordinate + columns and the box test collapses to a point-in-range check. + +Directional float32 rounding keeps both modes exact: the reject box is rounded outward and +the accept box inward, so any row a nearest-rounded float32 bbox could misclassify falls +through to the exact test. The mode each predicate uses: .. list-table:: :header-rows: 1 - :widths: 20 45 35 + :widths: 30 22 48 - * - CQL filter - - Emitted SQL pattern + * - Predicate (emitted by the data store / recognized in SQL) + - Page-source mode - Soundness - * - ``INTERSECTS(geom, axis-aligned rectangle)`` - - ``(bbox-overlap) AND CASE WHEN bbox-contained THEN TRUE ELSE ST_Intersects(geom, rect) END`` - - bbox⊆rect ⇒ geom⊆rect ⇒ ST_Intersects=TRUE (sufficient; the rectangle IS its envelope) - * - ``INTERSECTS(geom, non-rectangular polygon)`` - - ``(bbox-overlap) AND ST_Intersects(geom, polygon)`` - - no shortcut: bbox⊆env(polygon) does NOT imply intersection (holes, concavity) - * - ``WITHIN(geom, axis-aligned rectangle)`` - - ``(bbox-overlap) AND CASE WHEN bbox-in-shrunk-rect THEN TRUE ELSE ST_Within(geom, rect) END`` - - WITHIN is boundary-exclusive and the stored bbox is float32, so the shortcut - rectangle is shrunk by two float ulps per side (containment then proves the - geometry is strictly interior); boundary-adjacent rows take the exact test - * - ``WITHIN(geom, non-rectangular polygon)`` - - ``(bbox-overlap) AND ST_Within(geom, polygon)`` - - bbox⊆env(polygon) does NOT imply geom⊆polygon - * - ``DWITHIN(geom, ref, d)`` - - ``(outer-bbox) AND CASE WHEN bbox-in-inner-inscribed-rect THEN TRUE ELSE ST_Distance(...) ≤ d END`` - - outer bbox = env(ref) expanded by d (covers extended references end to end); - the inscribed-rect shortcut applies to point references only — for lines and - polygons the exact spherical check runs via the planar nearest-points pair - (Trino's spherical ``ST_Distance`` is point-only) - * - ``BBOX(geom, env)`` - - same as ``INTERSECTS(geom, envelope-rectangle)`` - - a bare float32 bbox-overlap is not exact (the stored bbox is rounded to - nearest, admitting rows up to ½ ulp outside the envelope), so BBOX takes the - rectangle-intersects shape: overlap prefilter + shrunk-contained shortcut + - exact ``ST_Intersects`` fallback - * - ``CROSSES`` / ``TOUCHES`` / ``OVERLAPS`` / ``EQUALS`` - - ``(bbox-overlap) AND ST_(geom, g)`` - - each implies a non-empty intersection ⇒ bbox-overlap is a valid necessary prefilter - * - ``CONTAINS(geom, g)`` - - ``(bbox-covers) AND ST_Contains(geom, g)`` - - geom ⊇ g ⇒ bbox(geom) ⊇ env(g) (necessary); reversed operands reuse the WITHIN paths - * - ``DISJOINT`` / ``BEYOND`` - - exact ``ST_Disjoint`` / spherical distance > d, **no prefilter** - - the matching rows lie outside the query neighborhood — an overlap prefilter would prune the answer - -CASE WHEN — not OR. Trino's optimizer distributes OR over AND, causing the -expensive predicate to evaluate up to 4× per row (3.3× wall-clock slowdown -measured). CASE WHEN survives the optimizer intact and short-circuits cleanly. + * - ``ST_Intersects(geom, axis-aligned rectangle)`` on a point/Z2 column + - short-circuit + - bbox⊆rect ⇒ geom⊆rect ⇒ intersects (the rectangle IS its own envelope); the + boundary shell falls to the exact test + * - ``ST_Intersects`` on a non-rectangular polygon, or on a non-point (XZ2) column + - reject-only + - bbox⊆env(polygon) does NOT imply intersection (holes, concavity); the exact + ``ST_Intersects`` runs on survivors + * - ``ST_Within`` / ``ST_Contains`` / ``ST_Crosses`` / ``ST_Touches`` / + ``ST_Overlaps`` / ``ST_Equals`` + - reject-only + - each implies a non-empty intersection ⇒ bbox-overlap is a valid necessary + prefilter; the exact predicate decides membership on survivors + * - ``DWITHIN`` — emitted as ``ST_Intersects(geom, outer rectangle) AND ST_Distance(...) ≤ d`` + - reject-only (on the outer rectangle) + - the outer rectangle (reference expanded by ``d``) is a necessary bound on the + distance neighborhood; the exact spherical ``ST_Distance`` runs on survivors + * - ``ST_Disjoint`` / ``BEYOND`` (distance > d) + - none + - matching rows lie *outside* the query neighborhood — a bbox-overlap prefilter would + prune the answer, so no bbox pushdown is derived + +Doing this in the connector rather than as ``CASE WHEN`` SQL is deliberate: it keeps the +data store's emitted SQL pure ``ST_*`` (so plans are identical however a query arrives), +and it sidesteps Trino's optimizer distributing an ``OR`` form over ``AND`` — which would +evaluate the expensive predicate up to 4× per row (a 3.3× wall-clock slowdown was measured +with the old SQL forms). The filter is a pure performance layer: disabling it +(``geomesa.spatial.bbox-page-filter=false``) changes runtime, never results. See +:ref:`trino_configuration` for the cost/benefit and when to disable. .. _trino_partitioning: diff --git a/docs/user/trino/index.rst b/docs/user/trino/index.rst index 3159c8fb985..fcaf56f9341 100644 --- a/docs/user/trino/index.rst +++ b/docs/user/trino/index.rst @@ -13,12 +13,16 @@ end-to-end: ``truncate(N_chars)`` on ``___z2__`` / ``___xz2__``). * **Per-file** ``___bbox__`` **column-stat pruning** at planning time (works for both the spatial connector and stock Iceberg). -* **Row-level CASE WHEN bbox-contained shortcut** in the SQL emitted by the GeoMesa - data store (skips ``ST_Intersects``/``ST_Distance`` for rows whose bbox is fully inside - the query envelope). +* **Connector-side page-source bbox filtering** (``geomesa.spatial.bbox-page-filter``, + on by default): before a surviving row's geometry WKB is decoded, the connector + rejects rows whose ``___bbox__`` cannot overlap the query envelope, and — for a + rectangle ``ST_Intersects`` on a point/Z2 column — accepts rows whose bbox is fully + inside without decoding at all. See :ref:`trino_configuration`. -For axis-aligned rectangular WITHIN queries the row-level test is eliminated entirely -— the bbox-contained predicate is exactly equivalent to ``ST_Within``. +The connector derives all three layers from the same ``ST_*`` call, so a CQL query +through the data store and the equivalent hand-written Trino SQL get identical plans. +The data store's ``TrinoFilterToSQL`` emits only row-level ``ST_*`` predicates; the +bbox/Z2 pruning and the row-level bbox filter are the connector's responsibility. ```` above is a placeholder for the geometry column's name; each geometry column in a table gets its own companion group, so multi-geometry tables carry @@ -42,5 +46,6 @@ GeoMesa Trino consists of two modules: design install + configuration usage security diff --git a/docs/user/trino/install.rst b/docs/user/trino/install.rst index 4086c6f50ec..5b562677d85 100644 --- a/docs/user/trino/install.rst +++ b/docs/user/trino/install.rst @@ -80,8 +80,10 @@ Glue example: fs.s3.enabled=true s3.region= -Optionally add the ``geomesa.security.*`` catalog properties for Trino-layer row -visibility (see :ref:`trino_security`). +The connector passes any non-``geomesa.*`` property straight through to the Iceberg +delegate. For the GeoMesa-specific catalog properties — including +``geomesa.spatial.bbox-page-filter`` and the ``geomesa.security.*`` row-visibility +keys — see :ref:`trino_configuration`. **3. Restart Trino** on all nodes. Verify the catalog is up with ``SHOW CATALOGS;``, then confirm pruning is active with the EXPLAIN checks under :ref:`trino_verify_pruning`. diff --git a/docs/user/trino/usage.rst b/docs/user/trino/usage.rst index 20b2b4d7654..4eba7fa8fbc 100644 --- a/docs/user/trino/usage.rst +++ b/docs/user/trino/usage.rst @@ -61,56 +61,56 @@ CQL consumers via the data store get the optimized SQL shapes described in SQL Patterns for Direct-SQL Consumers ------------------------------------- -Direct-SQL consumers (JDBC, BI tools) write their own SQL — these patterns let them -adopt the same optimization shapes manually. The examples below run against a table -whose geometry column is named ``geom``, so its companions are ``__geom_bbox__`` and -``__geom_z2__``; substitute your geometry column's name per the convention in -:ref:`trino_companion_columns` (e.g. a ``path`` column is pruned via -``__path_bbox__`` / ``__path_xz2__``). +Direct-SQL consumers (JDBC, BI tools) write their own SQL. Because the connector derives +all pruning from the ``ST_*`` call itself, hand-written SQL needs no special shapes — a +plain ``ST_*`` predicate gets the same Z2/XZ2 partition pruning, per-file bbox-stat +pruning, and page-source bbox filter as a query issued through the data store. The +examples below run against a table whose geometry column is named ``geom``, so its +companions are ``__geom_bbox__`` and ``__geom_z2__``; substitute your geometry column's +name per the convention in :ref:`trino_companion_columns` (e.g. a ``path`` column is pruned +via ``__path_bbox__`` / ``__path_xz2__``). .. code-block:: sql - -- BBOX: bbox-overlap on the struct fields. Both connectors prune files via - -- per-leaf Parquet stats on __geom_bbox__; SI also pushes Z2 partition pruning. + -- ST_Intersects with an axis-aligned rectangle. The connector derives Z2/XZ2 + -- partition pruning (Layer 1) and per-file bbox-stat pruning (Layer 2) from the + -- call; because the query is a rectangle on a point/Z2 column, its page source + -- accepts rows whose bbox is inside without decoding the WKB (Layer 3 short-circuit). SELECT COUNT(*) FROM spatial_iceberg.spatial.observations - WHERE "__geom_bbox__".xmax >= -80 AND "__geom_bbox__".xmin <= -70 - AND "__geom_bbox__".ymax >= 37 AND "__geom_bbox__".ymin <= 45; + WHERE ST_Intersects(ST_GeomFromBinary(geom), + ST_GeometryFromText('POLYGON ((-80 37, -70 37, -70 45, -80 45, -80 37))')); - -- ST_Intersects with row-level shortcut. The leading bbox-overlap conjunct - -- triggers file-level + Z2 pruning; the CASE WHEN short-circuits ST_Intersects - -- (and the WKB decode that precedes it) for rows whose bbox is fully inside. + -- ST_Intersects with a non-rectangular polygon. Same Layer 1/2 pruning; the page + -- source rejects rows whose bbox cannot overlap before the exact ST_Intersects runs. SELECT COUNT(*) FROM spatial_iceberg.spatial.observations - WHERE ("__geom_bbox__".xmax >= -80 AND "__geom_bbox__".xmin <= -70 - AND "__geom_bbox__".ymax >= 37 AND "__geom_bbox__".ymin <= 45) - AND CASE WHEN "__geom_bbox__".xmin >= -80 AND "__geom_bbox__".xmax <= -70 - AND "__geom_bbox__".ymin >= 37 AND "__geom_bbox__".ymax <= 45 - THEN TRUE - ELSE ST_Intersects(ST_GeomFromBinary(geom), ST_GeometryFromText('POLYGON ((-80 37, -70 37, -70 45, -80 45, -80 37))')) - END; - - -- ST_Within(geom, axis-aligned-rectangle): equivalent to bbox-contained, no - -- row-level ST_Within needed. Trino consolidates the two conjuncts into BETWEEN. + WHERE ST_Intersects(ST_GeomFromBinary(geom), + ST_GeometryFromText('POLYGON ((-80 37, -75 41, -70 45, -80 45, -80 37))')); + + -- ST_Within(geom, rectangle). bbox-overlap is a necessary prefilter; the exact + -- ST_Within decides membership on the survivors. SELECT COUNT(*) FROM spatial_iceberg.spatial.observations - WHERE ("__geom_bbox__".xmax >= -80 AND "__geom_bbox__".xmin <= -70 - AND "__geom_bbox__".ymax >= 37 AND "__geom_bbox__".ymin <= 45) - AND ("__geom_bbox__".xmin >= -80 AND "__geom_bbox__".xmax <= -70 - AND "__geom_bbox__".ymin >= 37 AND "__geom_bbox__".ymax <= 45); - - -- DWITHIN: outer-bbox-overlap (file pruning) + CASE WHEN inner-inscribed-rect - -- (sufficient for distance ≤ d) ELSE exact spherical distance. The inner - -- rectangle's corners land at distance 0.9 × d from ref; rows whose bbox fits - -- inside it skip ST_Distance entirely. + WHERE ST_Within(ST_GeomFromBinary(geom), + ST_GeometryFromText('POLYGON ((-80 37, -70 37, -70 45, -80 45, -80 37))')); + + -- DWITHIN: the data store emits an outer ST_Intersects rectangle (the reference + -- expanded by the radius) AND the exact spherical distance test. The connector + -- derives Layer 1/2 pruning + page-source rejection from the outer rectangle; the + -- exact ST_Distance runs only on the survivors. SELECT COUNT(*) FROM spatial_iceberg.spatial.observations - WHERE ("__geom_bbox__".xmax >= -77.94 AND "__geom_bbox__".xmin <= -76.14 - AND "__geom_bbox__".ymax >= 37.91 AND "__geom_bbox__".ymin <= 39.91) - AND CASE WHEN "__geom_bbox__".xmin >= -77.66 AND "__geom_bbox__".xmax <= -76.42 - AND "__geom_bbox__".ymin >= 38.18 AND "__geom_bbox__".ymax <= 39.64 - THEN TRUE - ELSE ST_Distance( - to_spherical_geography(ST_GeomFromBinary(geom)), - to_spherical_geography(ST_GeometryFromText('POINT (-77.04 38.91)')) - ) <= 100000 - END; + WHERE ST_Intersects(ST_GeomFromBinary(geom), + ST_GeometryFromText('POLYGON ((-77.94 37.91, -76.14 37.91, -76.14 39.91, -77.94 39.91, -77.94 37.91))')) + AND ST_Distance( + to_spherical_geography(ST_GeomFromBinary(geom)), + to_spherical_geography(ST_GeometryFromText('POINT (-77.04 38.91)')) + ) <= 100000; + + -- Raw bbox-overlap on the struct sub-fields is still recognized, for BI tools that + -- cannot emit ST_*: it drives Layer 1 + Layer 2 pruning but is NOT an exact spatial + -- test — a float32 stored bbox admits rows up to ½ ulp outside the envelope — so pair + -- it with an ST_* predicate when exactness matters. + SELECT COUNT(*) FROM spatial_iceberg.spatial.observations + WHERE "__geom_bbox__".xmax >= -80 AND "__geom_bbox__".xmin <= -70 + AND "__geom_bbox__".ymax >= 37 AND "__geom_bbox__".ymin <= 45; On **both** catalogs, ``geom`` is ``VARBINARY`` (raw WKB) — there is no Geometry-type overlay — so every spatial function call must wrap it with ``ST_GeomFromBinary(geom)``, @@ -131,22 +131,20 @@ spatial_iceberg --schema spatial``): SHOW TABLES; SELECT COUNT(*) FROM observations; - -- Check the three pruning layers are active. EXPLAIN shows layers 2 & 3 - -- directly; layer 1 (truncate-string partition projection) is NOT surfaced - -- in `constraint on [...]` — use EXPLAIN ANALYZE and confirm - -- `Splits: N` and `Input: M rows` are much smaller than the table's totals. + -- Check the pruning layers are active. Layer 1 (truncate-string partition + -- projection) is NOT surfaced in `constraint on [...]` — use EXPLAIN ANALYZE and + -- confirm `Splits: N` and `Input: M rows` are much smaller than the table totals. -- 1. EXPLAIN ANALYZE shows reduced scan-input rows/splits — Z2/XZ2 partition pushdown -- 2. `geom_bbox_xmax >= ... AND geom_bbox_xmin <= ...` in filterPredicate — bbox-stat pushdown - -- 3. (Optional) `CASE WHEN ... THEN TRUE ELSE st_intersects(...)` for row-level shortcut + -- 3. Layer 3 (page-source bbox filter) is not a plan node: for a rectangle + -- ST_Intersects on a point/Z2 column the `st_intersects` residual disappears + -- entirely (claimed enforced); otherwise it remains as the residual while rows + -- are pre-rejected in the page source — seen as EXPLAIN ANALYZE filter input + -- dropping below the scanned row count. EXPLAIN SELECT COUNT(*) FROM observations - WHERE ("__geom_bbox__".xmax >= -80 AND "__geom_bbox__".xmin <= -70 - AND "__geom_bbox__".ymax >= 37 AND "__geom_bbox__".ymin <= 45) - AND CASE WHEN "__geom_bbox__".xmin >= -80 AND "__geom_bbox__".xmax <= -70 - AND "__geom_bbox__".ymin >= 37 AND "__geom_bbox__".ymax <= 45 - THEN TRUE - ELSE ST_Intersects(ST_GeomFromBinary(geom), ST_GeometryFromText('POLYGON ((-80 37, -70 37, -70 45, -80 45, -80 37))')) - END; + WHERE ST_Intersects(ST_GeomFromBinary(geom), + ST_GeometryFromText('POLYGON ((-80 37, -70 37, -70 45, -80 45, -80 37))')); -- Inspect Iceberg metadata tables (use single quotes around the SQL when running -- through bash to avoid the shell eating $files / $manifests). From c125629819ba885d287aa177b587f0e8815e76b5 Mon Sep 17 00:00:00 2001 From: Chris Dobbins Date: Wed, 22 Jul 2026 15:47:58 +0000 Subject: [PATCH 5/5] Replaced magic-string st_* function comparisons with constants and provided case-insensitive matching --- .../connector/SpatialConnectorMetadata.java | 58 +++++++++++++------ 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java index 36d969babb6..61a5f7bba7d 100644 --- a/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java +++ b/geomesa-trino/geomesa-trino-plugin/src/main/java/org/locationtech/geomesa/trino/spatial/iceberg/connector/SpatialConnectorMetadata.java @@ -82,6 +82,20 @@ public class SpatialConnectorMetadata implements ConnectorMetadata { /** Ordered sub-field names of a {@code ___bbox__} struct column. */ private static final List BBOX_FIELDS = List.of("xmin", "ymin", "xmax", "ymax"); + // Trino function names this connector recognizes, lowercased for consistency. + // n.b. Trino itself resolves function names case-insensitively + private static final String ST_INTERSECTS = "st_intersects"; + private static final String ST_WITHIN = "st_within"; + private static final String ST_CONTAINS = "st_contains"; + private static final String ST_CROSSES = "st_crosses"; + private static final String ST_TOUCHES = "st_touches"; + private static final String ST_OVERLAPS = "st_overlaps"; + private static final String ST_EQUALS = "st_equals"; + private static final String ST_GEOMETRY_FROM_TEXT = "st_geometryfromtext"; + private static final String ST_GEOM_FROM_TEXT = "st_geomfromtext"; + private static final String ST_GEOM_FROM_BINARY = "st_geomfrombinary"; + private static final String BBOX_PATTERN = "bbox_pattern"; + // Cached reflective accessors for foreign-classloader JTS Geometry/Envelope values // (see envelopeOf). Keyed by concrete Class so each lookup runs at most once. private static final Map, Method> GEOM_ENVELOPE_METHOD = new ConcurrentHashMap<>(); @@ -166,7 +180,7 @@ public Optional> applyFilter( List bboxes = tryExtractBboxPatternMatches(constraint.getExpression()); if (bboxes.isEmpty()) return delegate.applyFilter(session, handle, constraint); matches = bboxes.stream() - .map(bp -> new SpatialMatch(bp.envelope(), "bbox_pattern", bp.geomName())) + .map(bp -> new SpatialMatch(bp.envelope(), BBOX_PATTERN, bp.geomName())) .toList(); } @@ -263,7 +277,7 @@ public Optional> applyFilter( if (bboxShortCircuit && matches.size() == 1 && resultHandle instanceof IcebergTableHandle ith) { SpatialMatch m = matches.get(0); GeometryColumn g = geoms.get(m.geomName()); - if ("st_intersects".equals(m.functionName()) + if (ST_INTERSECTS.equalsIgnoreCase(m.functionName()) && g != null && g.partition().map(p -> p.kind() == SpatialIndexKind.Z2).orElse(false) && queryIsRectangle(constraint.getExpression())) { @@ -388,6 +402,18 @@ private static Domain realBetween(float low, float high) { (long) Float.floatToIntBits(high), true))), false); } + /** The call's function name lowercased, for case-insensitive matching against the + * {@code ST_*} name constants (Trino resolves function names case-insensitively). */ + private static String functionNameOf(Call call) { + return call.getFunctionName().getName().toLowerCase(Locale.ROOT); + } + + /** True iff {@code call} names the given function, compared case-insensitively. + * {@code lowerCaseName} must be one of the lowercased {@code ST_*} constants above. */ + private static boolean matchesFunction(Call call, String lowerCaseName) { + return functionNameOf(call).equals(lowerCaseName); + } + /** * Function names whose row-side argument is a geometry and whose query-side * argument is a geometry literal — the shape this connector can extract an @@ -398,17 +424,17 @@ private static Domain realBetween(float low, float high) { * ST_Distance}-based predicates carry no extractable envelope; the datastore * lowers DWITHIN to bbox comparisons the pattern-reconstruction path picks up. */ - private static boolean isSpatialPredicateName(String fn) { - return switch (fn) { - case "st_intersects", "st_within", "st_contains", - "st_crosses", "st_touches", "st_overlaps", "st_equals" -> true; + private static boolean isSpatialPredicate(String fn) { + return switch (fn.toLowerCase()) { + case ST_INTERSECTS, ST_WITHIN, ST_CONTAINS, + ST_CROSSES, ST_TOUCHES, ST_OVERLAPS, ST_EQUALS -> true; default -> false; }; } /** * Extracts the envelope of the geometry-constant argument of a spatial - * function call (see {@link #isSpatialPredicateName}; the caller has + * function call (see {@link #isSpatialPredicate}; the caller has * already verified the function name). Used by collectSpatialMatches. */ Optional tryExtractEnvelope(Call call) { @@ -425,8 +451,7 @@ Optional tryExtractEnvelope(Call call) { } // Form 2: ST_GeometryFromText(varchar_literal) not yet folded. if (arg instanceof Call wktCall) { - String wktFn = wktCall.getFunctionName().getName().toLowerCase(Locale.ROOT); - if ((wktFn.equals("st_geometryfromtext") || wktFn.equals("st_geomfromtext")) + if ((matchesFunction(wktCall, ST_GEOMETRY_FROM_TEXT) || matchesFunction(wktCall, ST_GEOM_FROM_TEXT)) && !wktCall.getArguments().isEmpty() && wktCall.getArguments().get(0) instanceof Constant wktConst) { try { @@ -513,8 +538,7 @@ private boolean queryIsRectangle(ConnectorExpression expr) { return isRectangleGeom(c.getValue()); } if (arg instanceof Call wkt) { - String fn = wkt.getFunctionName().getName().toLowerCase(Locale.ROOT); - if ((fn.equals("st_geometryfromtext") || fn.equals("st_geomfromtext")) + if ((matchesFunction(wkt, ST_GEOMETRY_FROM_TEXT) || matchesFunction(wkt, ST_GEOM_FROM_TEXT)) && !wkt.getArguments().isEmpty() && wkt.getArguments().get(0) instanceof Constant wc && wc.getValue() instanceof Slice s) { @@ -534,7 +558,7 @@ private boolean queryIsRectangle(ConnectorExpression expr) { * which the engine must still apply. Only the spatial predicate is claimed enforced. */ static ConnectorExpression dropSpatialCall(ConnectorExpression expr) { if (expr instanceof Call call) { - if ("st_intersects".equals(call.getFunctionName().getName().toLowerCase(Locale.ROOT))) { + if (matchesFunction(call, ST_INTERSECTS)) { return Constant.TRUE; } if (AND_FUNCTION_NAME.equals(call.getFunctionName())) { @@ -553,7 +577,7 @@ private static Call findSpatialCall(ConnectorExpression expr) { if (!(expr instanceof Call call)) { return null; } - if ("st_intersects".equals(call.getFunctionName().getName().toLowerCase(Locale.ROOT))) { + if (matchesFunction(call, ST_INTERSECTS)) { return call; } if (AND_FUNCTION_NAME.equals(call.getFunctionName())) { @@ -612,8 +636,8 @@ List findAllSpatialMatches(ConnectorExpression expr) { private void collectSpatialMatches(ConnectorExpression expr, List acc) { if (!(expr instanceof Call call)) return; - String fn = call.getFunctionName().getName().toLowerCase(Locale.ROOT); - if (isSpatialPredicateName(fn)) { + String fn = functionNameOf(call); + if (isSpatialPredicate(fn)) { Optional envOpt = tryExtractEnvelope(call); Optional geomNameOpt = extractGeomColumnName(call); if (envOpt.isPresent() && geomNameOpt.isPresent()) { @@ -671,7 +695,7 @@ private List orSpatialMatches(Call or) { } List out = new ArrayList<>(); for (Map.Entry> e : unionByGeom.entrySet()) { - out.add(new SpatialMatch(List.copyOf(e.getValue()), "$or", e.getKey())); + out.add(new SpatialMatch(List.copyOf(e.getValue()), OR_FUNCTION_NAME.getName(), e.getKey())); } return out; } @@ -689,7 +713,7 @@ static Optional extractGeomColumnName(Call call) { } // Wrapped: ST_GeomFromBinary(var). if (arg instanceof Call inner - && "st_geomfrombinary".equals(inner.getFunctionName().getName().toLowerCase(Locale.ROOT)) + && matchesFunction(inner, ST_GEOM_FROM_BINARY) && !inner.getArguments().isEmpty() && inner.getArguments().get(0) instanceof Variable v) { return Optional.of(v.getName());