From e5f72f912d352227bdb92a336891068c026b29c7 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Mon, 30 Jun 2025 18:54:31 +0200 Subject: [PATCH 1/4] Use new objects per thread instead of per record --- .../definition/functions/st_geohash.json | 2 +- .../definition/functions/st_geohex.json | 2 +- .../definition/functions/st_geotile.json | 2 +- ...romFieldAndLiteralAndLiteralEvaluator.java | 11 ++--- ...ocValuesAndLiteralAndLiteralEvaluator.java | 11 ++--- ...romFieldAndLiteralAndLiteralEvaluator.java | 11 ++--- ...ocValuesAndLiteralAndLiteralEvaluator.java | 11 ++--- ...romFieldAndLiteralAndLiteralEvaluator.java | 11 ++--- ...ocValuesAndLiteralAndLiteralEvaluator.java | 11 ++--- .../spatial/GeoHexBoundedPredicate.java | 9 +---- .../function/scalar/spatial/StGeohash.java | 40 ++++++++++++++----- .../function/scalar/spatial/StGeohex.java | 40 ++++++++++++++----- .../function/scalar/spatial/StGeotile.java | 40 ++++++++++++++----- .../spatial/SpatialGridFunctionTestCase.java | 2 +- .../scalar/spatial/StGeohashTests.java | 2 +- .../scalar/spatial/StGeohexTests.java | 2 +- .../scalar/spatial/StGeotileTests.java | 2 +- 17 files changed, 132 insertions(+), 77 deletions(-) diff --git a/docs/reference/query-languages/esql/kibana/definition/functions/st_geohash.json b/docs/reference/query-languages/esql/kibana/definition/functions/st_geohash.json index d2fc83008c150..43633b336453a 100644 --- a/docs/reference/query-languages/esql/kibana/definition/functions/st_geohash.json +++ b/docs/reference/query-languages/esql/kibana/definition/functions/st_geohash.json @@ -52,4 +52,4 @@ ], "preview" : true, "snapshot_only" : true -} \ No newline at end of file +} diff --git a/docs/reference/query-languages/esql/kibana/definition/functions/st_geohex.json b/docs/reference/query-languages/esql/kibana/definition/functions/st_geohex.json index 9a3a04cb0a7f8..f29db14ed50e7 100644 --- a/docs/reference/query-languages/esql/kibana/definition/functions/st_geohex.json +++ b/docs/reference/query-languages/esql/kibana/definition/functions/st_geohex.json @@ -55,4 +55,4 @@ ], "preview" : true, "snapshot_only" : true -} \ No newline at end of file +} diff --git a/docs/reference/query-languages/esql/kibana/definition/functions/st_geotile.json b/docs/reference/query-languages/esql/kibana/definition/functions/st_geotile.json index 06df5e3076fea..d728f186fc5ae 100644 --- a/docs/reference/query-languages/esql/kibana/definition/functions/st_geotile.json +++ b/docs/reference/query-languages/esql/kibana/definition/functions/st_geotile.json @@ -52,4 +52,4 @@ ], "preview" : true, "snapshot_only" : true -} \ No newline at end of file +} diff --git a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashFromFieldAndLiteralAndLiteralEvaluator.java b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashFromFieldAndLiteralAndLiteralEvaluator.java index 2e4b8e7d538d8..982252f3d125d 100644 --- a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashFromFieldAndLiteralAndLiteralEvaluator.java +++ b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashFromFieldAndLiteralAndLiteralEvaluator.java @@ -7,6 +7,7 @@ import java.lang.IllegalArgumentException; import java.lang.Override; import java.lang.String; +import java.util.function.Function; import org.elasticsearch.compute.data.Block; import org.elasticsearch.compute.data.BytesRefBlock; import org.elasticsearch.compute.data.LongBlock; @@ -72,7 +73,7 @@ public LongBlock eval(int positionCount, BytesRefBlock inBlock) { @Override public String toString() { - return "StGeohashFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + ", bounds=" + bounds + "]"; + return "StGeohashFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + "]"; } @Override @@ -97,10 +98,10 @@ static class Factory implements EvalOperator.ExpressionEvaluator.Factory { private final EvalOperator.ExpressionEvaluator.Factory in; - private final StGeohash.GeoHashBoundedGrid bounds; + private final Function bounds; public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory in, - StGeohash.GeoHashBoundedGrid bounds) { + Function bounds) { this.source = source; this.in = in; this.bounds = bounds; @@ -108,12 +109,12 @@ public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory in, @Override public StGeohashFromFieldAndLiteralAndLiteralEvaluator get(DriverContext context) { - return new StGeohashFromFieldAndLiteralAndLiteralEvaluator(source, in.get(context), bounds, context); + return new StGeohashFromFieldAndLiteralAndLiteralEvaluator(source, in.get(context), bounds.apply(context), context); } @Override public String toString() { - return "StGeohashFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + ", bounds=" + bounds + "]"; + return "StGeohashFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + "]"; } } } diff --git a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator.java b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator.java index 862b2b90c9b2d..35febe6a846d1 100644 --- a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator.java +++ b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator.java @@ -7,6 +7,7 @@ import java.lang.IllegalArgumentException; import java.lang.Override; import java.lang.String; +import java.util.function.Function; import org.elasticsearch.compute.data.Block; import org.elasticsearch.compute.data.LongBlock; import org.elasticsearch.compute.data.Page; @@ -71,7 +72,7 @@ public LongBlock eval(int positionCount, LongBlock encodedBlock) { @Override public String toString() { - return "StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + ", bounds=" + bounds + "]"; + return "StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + "]"; } @Override @@ -96,10 +97,10 @@ static class Factory implements EvalOperator.ExpressionEvaluator.Factory { private final EvalOperator.ExpressionEvaluator.Factory encoded; - private final StGeohash.GeoHashBoundedGrid bounds; + private final Function bounds; public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory encoded, - StGeohash.GeoHashBoundedGrid bounds) { + Function bounds) { this.source = source; this.encoded = encoded; this.bounds = bounds; @@ -107,12 +108,12 @@ public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory encoded, @Override public StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator get(DriverContext context) { - return new StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator(source, encoded.get(context), bounds, context); + return new StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator(source, encoded.get(context), bounds.apply(context), context); } @Override public String toString() { - return "StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + ", bounds=" + bounds + "]"; + return "StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + "]"; } } } diff --git a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexFromFieldAndLiteralAndLiteralEvaluator.java b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexFromFieldAndLiteralAndLiteralEvaluator.java index 24e070c46adda..9d7fd6913c058 100644 --- a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexFromFieldAndLiteralAndLiteralEvaluator.java +++ b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexFromFieldAndLiteralAndLiteralEvaluator.java @@ -7,6 +7,7 @@ import java.lang.IllegalArgumentException; import java.lang.Override; import java.lang.String; +import java.util.function.Function; import org.elasticsearch.compute.data.Block; import org.elasticsearch.compute.data.BytesRefBlock; import org.elasticsearch.compute.data.LongBlock; @@ -72,7 +73,7 @@ public LongBlock eval(int positionCount, BytesRefBlock inBlock) { @Override public String toString() { - return "StGeohexFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + ", bounds=" + bounds + "]"; + return "StGeohexFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + "]"; } @Override @@ -97,10 +98,10 @@ static class Factory implements EvalOperator.ExpressionEvaluator.Factory { private final EvalOperator.ExpressionEvaluator.Factory in; - private final StGeohex.GeoHexBoundedGrid bounds; + private final Function bounds; public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory in, - StGeohex.GeoHexBoundedGrid bounds) { + Function bounds) { this.source = source; this.in = in; this.bounds = bounds; @@ -108,12 +109,12 @@ public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory in, @Override public StGeohexFromFieldAndLiteralAndLiteralEvaluator get(DriverContext context) { - return new StGeohexFromFieldAndLiteralAndLiteralEvaluator(source, in.get(context), bounds, context); + return new StGeohexFromFieldAndLiteralAndLiteralEvaluator(source, in.get(context), bounds.apply(context), context); } @Override public String toString() { - return "StGeohexFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + ", bounds=" + bounds + "]"; + return "StGeohexFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + "]"; } } } diff --git a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator.java b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator.java index 0ac32cbdbedad..2a37c301bbed4 100644 --- a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator.java +++ b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator.java @@ -7,6 +7,7 @@ import java.lang.IllegalArgumentException; import java.lang.Override; import java.lang.String; +import java.util.function.Function; import org.elasticsearch.compute.data.Block; import org.elasticsearch.compute.data.LongBlock; import org.elasticsearch.compute.data.Page; @@ -71,7 +72,7 @@ public LongBlock eval(int positionCount, LongBlock encodedBlock) { @Override public String toString() { - return "StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + ", bounds=" + bounds + "]"; + return "StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + "]"; } @Override @@ -96,10 +97,10 @@ static class Factory implements EvalOperator.ExpressionEvaluator.Factory { private final EvalOperator.ExpressionEvaluator.Factory encoded; - private final StGeohex.GeoHexBoundedGrid bounds; + private final Function bounds; public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory encoded, - StGeohex.GeoHexBoundedGrid bounds) { + Function bounds) { this.source = source; this.encoded = encoded; this.bounds = bounds; @@ -107,12 +108,12 @@ public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory encoded, @Override public StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator get(DriverContext context) { - return new StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator(source, encoded.get(context), bounds, context); + return new StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator(source, encoded.get(context), bounds.apply(context), context); } @Override public String toString() { - return "StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + ", bounds=" + bounds + "]"; + return "StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + "]"; } } } diff --git a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileFromFieldAndLiteralAndLiteralEvaluator.java b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileFromFieldAndLiteralAndLiteralEvaluator.java index 13a8da4ad0c9c..4e8fe75c80d9d 100644 --- a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileFromFieldAndLiteralAndLiteralEvaluator.java +++ b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileFromFieldAndLiteralAndLiteralEvaluator.java @@ -7,6 +7,7 @@ import java.lang.IllegalArgumentException; import java.lang.Override; import java.lang.String; +import java.util.function.Function; import org.elasticsearch.compute.data.Block; import org.elasticsearch.compute.data.BytesRefBlock; import org.elasticsearch.compute.data.LongBlock; @@ -72,7 +73,7 @@ public LongBlock eval(int positionCount, BytesRefBlock inBlock) { @Override public String toString() { - return "StGeotileFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + ", bounds=" + bounds + "]"; + return "StGeotileFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + "]"; } @Override @@ -97,10 +98,10 @@ static class Factory implements EvalOperator.ExpressionEvaluator.Factory { private final EvalOperator.ExpressionEvaluator.Factory in; - private final StGeotile.GeoTileBoundedGrid bounds; + private final Function bounds; public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory in, - StGeotile.GeoTileBoundedGrid bounds) { + Function bounds) { this.source = source; this.in = in; this.bounds = bounds; @@ -108,12 +109,12 @@ public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory in, @Override public StGeotileFromFieldAndLiteralAndLiteralEvaluator get(DriverContext context) { - return new StGeotileFromFieldAndLiteralAndLiteralEvaluator(source, in.get(context), bounds, context); + return new StGeotileFromFieldAndLiteralAndLiteralEvaluator(source, in.get(context), bounds.apply(context), context); } @Override public String toString() { - return "StGeotileFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + ", bounds=" + bounds + "]"; + return "StGeotileFromFieldAndLiteralAndLiteralEvaluator[" + "in=" + in + "]"; } } } diff --git a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator.java b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator.java index c6dc1d23b9f3d..b6951f2ab47a3 100644 --- a/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator.java +++ b/x-pack/plugin/esql/src/main/generated/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator.java @@ -7,6 +7,7 @@ import java.lang.IllegalArgumentException; import java.lang.Override; import java.lang.String; +import java.util.function.Function; import org.elasticsearch.compute.data.Block; import org.elasticsearch.compute.data.LongBlock; import org.elasticsearch.compute.data.Page; @@ -71,7 +72,7 @@ public LongBlock eval(int positionCount, LongBlock encodedBlock) { @Override public String toString() { - return "StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + ", bounds=" + bounds + "]"; + return "StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + "]"; } @Override @@ -96,10 +97,10 @@ static class Factory implements EvalOperator.ExpressionEvaluator.Factory { private final EvalOperator.ExpressionEvaluator.Factory encoded; - private final StGeotile.GeoTileBoundedGrid bounds; + private final Function bounds; public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory encoded, - StGeotile.GeoTileBoundedGrid bounds) { + Function bounds) { this.source = source; this.encoded = encoded; this.bounds = bounds; @@ -107,12 +108,12 @@ public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory encoded, @Override public StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator get(DriverContext context) { - return new StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator(source, encoded.get(context), bounds, context); + return new StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator(source, encoded.get(context), bounds.apply(context), context); } @Override public String toString() { - return "StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + ", bounds=" + bounds + "]"; + return "StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator[" + "encoded=" + encoded + "]"; } } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/GeoHexBoundedPredicate.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/GeoHexBoundedPredicate.java index c0d8b25e07860..368e2e37423ec 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/GeoHexBoundedPredicate.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/GeoHexBoundedPredicate.java @@ -17,20 +17,15 @@ public class GeoHexBoundedPredicate { private final boolean crossesDateline; - private final GeoBoundingBox bbox; + private final GeoBoundingBox bbox, scratch; GeoHexBoundedPredicate(GeoBoundingBox bbox) { this.crossesDateline = bbox.right() < bbox.left(); - // TODO remove this once we get serverless flaky tests to pass - // assert this.crossesDateline == false; this.bbox = bbox; + this.scratch = new GeoBoundingBox(new org.elasticsearch.common.geo.GeoPoint(), new org.elasticsearch.common.geo.GeoPoint()); } public boolean validHex(long hex) { - GeoBoundingBox scratch = new GeoBoundingBox( - new org.elasticsearch.common.geo.GeoPoint(), - new org.elasticsearch.common.geo.GeoPoint() - ); H3SphericalUtil.computeGeoBounds(hex, scratch); if (bbox.top() > scratch.bottom() && bbox.bottom() < scratch.top()) { if (scratch.left() > scratch.right()) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohash.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohash.java index b4a302824c924..af0a1c3f465c4 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohash.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohash.java @@ -15,6 +15,7 @@ import org.elasticsearch.compute.ann.Fixed; import org.elasticsearch.compute.data.BytesRefBlock; import org.elasticsearch.compute.data.LongBlock; +import org.elasticsearch.compute.operator.DriverContext; import org.elasticsearch.compute.operator.EvalOperator; import org.elasticsearch.geometry.Point; import org.elasticsearch.geometry.utils.Geohash; @@ -33,6 +34,7 @@ import java.io.IOException; +import static org.elasticsearch.compute.ann.Fixed.Scope.THREAD_LOCAL; import static org.elasticsearch.xpack.esql.core.type.DataType.LONG; import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.GEO; @@ -52,12 +54,10 @@ public class StGeohash extends SpatialGridFunction implements EvaluatorMapper { */ protected static class GeoHashBoundedGrid implements BoundedGrid { private final int precision; - private final GeoBoundingBox bbox; private final GeoHashBoundedPredicate bounds; - public GeoHashBoundedGrid(int precision, GeoBoundingBox bbox) { + private GeoHashBoundedGrid(int precision, GeoBoundingBox bbox) { this.precision = checkPrecisionRange(precision); - this.bbox = bbox; this.bounds = new GeoHashBoundedPredicate(precision, bbox); } @@ -75,9 +75,18 @@ public int precision() { return precision; } - @Override - public String toString() { - return "[" + bbox + "]"; + protected static class Factory { + private final int precision; + private final GeoBoundingBox bbox; + + Factory(int precision, GeoBoundingBox bbox) { + this.precision = checkPrecisionRange(precision); + this.bbox = bbox; + } + + public GeoHashBoundedGrid get(DriverContext context) { + return new GeoHashBoundedGrid(precision, bbox); + } } } @@ -175,10 +184,14 @@ public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvalua } GeoBoundingBox bbox = asGeoBoundingBox(bounds.fold(toEvaluator.foldCtx())); int precision = (int) parameter.fold(toEvaluator.foldCtx()); - GeoHashBoundedGrid bounds = new GeoHashBoundedGrid(precision, bbox); + GeoHashBoundedGrid.Factory bounds = new GeoHashBoundedGrid.Factory(precision, bbox); return spatialDocsValues - ? new StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator.Factory(source(), toEvaluator.apply(spatialField()), bounds) - : new StGeohashFromFieldAndLiteralAndLiteralEvaluator.Factory(source(), toEvaluator.apply(spatialField), bounds); + ? new StGeohashFromFieldDocValuesAndLiteralAndLiteralEvaluator.Factory( + source(), + toEvaluator.apply(spatialField()), + bounds::get + ) + : new StGeohashFromFieldAndLiteralAndLiteralEvaluator.Factory(source(), toEvaluator.apply(spatialField), bounds::get); } else { int precision = checkPrecisionRange((int) parameter.fold(toEvaluator.foldCtx())); return spatialDocsValues @@ -211,7 +224,12 @@ static void fromFieldDocValuesAndLiteral(LongBlock.Builder results, int p, LongB } @Evaluator(extraName = "FromFieldAndLiteralAndLiteral", warnExceptions = { IllegalArgumentException.class }) - static void fromFieldAndLiteralAndLiteral(LongBlock.Builder results, int p, BytesRefBlock in, @Fixed GeoHashBoundedGrid bounds) { + static void fromFieldAndLiteralAndLiteral( + LongBlock.Builder results, + int p, + BytesRefBlock in, + @Fixed(includeInToString = false, scope = THREAD_LOCAL) GeoHashBoundedGrid bounds + ) { fromWKB(results, p, in, bounds); } @@ -220,7 +238,7 @@ static void fromFieldDocValuesAndLiteralAndLiteral( LongBlock.Builder results, int p, LongBlock encoded, - @Fixed GeoHashBoundedGrid bounds + @Fixed(includeInToString = false, scope = THREAD_LOCAL) GeoHashBoundedGrid bounds ) { fromEncodedLong(results, p, encoded, bounds); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohex.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohex.java index 23ca277f4b3b6..820f443ef2726 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohex.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohex.java @@ -15,6 +15,7 @@ import org.elasticsearch.compute.ann.Fixed; import org.elasticsearch.compute.data.BytesRefBlock; import org.elasticsearch.compute.data.LongBlock; +import org.elasticsearch.compute.operator.DriverContext; import org.elasticsearch.compute.operator.EvalOperator; import org.elasticsearch.geometry.Point; import org.elasticsearch.h3.H3; @@ -34,6 +35,7 @@ import java.io.IOException; +import static org.elasticsearch.compute.ann.Fixed.Scope.THREAD_LOCAL; import static org.elasticsearch.xpack.esql.core.type.DataType.LONG; import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.GEO; @@ -49,12 +51,10 @@ public class StGeohex extends SpatialGridFunction implements EvaluatorMapper { */ protected static class GeoHexBoundedGrid implements BoundedGrid { private final int precision; - private final GeoBoundingBox bbox; private final GeoHexBoundedPredicate bounds; - public GeoHexBoundedGrid(int precision, GeoBoundingBox bbox) { + private GeoHexBoundedGrid(int precision, GeoBoundingBox bbox) { this.precision = checkPrecisionRange(precision); - this.bbox = bbox; this.bounds = new GeoHexBoundedPredicate(bbox); } @@ -73,9 +73,18 @@ public int precision() { return precision; } - @Override - public String toString() { - return "[" + bbox + "]"; + protected static class Factory { + private final int precision; + private final GeoBoundingBox bbox; + + Factory(int precision, GeoBoundingBox bbox) { + this.precision = checkPrecisionRange(precision); + this.bbox = bbox; + } + + public GeoHexBoundedGrid get(DriverContext context) { + return new GeoHexBoundedGrid(precision, bbox); + } } } @@ -178,10 +187,14 @@ public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvalua } GeoBoundingBox bbox = asGeoBoundingBox(bounds.fold(toEvaluator.foldCtx())); int precision = (int) parameter.fold(toEvaluator.foldCtx()); - GeoHexBoundedGrid bounds = new GeoHexBoundedGrid(precision, bbox); + GeoHexBoundedGrid.Factory bounds = new GeoHexBoundedGrid.Factory(precision, bbox); return spatialDocsValues - ? new StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator.Factory(source(), toEvaluator.apply(spatialField()), bounds) - : new StGeohexFromFieldAndLiteralAndLiteralEvaluator.Factory(source(), toEvaluator.apply(spatialField), bounds); + ? new StGeohexFromFieldDocValuesAndLiteralAndLiteralEvaluator.Factory( + source(), + toEvaluator.apply(spatialField()), + bounds::get + ) + : new StGeohexFromFieldAndLiteralAndLiteralEvaluator.Factory(source(), toEvaluator.apply(spatialField), bounds::get); } else { int precision = checkPrecisionRange((int) parameter.fold(toEvaluator.foldCtx())); return spatialDocsValues @@ -214,7 +227,12 @@ static void fromFieldDocValuesAndLiteral(LongBlock.Builder results, int p, LongB } @Evaluator(extraName = "FromFieldAndLiteralAndLiteral", warnExceptions = { IllegalArgumentException.class }) - static void fromFieldAndLiteralAndLiteral(LongBlock.Builder results, int p, BytesRefBlock in, @Fixed GeoHexBoundedGrid bounds) { + static void fromFieldAndLiteralAndLiteral( + LongBlock.Builder results, + int p, + BytesRefBlock in, + @Fixed(includeInToString = false, scope = THREAD_LOCAL) GeoHexBoundedGrid bounds + ) { fromWKB(results, p, in, bounds); } @@ -223,7 +241,7 @@ static void fromFieldDocValuesAndLiteralAndLiteral( LongBlock.Builder results, int p, LongBlock encoded, - @Fixed GeoHexBoundedGrid bounds + @Fixed(includeInToString = false, scope = THREAD_LOCAL) GeoHexBoundedGrid bounds ) { fromEncodedLong(results, p, encoded, bounds); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotile.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotile.java index 81b4e676ceafd..6dabe5a6a7d04 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotile.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotile.java @@ -15,6 +15,7 @@ import org.elasticsearch.compute.ann.Fixed; import org.elasticsearch.compute.data.BytesRefBlock; import org.elasticsearch.compute.data.LongBlock; +import org.elasticsearch.compute.operator.DriverContext; import org.elasticsearch.compute.operator.EvalOperator; import org.elasticsearch.geometry.Point; import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileBoundedPredicate; @@ -33,6 +34,7 @@ import java.io.IOException; +import static org.elasticsearch.compute.ann.Fixed.Scope.THREAD_LOCAL; import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.checkPrecisionRange; import static org.elasticsearch.xpack.esql.core.type.DataType.LONG; import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.GEO; @@ -53,12 +55,10 @@ public class StGeotile extends SpatialGridFunction implements EvaluatorMapper { */ protected static class GeoTileBoundedGrid implements BoundedGrid { private final int precision; - private final GeoBoundingBox bbox; private final GeoTileBoundedPredicate bounds; - public GeoTileBoundedGrid(int precision, GeoBoundingBox bbox) { + private GeoTileBoundedGrid(int precision, GeoBoundingBox bbox) { this.precision = checkPrecisionRange(precision); - this.bbox = bbox; this.bounds = new GeoTileBoundedPredicate(precision, bbox); } @@ -78,9 +78,18 @@ public int precision() { return precision; } - @Override - public String toString() { - return "[" + bbox + "]"; + protected static class Factory { + private final int precision; + private final GeoBoundingBox bbox; + + Factory(int precision, GeoBoundingBox bbox) { + this.precision = checkPrecisionRange(precision); + this.bbox = bbox; + } + + public GeoTileBoundedGrid get(DriverContext context) { + return new GeoTileBoundedGrid(precision, bbox); + } } } @@ -169,10 +178,14 @@ public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvalua } GeoBoundingBox bbox = asGeoBoundingBox(bounds.fold(toEvaluator.foldCtx())); int precision = (int) parameter.fold(toEvaluator.foldCtx()); - GeoTileBoundedGrid bounds = new GeoTileBoundedGrid(precision, bbox); + GeoTileBoundedGrid.Factory bounds = new GeoTileBoundedGrid.Factory(precision, bbox); return spatialDocsValues - ? new StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator.Factory(source(), toEvaluator.apply(spatialField()), bounds) - : new StGeotileFromFieldAndLiteralAndLiteralEvaluator.Factory(source(), toEvaluator.apply(spatialField), bounds); + ? new StGeotileFromFieldDocValuesAndLiteralAndLiteralEvaluator.Factory( + source(), + toEvaluator.apply(spatialField()), + bounds::get + ) + : new StGeotileFromFieldAndLiteralAndLiteralEvaluator.Factory(source(), toEvaluator.apply(spatialField), bounds::get); } else { int precision = checkPrecisionRange((int) parameter.fold(toEvaluator.foldCtx())); return spatialDocsValues @@ -205,7 +218,12 @@ static void fromFieldDocValuesAndLiteral(LongBlock.Builder results, int p, LongB } @Evaluator(extraName = "FromFieldAndLiteralAndLiteral", warnExceptions = { IllegalArgumentException.class }) - static void fromFieldAndLiteralAndLiteral(LongBlock.Builder results, int p, BytesRefBlock in, @Fixed GeoTileBoundedGrid bounds) { + static void fromFieldAndLiteralAndLiteral( + LongBlock.Builder results, + int p, + BytesRefBlock in, + @Fixed(includeInToString = false, scope = THREAD_LOCAL) GeoTileBoundedGrid bounds + ) { fromWKB(results, p, in, bounds); } @@ -214,7 +232,7 @@ static void fromFieldDocValuesAndLiteralAndLiteral( LongBlock.Builder results, int p, LongBlock encoded, - @Fixed GeoTileBoundedGrid bounds + @Fixed(includeInToString = false, scope = THREAD_LOCAL) GeoTileBoundedGrid bounds ) { fromEncodedLong(results, p, encoded, bounds); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialGridFunctionTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialGridFunctionTestCase.java index 328e166ede5e9..b1a83de93c4ee 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialGridFunctionTestCase.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialGridFunctionTestCase.java @@ -106,7 +106,7 @@ protected static void addTestCaseSuppliers( String evaluatorName = "FromFieldAndLiteralAndLiteralEvaluator[in=Attribute[channel=0], bounds=["; if (literalPrecision) { precisionData = precisionData.forceLiteral(); - evaluatorName = "FromFieldAndLiteralAndLiteralEvaluator[in=Attribute[channel=0], bounds=["; + evaluatorName = "FromFieldAndLiteralAndLiteralEvaluator[in=Attribute[channel=0]"; } TestCaseSupplier.TypedData boundsData; // Create a rectangle as bounds diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashTests.java index 625f23f4c9a3c..03d5047f4dde7 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashTests.java @@ -51,7 +51,7 @@ private static long valueOf(BytesRef wkb, int precision) { } private static long boundedValueOf(BytesRef wkb, int precision, GeoBoundingBox bbox) { - StGeohash.GeoHashBoundedGrid bounds = new StGeohash.GeoHashBoundedGrid(precision, bbox); + StGeohash.GeoHashBoundedGrid bounds = new StGeohash.GeoHashBoundedGrid.Factory(precision, bbox).get(null); return bounds.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb)); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexTests.java index e48b65df050af..96e93c5354eb5 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexTests.java @@ -51,7 +51,7 @@ private static long valueOf(BytesRef wkb, int precision) { } private static long boundedValueOf(BytesRef wkb, int precision, GeoBoundingBox bbox) { - StGeohex.GeoHexBoundedGrid bounds = new StGeohex.GeoHexBoundedGrid(precision, bbox); + StGeohex.GeoHexBoundedGrid bounds = new StGeohex.GeoHexBoundedGrid.Factory(precision, bbox).get(null); return bounds.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb)); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileTests.java index 01632fb07bf38..1a247ec1afa47 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileTests.java @@ -51,7 +51,7 @@ private static long valueOf(BytesRef wkb, int precision) { } private static long boundedValueOf(BytesRef wkb, int precision, GeoBoundingBox bbox) { - StGeotile.GeoTileBoundedGrid bounds = new StGeotile.GeoTileBoundedGrid(precision, bbox); + StGeotile.GeoTileBoundedGrid bounds = new StGeotile.GeoTileBoundedGrid.Factory(precision, bbox).get(null); return bounds.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb)); } From 889e89540ac9f76bea07b6a35012dfe1f21aa078 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Mon, 30 Jun 2025 19:00:02 +0200 Subject: [PATCH 2/4] Update docs/changelog/130343.yaml --- docs/changelog/130343.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/130343.yaml diff --git a/docs/changelog/130343.yaml b/docs/changelog/130343.yaml new file mode 100644 index 0000000000000..ea759f57e5fe0 --- /dev/null +++ b/docs/changelog/130343.yaml @@ -0,0 +1,5 @@ +pr: 130343 +summary: ST_GEOHEX fix for excessive object creation in bounded grids +area: ES|QL +type: tech debt +issues: [] From bf3b40dbd5e9ac0f1350f6a40b974d7fddbf7101 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Mon, 30 Jun 2025 21:30:15 +0200 Subject: [PATCH 3/4] Delete docs/changelog/130343.yaml --- docs/changelog/130343.yaml | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 docs/changelog/130343.yaml diff --git a/docs/changelog/130343.yaml b/docs/changelog/130343.yaml deleted file mode 100644 index ea759f57e5fe0..0000000000000 --- a/docs/changelog/130343.yaml +++ /dev/null @@ -1,5 +0,0 @@ -pr: 130343 -summary: ST_GEOHEX fix for excessive object creation in bounded grids -area: ES|QL -type: tech debt -issues: [] From 55a9af09a6467a965505ca3c970d32cc157de35a Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Tue, 1 Jul 2025 12:36:30 +0200 Subject: [PATCH 4/4] Expand tests to use random bounds, after multi-thread fix Also fix an issue around nulls for out-of-bounds results And investigate and affirm that the use of -1 internally to mark invalid grid-ids is fine for all three grid types (ie. -1 is not a valid grid-id in any of these cases, for different reasons in each). --- .../function/scalar/spatial/StGeohash.java | 6 ++-- .../function/scalar/spatial/StGeohex.java | 5 ++-- .../function/scalar/spatial/StGeotile.java | 6 ++-- .../spatial/SpatialGridFunctionTestCase.java | 29 ++++++++++++++----- .../scalar/spatial/StGeohashTests.java | 5 ++-- .../scalar/spatial/StGeohexTests.java | 5 ++-- .../scalar/spatial/StGeotileTests.java | 9 +++--- 7 files changed, 44 insertions(+), 21 deletions(-) diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohash.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohash.java index af0a1c3f465c4..c152108c67649 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohash.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohash.java @@ -66,7 +66,8 @@ public long calculateGridId(Point point) { if (bounds.validHash(geohash)) { return Geohash.longEncode(geohash); } - // TODO: Are negative values allowed in geohash long encoding? + // Geohash uses the lowest 4 bites for precision, and if all four are set, we get 15, which is invalid since the + // max precision allowed is 12. This allows us to return -1 to indicate invalid geohashes (since all bits are set to 1). return -1; } @@ -209,7 +210,8 @@ public Object fold(FoldContext ctx) { } else { GeoBoundingBox bbox = asGeoBoundingBox(bounds().fold(ctx)); GeoHashBoundedGrid bounds = new GeoHashBoundedGrid(precision, bbox); - return bounds.calculateGridId(GEO.wkbAsPoint(point)); + long gridId = bounds.calculateGridId(GEO.wkbAsPoint(point)); + return gridId < 0 ? null : gridId; } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohex.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohex.java index 820f443ef2726..936b22ba03ccf 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohex.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohex.java @@ -64,7 +64,7 @@ public long calculateGridId(Point point) { if (bounds.validHex(geohex)) { return geohex; } - // TODO: Are we sure negative numbers are not valid + // H3 explicitly requires the highest bit to be zero, freeing up all negative numbers as invalid ids. See H3.isValidHex() return -1L; } @@ -212,7 +212,8 @@ public Object fold(FoldContext ctx) { } else { GeoBoundingBox bbox = asGeoBoundingBox(bounds().fold(ctx)); GeoHexBoundedGrid bounds = new GeoHexBoundedGrid(precision, bbox); - return bounds.calculateGridId(GEO.wkbAsPoint(point)); + long gridId = bounds.calculateGridId(GEO.wkbAsPoint(point)); + return gridId < 0 ? null : gridId; } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotile.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotile.java index 6dabe5a6a7d04..86a9e2bbc6030 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotile.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotile.java @@ -69,7 +69,8 @@ public long calculateGridId(Point point) { if (bounds.validTile(x, y, precision)) { return GeoTileUtils.longEncodeTiles(precision, x, y); } - // TODO: Are we sure negative numbers are not valid + // GeoTileUtils uses the highest 6 bits to store the zoom level. However, MAX_ZOOM is 29, which takes 5 bits. + // This leaves the sign bit unused, so it can be used to indicate an invalid tile. return -1L; } @@ -203,7 +204,8 @@ public Object fold(FoldContext ctx) { } else { GeoBoundingBox bbox = asGeoBoundingBox(bounds().fold(ctx)); GeoTileBoundedGrid bounds = new GeoTileBoundedGrid(precision, bbox); - return bounds.calculateGridId(GEO.wkbAsPoint(point)); + long gridId = bounds.calculateGridId(GEO.wkbAsPoint(point)); + return gridId < 0 ? null : gridId; } } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialGridFunctionTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialGridFunctionTestCase.java index b1a83de93c4ee..0d4eaf6e826d9 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialGridFunctionTestCase.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialGridFunctionTestCase.java @@ -102,21 +102,17 @@ protected static void addTestCaseSuppliers( BytesRef geometry = (BytesRef) geoTypedData.data(); int precision = between(1, 8); TestCaseSupplier.TypedData precisionData = new TestCaseSupplier.TypedData(precision, INTEGER, "precision"); - Rectangle bounds = new Rectangle(-180, 180, 90, -90); String evaluatorName = "FromFieldAndLiteralAndLiteralEvaluator[in=Attribute[channel=0], bounds=["; if (literalPrecision) { precisionData = precisionData.forceLiteral(); evaluatorName = "FromFieldAndLiteralAndLiteralEvaluator[in=Attribute[channel=0]"; } - TestCaseSupplier.TypedData boundsData; - // Create a rectangle as bounds - BytesRef boundsBytesRef = GEO.asWkb(bounds); - boundsData = new TestCaseSupplier.TypedData(boundsBytesRef, GEO_SHAPE, "bounds").forceLiteral(); + var boundsData = randomBoundsData(); return new TestCaseSupplier.TestCase( - List.of(geoTypedData, precisionData, boundsData), + List.of(geoTypedData, precisionData, boundsData.typedData), startsWith(getFunctionClassName() + evaluatorName), LONG, - equalTo(expectedValueWithBounds.apply(geometry, precision, SpatialGridFunction.asGeoBoundingBox(bounds))) + equalTo(expectedValueWithBounds.apply(geometry, precision, boundsData.geoBoundingBox())) ); })); } @@ -141,6 +137,25 @@ public static TestCaseSupplier.TypedDataSupplier testCaseSupplier(DataType dataT } } + protected record TestBoundsData(GeoBoundingBox geoBoundingBox, TestCaseSupplier.TypedData typedData) {} + + private static TestBoundsData randomBoundsData() { + // Create a rectangle with random center and random size, that does not exceed geographic bounds + double x = randomDoubleBetween(-180.1, 179.1, false); + double y = randomDoubleBetween(-90.1, 89.1, false); + double width = randomDoubleBetween(0.1, 180.0, true); + double height = randomDoubleBetween(0.1, 90.0, true); + double minX = Math.max(-180, x - width / 2); + double maxX = Math.min(180, x + width / 2); + double minY = Math.max(-90, y - height / 2); + double maxY = Math.min(90, y + height / 2); + Rectangle bounds = new Rectangle(minX, maxX, maxY, minY); + return new TestBoundsData( + SpatialGridFunction.asGeoBoundingBox(bounds), + new TestCaseSupplier.TypedData(GEO.asWkb(bounds), GEO_SHAPE, "bounds").forceLiteral() + ); + } + protected Long process(int precision, BiFunction expectedValue) { Object spatialObj = this.testCase.getDataValues().getFirst(); assumeNotNull(spatialObj); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashTests.java index 03d5047f4dde7..185591df91b4d 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohashTests.java @@ -50,9 +50,10 @@ private static long valueOf(BytesRef wkb, int precision) { return StGeohash.unboundedGrid.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb), precision); } - private static long boundedValueOf(BytesRef wkb, int precision, GeoBoundingBox bbox) { + private static Long boundedValueOf(BytesRef wkb, int precision, GeoBoundingBox bbox) { StGeohash.GeoHashBoundedGrid bounds = new StGeohash.GeoHashBoundedGrid.Factory(precision, bbox).get(null); - return bounds.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb)); + long gridId = bounds.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb)); + return gridId < 0 ? null : gridId; } @Override diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexTests.java index 96e93c5354eb5..f52241806a3be 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeohexTests.java @@ -50,9 +50,10 @@ private static long valueOf(BytesRef wkb, int precision) { return StGeohex.unboundedGrid.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb), precision); } - private static long boundedValueOf(BytesRef wkb, int precision, GeoBoundingBox bbox) { + private static Long boundedValueOf(BytesRef wkb, int precision, GeoBoundingBox bbox) { StGeohex.GeoHexBoundedGrid bounds = new StGeohex.GeoHexBoundedGrid.Factory(precision, bbox).get(null); - return bounds.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb)); + long gridId = bounds.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb)); + return gridId < 0 ? null : gridId; } @Override diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileTests.java index 1a247ec1afa47..5661f1eacf533 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StGeotileTests.java @@ -13,7 +13,6 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.common.geo.GeoBoundingBox; import org.elasticsearch.license.License; -import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils; import org.elasticsearch.xpack.esql.core.expression.Expression; import org.elasticsearch.xpack.esql.core.tree.Source; import org.elasticsearch.xpack.esql.core.type.DataType; @@ -23,6 +22,7 @@ import java.util.List; import java.util.function.Supplier; +import static org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils.MAX_ZOOM; import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.UNSPECIFIED; import static org.hamcrest.Matchers.containsString; @@ -50,9 +50,10 @@ private static long valueOf(BytesRef wkb, int precision) { return StGeotile.unboundedGrid.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb), precision); } - private static long boundedValueOf(BytesRef wkb, int precision, GeoBoundingBox bbox) { + private static Long boundedValueOf(BytesRef wkb, int precision, GeoBoundingBox bbox) { StGeotile.GeoTileBoundedGrid bounds = new StGeotile.GeoTileBoundedGrid.Factory(precision, bbox).get(null); - return bounds.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb)); + long gridId = bounds.calculateGridId(UNSPECIFIED.wkbAsPoint(wkb)); + return gridId < 0 ? null : gridId; } @Override @@ -64,7 +65,7 @@ protected Expression build(Source source, List args) { public void testInvalidPrecision() { IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> process(-1, StGeotileTests::valueOf)); assertThat(ex.getMessage(), containsString("Invalid geotile_grid precision of -1. Must be between 0 and 29.")); - ex = expectThrows(IllegalArgumentException.class, () -> process(GeoTileUtils.MAX_ZOOM + 1, StGeotileTests::valueOf)); + ex = expectThrows(IllegalArgumentException.class, () -> process(MAX_ZOOM + 1, StGeotileTests::valueOf)); assertThat(ex.getMessage(), containsString("Invalid geotile_grid precision of 30. Must be between 0 and 29.")); } }