From 814ae97c60d9bd352b4f24e5135df4b39041074f Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Mon, 1 Sep 2025 13:11:09 +0200 Subject: [PATCH 1/4] Fix snapshot/release for geogrid spatial sort tests We run the tests for both release and snapshot, so we can see the different errors. This also increases the chance that when we move out of snapshot we wil see failures and need to fix the tests. --- .../xpack/esql/analysis/VerifierTests.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java index 919db367115cf..e57635c6d142c 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java @@ -67,6 +67,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.matchesRegex; +import static org.hamcrest.Matchers.startsWith; //@TestLogging(value = "org.elasticsearch.xpack.esql:TRACE,org.elasticsearch.compute:TRACE", reason = "debug") public class VerifierTests extends ESTestCase { @@ -1058,9 +1059,6 @@ public void testSpatialSort() { "1:136: cannot sort on cartesian_shape", error(prefix + "| EVAL shape = TO_CARTESIANSHAPE(wkt) | limit 5 | sort shape") ); - assertEquals("1:143: cannot sort on geohash", error(prefix + "| EVAL grid = ST_GEOHASH(TO_GEOPOINT(wkt),1) | limit 5 | sort grid")); - assertEquals("1:143: cannot sort on geotile", error(prefix + "| EVAL grid = ST_GEOTILE(TO_GEOPOINT(wkt),1) | limit 5 | sort grid")); - assertEquals("1:142: cannot sort on geohex", error(prefix + "| EVAL grid = ST_GEOHEX(TO_GEOPOINT(wkt),1) | limit 5 | sort grid")); var airports = AnalyzerTestUtils.analyzer(loadMapping("mapping-airports.json", "airports")); var airportsWeb = AnalyzerTestUtils.analyzer(loadMapping("mapping-airports_web.json", "airports_web")); var countriesBbox = AnalyzerTestUtils.analyzer(loadMapping("mapping-countries_bbox.json", "countries_bbox")); @@ -1069,15 +1067,21 @@ public void testSpatialSort() { assertEquals("1:36: cannot sort on cartesian_point", error("FROM airports_web | LIMIT 5 | sort location", airportsWeb)); assertEquals("1:38: cannot sort on geo_shape", error("FROM countries_bbox | LIMIT 5 | sort shape", countriesBbox)); assertEquals("1:42: cannot sort on cartesian_shape", error("FROM countries_bbox_web | LIMIT 5 | sort shape", countriesBboxWeb)); - assertEquals( - "1:67: cannot sort on geohash", - error("FROM airports | LIMIT 5 | EVAL g = ST_GEOHASH(location, 1) | sort g", airports) - ); - assertEquals( - "1:67: cannot sort on geotile", - error("FROM airports | LIMIT 5 | EVAL g = ST_GEOTILE(location, 1) | sort g", airports) - ); - assertEquals("1:66: cannot sort on geohex", error("FROM airports | LIMIT 5 | EVAL g = ST_GEOHEX(location, 1) | sort g", airports)); + for (String grid : new String[] { "geohash", "geotile", "geohex" }) { + String gridFunc = "ST_" + grid.toUpperCase(Locale.ROOT); + String error = Build.current().isSnapshot() + ? "1:" + (136 + grid.length()) + ": cannot sort on " + grid + : "1:95: Unknown function [" + gridFunc + "]"; + assertThat(grid, error(prefix + "| EVAL grid = " + gridFunc + "(TO_GEOPOINT(wkt),1) | limit 5 | sort grid"), startsWith(error)); + error = Build.current().isSnapshot() + ? "1:" + (63 + grid.length()) + ": cannot sort on " + grid + : "1:39: Unknown function [" + gridFunc + "]"; + assertThat( + grid, + error("FROM airports | LIMIT 5 | EVAL grid = " + gridFunc + "(location, 1) | sort grid", airports), + startsWith(error) + ); + } } public void testSourceSorting() { From 62c4778d6e97bc24a2e08807a4bce6d98b39dd69 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Mon, 1 Sep 2025 13:19:24 +0200 Subject: [PATCH 2/4] Fix snapshot/release for geogrid spatial changepoint tests --- .../xpack/esql/analysis/VerifierTests.java | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java index e57635c6d142c..835154e47b00e 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java @@ -2077,7 +2077,9 @@ public void testChangePoint() { public void testChangePoint_keySortable() { assumeTrue("change_point must be enabled", EsqlCapabilities.Cap.CHANGE_POINT.isEnabled()); List sortableTypes = List.of(BOOLEAN, DOUBLE, DATE_NANOS, DATETIME, INTEGER, IP, KEYWORD, LONG, UNSIGNED_LONG, VERSION); - List unsortableTypes = List.of(CARTESIAN_POINT, CARTESIAN_SHAPE, GEO_POINT, GEO_SHAPE, GEOHASH, GEOTILE, GEOHEX); + List unsortableTypes = Build.current().isSnapshot() + ? List.of(CARTESIAN_POINT, CARTESIAN_SHAPE, GEO_POINT, GEO_SHAPE, GEOHASH, GEOTILE, GEOHEX) + : List.of(CARTESIAN_POINT, CARTESIAN_SHAPE, GEO_POINT, GEO_SHAPE); for (DataType type : sortableTypes) { query(Strings.format("ROW key=NULL::%s, value=0\n | CHANGE_POINT value ON key", type)); } @@ -2092,21 +2094,23 @@ public void testChangePoint_keySortable() { public void testChangePoint_valueNumeric() { assumeTrue("change_point must be enabled", EsqlCapabilities.Cap.CHANGE_POINT.isEnabled()); List numericTypes = List.of(DOUBLE, INTEGER, LONG, UNSIGNED_LONG); - List nonNumericTypes = List.of( - BOOLEAN, - CARTESIAN_POINT, - CARTESIAN_SHAPE, - DATE_NANOS, - DATETIME, - GEO_POINT, - GEO_SHAPE, - GEOHASH, - GEOTILE, - GEOHEX, - IP, - KEYWORD, - VERSION - ); + List nonNumericTypes = Build.current().isSnapshot() + ? List.of( + BOOLEAN, + CARTESIAN_POINT, + CARTESIAN_SHAPE, + DATE_NANOS, + DATETIME, + GEO_POINT, + GEO_SHAPE, + GEOHASH, + GEOTILE, + GEOHEX, + IP, + KEYWORD, + VERSION + ) + : List.of(BOOLEAN, CARTESIAN_POINT, CARTESIAN_SHAPE, DATE_NANOS, DATETIME, GEO_POINT, GEO_SHAPE, IP, KEYWORD, VERSION); for (DataType type : numericTypes) { query(Strings.format("ROW key=0, value=NULL::%s\n | CHANGE_POINT value ON key", type)); } From 28841744c9859ee19c12d56957fc3f3b68d0036a Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Mon, 1 Sep 2025 13:24:59 +0200 Subject: [PATCH 3/4] Do not test converters for types that are in snapshot only --- .../org/elasticsearch/xpack/esql/analysis/ParsingTests.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java index 35bcb75714d4b..d4031d5490df6 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.esql.analysis; +import org.elasticsearch.Build; import org.elasticsearch.core.PathUtils; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentBuilder; @@ -93,6 +94,10 @@ public void testInlineCast() throws IOException { report.humanReadable(true).prettyPrint(); report.startObject(); List namesAndAliases = new ArrayList<>(DataType.namesAndAliases()); + if (Build.current().isSnapshot() == false) { + // Some types do not have a converter in release builds + namesAndAliases.removeAll(List.of("geohash", "geotile", "geohex")); + } Collections.sort(namesAndAliases); for (String nameOrAlias : namesAndAliases) { DataType expectedType = DataType.fromNameOrAlias(nameOrAlias); From 424c1cd34c3dafb89fb412585b1a66835e306f62 Mon Sep 17 00:00:00 2001 From: Craig Taverner Date: Mon, 1 Sep 2025 15:48:49 +0200 Subject: [PATCH 4/4] Modified tests to look at capabilities instead of snapshot/release --- .../xpack/esql/analysis/ParsingTests.java | 5 ++-- .../xpack/esql/analysis/VerifierTests.java | 26 +++++++++---------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java index d4031d5490df6..225e385c13ddb 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/ParsingTests.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.esql.analysis; -import org.elasticsearch.Build; import org.elasticsearch.core.PathUtils; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentBuilder; @@ -94,8 +93,8 @@ public void testInlineCast() throws IOException { report.humanReadable(true).prettyPrint(); report.startObject(); List namesAndAliases = new ArrayList<>(DataType.namesAndAliases()); - if (Build.current().isSnapshot() == false) { - // Some types do not have a converter in release builds + if (EsqlCapabilities.Cap.SPATIAL_GRID_TYPES.isEnabled() == false) { + // Some types do not have a converter function if the capability is disabled namesAndAliases.removeAll(List.of("geohash", "geotile", "geohex")); } Collections.sort(namesAndAliases); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java index 835154e47b00e..4e0814d6cc6f5 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java @@ -1069,18 +1069,16 @@ public void testSpatialSort() { assertEquals("1:42: cannot sort on cartesian_shape", error("FROM countries_bbox_web | LIMIT 5 | sort shape", countriesBboxWeb)); for (String grid : new String[] { "geohash", "geotile", "geohex" }) { String gridFunc = "ST_" + grid.toUpperCase(Locale.ROOT); - String error = Build.current().isSnapshot() - ? "1:" + (136 + grid.length()) + ": cannot sort on " + grid - : "1:95: Unknown function [" + gridFunc + "]"; - assertThat(grid, error(prefix + "| EVAL grid = " + gridFunc + "(TO_GEOPOINT(wkt),1) | limit 5 | sort grid"), startsWith(error)); - error = Build.current().isSnapshot() - ? "1:" + (63 + grid.length()) + ": cannot sort on " + grid - : "1:39: Unknown function [" + gridFunc + "]"; - assertThat( - grid, - error("FROM airports | LIMIT 5 | EVAL grid = " + gridFunc + "(location, 1) | sort grid", airports), - startsWith(error) - ); + String literalQuery = prefix + "| EVAL grid = " + gridFunc + "(TO_GEOPOINT(wkt),1) | limit 5 | sort grid"; + String indexQuery = "FROM airports | LIMIT 5 | EVAL grid = " + gridFunc + "(location, 1) | sort grid"; + String literalError = "1:" + (136 + grid.length()) + ": cannot sort on " + grid; + String indexError = "1:" + (63 + grid.length()) + ": cannot sort on " + grid; + if (EsqlCapabilities.Cap.SPATIAL_GRID_TYPES.isEnabled() == false) { + literalError = "1:95: Unknown function [" + gridFunc + "]"; + indexError = "1:39: Unknown function [" + gridFunc + "]"; + } + assertThat(grid, error(literalQuery), startsWith(literalError)); + assertThat(grid, error(indexQuery, airports), startsWith(indexError)); } } @@ -2077,7 +2075,7 @@ public void testChangePoint() { public void testChangePoint_keySortable() { assumeTrue("change_point must be enabled", EsqlCapabilities.Cap.CHANGE_POINT.isEnabled()); List sortableTypes = List.of(BOOLEAN, DOUBLE, DATE_NANOS, DATETIME, INTEGER, IP, KEYWORD, LONG, UNSIGNED_LONG, VERSION); - List unsortableTypes = Build.current().isSnapshot() + List unsortableTypes = EsqlCapabilities.Cap.SPATIAL_GRID_TYPES.isEnabled() ? List.of(CARTESIAN_POINT, CARTESIAN_SHAPE, GEO_POINT, GEO_SHAPE, GEOHASH, GEOTILE, GEOHEX) : List.of(CARTESIAN_POINT, CARTESIAN_SHAPE, GEO_POINT, GEO_SHAPE); for (DataType type : sortableTypes) { @@ -2094,7 +2092,7 @@ public void testChangePoint_keySortable() { public void testChangePoint_valueNumeric() { assumeTrue("change_point must be enabled", EsqlCapabilities.Cap.CHANGE_POINT.isEnabled()); List numericTypes = List.of(DOUBLE, INTEGER, LONG, UNSIGNED_LONG); - List nonNumericTypes = Build.current().isSnapshot() + List nonNumericTypes = EsqlCapabilities.Cap.SPATIAL_GRID_TYPES.isEnabled() ? List.of( BOOLEAN, CARTESIAN_POINT,