浏览代码

ESQL: Fix error on sorting unsortable geo_point and cartesian_point (#106351)

* Fix error on sorting unsortable geo_point and cartesian_point

Without a LIMIT the correct error worked, but with LIMIT it did not. This fix mimics the same error with LIMIT and adds tests for all three scenarios:
* Without limit
* With Limit
* From row with limit

* Update docs/changelog/106351.yaml

* Add tests for geo_shape and cartesian_shape also

* Updated changelog

* Separate point and shape error messages

* Move error to later so we get it only if geo field is actually used in sort.

* Implemented planner check in Verifier instead

This is a much better solution.

* Revert previous solution

* Also check non-field attributes so the same error is provided for ROW

* Changed "can't" to "cannot"

* Add unit tests for verifier error

* Added sort limitations to documentation

* Added unit tests for spatial fields in VerifierTests

* Don't run the new yaml tests on older versions

These tests mostly test the validation errors which were changed only in 8.14.0, so should not be tested in earlier versions.

* Simplify check based on code review, skip duplicate forEachDown
Craig Taverner 1 年之前
父节点
当前提交
002ed8d49d

+ 6 - 0
docs/changelog/106351.yaml

@@ -0,0 +1,6 @@
+pr: 106351
+summary: "Fix error on sorting unsortable `geo_point` and `cartesian_point`"
+area: ES|QL
+type: bug
+issues:
+ - 106007

+ 12 - 0
docs/reference/esql/esql-limitations.asciidoc

@@ -73,6 +73,18 @@ unsupported type is not explicitly used in a query, it is returned with `null`
 values, with the exception of nested fields. Nested fields are not returned at
 values, with the exception of nested fields. Nested fields are not returned at
 all.
 all.
 
 
+[discrete]
+==== Limitations on supported types
+
+Some <<mapping-types,field types>> are not supported in all contexts:
+
+* Spatial types are not supported in the <<esql-sort,SORT>> processing command.
+  Specifying a column of one of these types as a sort parameter will result in an error:
+** `geo_point`
+** `geo_shape`
+** `cartesian_point`
+** `cartesian_shape`
+
 [discrete]
 [discrete]
 [[esql-_source-availability]]
 [[esql-_source-availability]]
 === _source availability
 === _source availability

+ 17 - 0
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java

@@ -21,6 +21,7 @@ import org.elasticsearch.xpack.esql.type.EsqlDataTypes;
 import org.elasticsearch.xpack.ql.capabilities.Unresolvable;
 import org.elasticsearch.xpack.ql.capabilities.Unresolvable;
 import org.elasticsearch.xpack.ql.common.Failure;
 import org.elasticsearch.xpack.ql.common.Failure;
 import org.elasticsearch.xpack.ql.expression.Alias;
 import org.elasticsearch.xpack.ql.expression.Alias;
+import org.elasticsearch.xpack.ql.expression.Attribute;
 import org.elasticsearch.xpack.ql.expression.AttributeMap;
 import org.elasticsearch.xpack.ql.expression.AttributeMap;
 import org.elasticsearch.xpack.ql.expression.Expression;
 import org.elasticsearch.xpack.ql.expression.Expression;
 import org.elasticsearch.xpack.ql.expression.NamedExpression;
 import org.elasticsearch.xpack.ql.expression.NamedExpression;
@@ -32,6 +33,7 @@ import org.elasticsearch.xpack.ql.expression.predicate.operator.comparison.Binar
 import org.elasticsearch.xpack.ql.plan.logical.Aggregate;
 import org.elasticsearch.xpack.ql.plan.logical.Aggregate;
 import org.elasticsearch.xpack.ql.plan.logical.Limit;
 import org.elasticsearch.xpack.ql.plan.logical.Limit;
 import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;
 import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan;
+import org.elasticsearch.xpack.ql.plan.logical.OrderBy;
 import org.elasticsearch.xpack.ql.plan.logical.Project;
 import org.elasticsearch.xpack.ql.plan.logical.Project;
 import org.elasticsearch.xpack.ql.plan.logical.UnaryPlan;
 import org.elasticsearch.xpack.ql.plan.logical.UnaryPlan;
 import org.elasticsearch.xpack.ql.type.DataType;
 import org.elasticsearch.xpack.ql.type.DataType;
@@ -139,6 +141,7 @@ public class Verifier {
 
 
             checkOperationsOnUnsignedLong(p, failures);
             checkOperationsOnUnsignedLong(p, failures);
             checkBinaryComparison(p, failures);
             checkBinaryComparison(p, failures);
+            checkForSortOnSpatialTypes(p, failures);
         });
         });
         checkRemoteEnrich(plan, failures);
         checkRemoteEnrich(plan, failures);
 
 
@@ -381,6 +384,20 @@ public class Verifier {
         return null;
         return null;
     }
     }
 
 
+    /**
+     * Makes sure that spatial types do not appear in sorting contexts.
+     */
+    private static void checkForSortOnSpatialTypes(LogicalPlan p, Set<Failure> localFailures) {
+        if (p instanceof OrderBy ob) {
+            ob.forEachExpression(Attribute.class, attr -> {
+                DataType dataType = attr.dataType();
+                if (EsqlDataTypes.isSpatial(dataType)) {
+                    localFailures.add(fail(attr, "cannot sort on " + dataType.typeName()));
+                }
+            });
+        }
+    }
+
     /**
     /**
      * Ensure that no remote enrich is allowed after a reduction or an enrich with coordinator mode.
      * Ensure that no remote enrich is allowed after a reduction or an enrich with coordinator mode.
      * <p>
      * <p>

+ 23 - 0
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

@@ -18,6 +18,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.List;
 
 
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning;
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.withDefaultLimitWarning;
+import static org.elasticsearch.xpack.esql.analysis.AnalyzerTestUtils.loadMapping;
 import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG;
 import static org.elasticsearch.xpack.ql.type.DataTypes.UNSIGNED_LONG;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.containsString;
 
 
@@ -355,6 +356,28 @@ public class VerifierTests extends ESTestCase {
         assertEquals("1:23: invalid stats declaration; [avg] is not an aggregate function", error("from test | stats c = avg"));
         assertEquals("1:23: invalid stats declaration; [avg] is not an aggregate function", error("from test | stats c = avg"));
     }
     }
 
 
+    public void testSpatialSort() {
+        String prefix = "ROW wkt = [\"POINT(42.9711 -14.7553)\", \"POINT(75.8093 22.7277)\"] | MV_EXPAND wkt ";
+        assertEquals("1:130: cannot sort on geo_point", error(prefix + "| EVAL shape = TO_GEOPOINT(wkt) | limit 5 | sort shape"));
+        assertEquals(
+            "1:136: cannot sort on cartesian_point",
+            error(prefix + "| EVAL shape = TO_CARTESIANPOINT(wkt) | limit 5 | sort shape")
+        );
+        assertEquals("1:130: cannot sort on geo_shape", error(prefix + "| EVAL shape = TO_GEOSHAPE(wkt) | limit 5 | sort shape"));
+        assertEquals(
+            "1:136: cannot sort on cartesian_shape",
+            error(prefix + "| EVAL shape = TO_CARTESIANSHAPE(wkt) | limit 5 | sort shape")
+        );
+        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"));
+        var countriesBboxWeb = AnalyzerTestUtils.analyzer(loadMapping("mapping-countries_bbox_web.json", "countries_bbox_web"));
+        assertEquals("1:32: cannot sort on geo_point", error("FROM airports | LIMIT 5 | sort location", airports));
+        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));
+    }
+
     private String error(String query) {
     private String error(String query) {
         return error(query, defaultAnalyzer);
         return error(query, defaultAnalyzer);
     }
     }

+ 234 - 0
x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/130_spatial.yml

@@ -0,0 +1,234 @@
+---
+setup:
+  - skip:
+      version: " - 8.13.99"
+      reason: "Mixed cluster tests don't work with the changed error message from sort"
+      features: allowed_warnings_regex
+
+  - do:
+      indices.create:
+        index: geo_points
+        body:
+          mappings:
+            properties:
+              location:
+                type: geo_point
+
+  - do:
+      bulk:
+        index: geo_points
+        refresh: true
+        body:
+          - { "index": { } }
+          - { "location": "POINT(1 -1)" }
+          - { "index": { } }
+          - { "location": "POINT(-1 1)" }
+
+  - do:
+      indices.create:
+        index: cartesian_points
+        body:
+          mappings:
+            properties:
+              location:
+                type: point
+
+  - do:
+      bulk:
+        index: cartesian_points
+        refresh: true
+        body:
+          - { "index": { } }
+          - { "location": "POINT(4321 -1234)" }
+          - { "index": { } }
+          - { "location": "POINT(-4321 1234)" }
+
+  - do:
+      indices.create:
+        index: geo_shapes
+        body:
+          mappings:
+            properties:
+              shape:
+                type: geo_shape
+
+  - do:
+      bulk:
+        index: geo_shapes
+        refresh: true
+        body:
+          - { "index": { } }
+          - { "shape": "POINT(0 0)" }
+          - { "index": { } }
+          - { "shape": "POLYGON((-1 -1, 1 -1, 1 1, -1 1, -1 -1))" }
+
+  - do:
+      indices.create:
+        index: cartesian_shapes
+        body:
+          mappings:
+            properties:
+              shape:
+                type: shape
+
+  - do:
+      bulk:
+        index: cartesian_shapes
+        refresh: true
+        body:
+          - { "index": { } }
+          - { "shape": "POINT(0 0)" }
+          - { "index": { } }
+          - { "shape": "POLYGON((-1 -1, 1 -1, 1 1, -1 1, -1 -1))" }
+
+---
+geo_point:
+  - do:
+      allowed_warnings_regex:
+        - "No limit defined, adding default limit of \\[.*\\]"
+      esql.query:
+        body:
+          query: 'from geo_points'
+  - match: { columns.0.name: location }
+  - match: { columns.0.type: geo_point }
+  - length: { values: 2 }
+  - match: { values.0.0: "POINT (1.0 -1.0)" }
+  - match: { values.1.0: "POINT (-1.0 1.0)" }
+
+---
+geo_point unsortable:
+  - do:
+      catch: /cannot sort on geo_point/
+      esql.query:
+        body:
+          query: 'from geo_points | sort location'
+
+---
+geo_point unsortable with limit:
+  - do:
+      catch: /cannot sort on geo_point/
+      esql.query:
+        body:
+          query: 'from geo_points | LIMIT 10 | sort location'
+
+---
+geo_point unsortable with limit from row:
+  - do:
+      catch: /cannot sort on geo_point/
+      esql.query:
+        body:
+          query: 'ROW wkt = ["POINT(42.9711 -14.7553)", "POINT(75.8093 22.7277)"] | MV_EXPAND wkt | EVAL pt = TO_GEOPOINT(wkt) | limit 5 | sort pt'
+
+---
+cartesian_point:
+  - do:
+      allowed_warnings_regex:
+        - "No limit defined, adding default limit of \\[.*\\]"
+      esql.query:
+        body:
+          query: 'from cartesian_points'
+  - match: { columns.0.name: location }
+  - match: { columns.0.type: cartesian_point }
+  - length: { values: 2 }
+  - match: { values.0.0: "POINT (4321.0 -1234.0)" }
+  - match: { values.1.0: "POINT (-4321.0 1234.0)" }
+
+---
+cartesian_point unsortable:
+  - do:
+      catch: /cannot sort on cartesian_point/
+      esql.query:
+        body:
+          query: 'from cartesian_points | sort location'
+
+---
+cartesian_point unsortable with limit:
+  - do:
+      catch: /cannot sort on cartesian_point/
+      esql.query:
+        body:
+          query: 'from cartesian_points | LIMIT 10 | sort location'
+
+---
+cartesian_point unsortable with limit from row:
+  - do:
+      catch: /cannot sort on cartesian_point/
+      esql.query:
+        body:
+          query: 'ROW wkt = ["POINT(4297.11 -1475.53)", "POINT(7580.93 2272.77)"] | MV_EXPAND wkt | EVAL pt = TO_CARTESIANPOINT(wkt) | limit 5 | sort pt'
+
+---
+geo_shape:
+  - do:
+      allowed_warnings_regex:
+        - "No limit defined, adding default limit of \\[.*\\]"
+      esql.query:
+        body:
+          query: 'from geo_shapes'
+  - match: { columns.0.name: shape }
+  - match: { columns.0.type: geo_shape }
+  - length: { values: 2 }
+  - match: { values.0.0: "POINT (0.0 0.0)" }
+  - match: { values.1.0: "POLYGON ((-1.0 -1.0, 1.0 -1.0, 1.0 1.0, -1.0 1.0, -1.0 -1.0))" }
+
+---
+geo_shape unsortable:
+  - do:
+      catch: /cannot sort on geo_shape/
+      esql.query:
+        body:
+          query: 'from geo_shapes | sort shape'
+
+---
+geo_shape unsortable with limit:
+  - do:
+      catch: /cannot sort on geo_shape/
+      esql.query:
+        body:
+          query: 'from geo_shapes | LIMIT 10 | sort shape'
+
+---
+geo_shape unsortable with limit from row:
+  - do:
+      catch: /cannot sort on geo_shape/
+      esql.query:
+        body:
+          query: 'ROW wkt = ["POINT(42.9711 -14.7553)", "POINT(75.8093 22.7277)"] | MV_EXPAND wkt | EVAL shape = TO_GEOSHAPE(wkt) | limit 5 | sort shape'
+
+---
+cartesian_shape:
+  - do:
+      allowed_warnings_regex:
+        - "No limit defined, adding default limit of \\[.*\\]"
+      esql.query:
+        body:
+          query: 'from cartesian_shapes'
+  - match: { columns.0.name: shape }
+  - match: { columns.0.type: cartesian_shape }
+  - length: { values: 2 }
+  - match: { values.0.0: "POINT (0.0 0.0)" }
+  - match: { values.1.0: "POLYGON ((-1.0 -1.0, 1.0 -1.0, 1.0 1.0, -1.0 1.0, -1.0 -1.0))" }
+
+---
+cartesian_shape unsortable:
+  - do:
+      catch: /cannot sort on cartesian_shape/
+      esql.query:
+        body:
+          query: 'from cartesian_shapes | sort shape'
+
+---
+cartesian_shape unsortable with limit:
+  - do:
+      catch: /cannot sort on cartesian_shape/
+      esql.query:
+        body:
+          query: 'from cartesian_shapes | LIMIT 10 | sort shape'
+
+---
+cartesian_shape unsortable with limit from row:
+  - do:
+      catch: /cannot sort on cartesian_shape/
+      esql.query:
+        body:
+          query: 'ROW wkt = ["POINT(4297.11 -1475.53)", "POINT(7580.93 2272.77)"] | MV_EXPAND wkt | EVAL shape = TO_CARTESIANSHAPE(wkt) | limit 5 | sort shape'