Always check crsType when folding spatial functions (elastic#112090) (elastic#112099)

craigtaverner · web-flow · commit f3f0d2e86567 · 2024-08-22T23:42:12.000+10:00
* Always check crsType when folding spatial functions

* Update docs/changelog/112090.yaml

* Only require capability for fixed test

The other tests passed on older versions anyway.
diff --git a/docs/changelog/112090.yaml b/docs/changelog/112090.yaml
@@ -0,0 +1,6 @@
+pr: 112090
+summary: Always check `crsType` when folding spatial functions
+area: Geo
+type: bug
+issues:
+ - 112089
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec
@@ -487,6 +487,17 @@ POINT (42.97109629958868 14.7552534006536)    | 1
 ###############################################
 # Tests for ST_INTERSECTS on GEO_POINT type
 
+literalGeoPointIntersectsLiteralPolygon
+required_capability: st_intersects
+
+ROW pt = TO_GEOPOINT("POINT(0 85)"), polygon = TO_GEOSHAPE("POLYGON((-10 70, 10 70, 10 85, -10 85, -10 70))")
+| EVAL intersects = ST_INTERSECTS(pt, polygon)
+;
+
+pt:geo_point  | polygon:geo_shape                               | intersects:boolean
+POINT(0 85)   | POLYGON((-10 70, 10 70, 10 85, -10 85, -10 70)) | true
+;
+
 pointIntersectsLiteralPolygon
 required_capability: st_intersects
 
@@ -889,6 +900,34 @@ wkt:keyword    | pt:geo_point  | distance:double
 "POINT(1 -1)"  | POINT(1 -1)   | 157249.59498573805
 ;
 
+literalGeoPointDistanceOneDegree
+required_capability: st_distance
+
+ROW wkt = ["POINT(1 0)", "POINT(-1 0)", "POINT(0 1)", "POINT(0 -1)"]
+| MV_EXPAND wkt
+| EVAL pt = TO_GEOPOINT(wkt)
+| EVAL distance = ST_DISTANCE(pt, TO_GEOPOINT("POINT(0 0)"))
+;
+
+wkt:keyword    | pt:geo_point  | distance:double
+"POINT(1 0)"   | POINT(1 0)    | 111195.07310665186
+"POINT(-1 0)"  | POINT(-1 0)   | 111195.08242688453
+"POINT(0 1)"   | POINT(0 1)    | 111195.07776676829
+"POINT(0 -1)"  | POINT(0 -1)   | 111195.08242688453
+;
+
+twoCitiesPointDistanceGeo
+required_capability: st_distance
+required_capability: spatial_functions_fix_crstype_folding
+
+ROW p1 = TO_GEOPOINT("POINT(-90.82814 29.79511)"), p2 = TO_GEOPOINT("POINT(-90.79731509999999 29.8835389)")
+| EVAL d = ST_DISTANCE(p1, p2)
+;
+
+p1:geo_point                | p2:geo_point                           | d:double
+POINT (-90.82814 29.79511)  | POINT (-90.79731509999999 29.8835389)  | 10272.529272836206
+;
+
 airportCityLocationPointDistance
 required_capability: st_distance
 
@@ -1433,6 +1472,17 @@ POINT (726480.0130685265 3359566.331716279) | 849
 ###############################################
 # Tests for ST_INTERSECTS on CARTESIAN_POINT type
 
+literalCartesianPointIntersectsLiteralPolygon
+required_capability: st_intersects
+
+ROW pt = TO_CARTESIANPOINT("POINT(0 85)"), polygon = TO_CARTESIANSHAPE("POLYGON((-10 70, 10 70, 10 85, -10 85, -10 70))")
+| EVAL intersects = ST_INTERSECTS(pt, polygon)
+;
+
+pt:cartesian_point  | polygon:cartesian_shape                         | intersects:boolean
+POINT(0 85)         | POLYGON((-10 70, 10 70, 10 85, -10 85, -10 70)) | true
+;
+
 cartesianCentroidFromAirportsAfterIntersectsPredicate
 required_capability: st_intersects
 
@@ -1996,6 +2046,33 @@ wkt:keyword    | pt:cartesian_point  | distance:double
 "POINT(1 -1)"  | POINT(1 -1)         | 1.4142135623730951
 ;
 
+literalCartesianPointDistanceOneUnit
+required_capability: st_distance
+
+ROW wkt = ["POINT(1 0)", "POINT(-1 0)", "POINT(0 1)", "POINT(0 -1)"]
+| MV_EXPAND wkt
+| EVAL pt = TO_CARTESIANPOINT(wkt)
+| EVAL distance = ST_DISTANCE(pt, TO_CARTESIANPOINT("POINT(0 0)"))
+;
+
+wkt:keyword    | pt:cartesian_point  | distance:double
+"POINT(1 0)"   | POINT(1 0)          | 1.0
+"POINT(-1 0)"  | POINT(-1 0)         | 1.0
+"POINT(0 1)"   | POINT(0 1)          | 1.0
+"POINT(0 -1)"  | POINT(0 -1)         | 1.0
+;
+
+twoCitiesPointDistanceCartesian
+required_capability: st_distance
+
+ROW p1 = TO_CARTESIANPOINT("POINT(-90.82814 29.79511)"), p2 = TO_CARTESIANPOINT("POINT(-90.79731509999999 29.8835389)")
+| EVAL d = ST_DISTANCE(p1, p2)
+;
+
+p1:cartesian_point          | p2:cartesian_point                     | d:double
+POINT (-90.82814 29.79511)  | POINT (-90.79731509999999 29.8835389)  | 0.09364744959271905
+;
+
 airportCartesianCityLocationPointDistance
 required_capability: st_distance
 
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
@@ -92,6 +92,11 @@ public enum Cap {
          */
         ST_DISTANCE,
 
+        /**
+         * Fix determination of CRS types in spatial functions when folding.
+         */
+        SPATIAL_FUNCTIONS_FIX_CRSTYPE_FOLDING,
+
         /**
          * Fix to GROK and DISSECT that allows extracting attributes with the same name as the input
          * https://github.com/elastic/elasticsearch/issues/110184
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/BinarySpatialFunction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/BinarySpatialFunction.java
@@ -43,7 +43,7 @@ public static List<NamedWriteableRegistry.Entry> getNamedWriteables() {
     }
 
     private final SpatialTypeResolver spatialTypeResolver;
-    protected SpatialCrsType crsType;
+    private SpatialCrsType crsType;
     protected final boolean leftDocValues;
     protected final boolean rightDocValues;
 
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialContains.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialContains.java
@@ -177,10 +177,10 @@ protected NodeInfo<? extends Expression> info() {
     @Override
     public Object fold() {
         try {
-            GeometryDocValueReader docValueReader = asGeometryDocValueReader(crsType, left());
+            GeometryDocValueReader docValueReader = asGeometryDocValueReader(crsType(), left());
             Geometry rightGeom = makeGeometryFromLiteral(right());
-            Component2D[] components = asLuceneComponent2Ds(crsType, rightGeom);
-            return (crsType == SpatialCrsType.GEO)
+            Component2D[] components = asLuceneComponent2Ds(crsType(), rightGeom);
+            return (crsType() == SpatialCrsType.GEO)
                 ? GEO.geometryRelatesGeometries(docValueReader, components)
                 : CARTESIAN.geometryRelatesGeometries(docValueReader, components);
         } catch (IOException e) {
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialDisjoint.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialDisjoint.java
@@ -132,9 +132,9 @@ protected NodeInfo<? extends Expression> info() {
     @Override
     public Object fold() {
         try {
-            GeometryDocValueReader docValueReader = asGeometryDocValueReader(crsType, left());
-            Component2D component2D = asLuceneComponent2D(crsType, right());
-            return (crsType == SpatialCrsType.GEO)
+            GeometryDocValueReader docValueReader = asGeometryDocValueReader(crsType(), left());
+            Component2D component2D = asLuceneComponent2D(crsType(), right());
+            return (crsType() == SpatialCrsType.GEO)
                 ? GEO.geometryRelatesGeometry(docValueReader, component2D)
                 : CARTESIAN.geometryRelatesGeometry(docValueReader, component2D);
         } catch (IOException e) {
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialIntersects.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialIntersects.java
@@ -130,9 +130,9 @@ protected NodeInfo<? extends Expression> info() {
     @Override
     public Object fold() {
         try {
-            GeometryDocValueReader docValueReader = asGeometryDocValueReader(crsType, left());
-            Component2D component2D = asLuceneComponent2D(crsType, right());
-            return (crsType == SpatialCrsType.GEO)
+            GeometryDocValueReader docValueReader = asGeometryDocValueReader(crsType(), left());
+            Component2D component2D = asLuceneComponent2D(crsType(), right());
+            return (crsType() == SpatialCrsType.GEO)
                 ? GEO.geometryRelatesGeometry(docValueReader, component2D)
                 : CARTESIAN.geometryRelatesGeometry(docValueReader, component2D);
         } catch (IOException e) {
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialWithin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialWithin.java
@@ -132,9 +132,9 @@ protected NodeInfo<? extends Expression> info() {
     @Override
     public Object fold() {
         try {
-            GeometryDocValueReader docValueReader = asGeometryDocValueReader(crsType, left());
-            Component2D component2D = asLuceneComponent2D(crsType, right());
-            return (crsType == SpatialCrsType.GEO)
+            GeometryDocValueReader docValueReader = asGeometryDocValueReader(crsType(), left());
+            Component2D component2D = asLuceneComponent2D(crsType(), right());
+            return (crsType() == SpatialCrsType.GEO)
                 ? GEO.geometryRelatesGeometry(docValueReader, component2D)
                 : CARTESIAN.geometryRelatesGeometry(docValueReader, component2D);
         } catch (IOException e) {
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StDistance.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StDistance.java
@@ -173,7 +173,7 @@ protected NodeInfo<? extends Expression> info() {
     public Object fold() {
         var leftGeom = makeGeometryFromLiteral(left());
         var rightGeom = makeGeometryFromLiteral(right());
-        return (crsType == SpatialCrsType.GEO) ? GEO.distance(leftGeom, rightGeom) : CARTESIAN.distance(leftGeom, rightGeom);
+        return (crsType() == SpatialCrsType.GEO) ? GEO.distance(leftGeom, rightGeom) : CARTESIAN.distance(leftGeom, rightGeom);
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ public static List<NamedWriteableRegistry.Entry> getNamedWriteables() {`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`private final SpatialTypeResolver spatialTypeResolver;`
`46`		`- protected SpatialCrsType crsType;`
	`46`	`+ private SpatialCrsType crsType;`
`47`	`47`	`protected final boolean leftDocValues;`
`48`	`48`	`protected final boolean rightDocValues;`
`49`	`49`
Original file line number	Diff line number	Diff line change
`@@ -173,7 +173,7 @@ protected NodeInfo<? extends Expression> info() {`
`173`	`173`	`public Object fold() {`
`174`	`174`	`var leftGeom = makeGeometryFromLiteral(left());`
`175`	`175`	`var rightGeom = makeGeometryFromLiteral(right());`
`176`		`- return (crsType == SpatialCrsType.GEO) ? GEO.distance(leftGeom, rightGeom) : CARTESIAN.distance(leftGeom, rightGeom);`
	`176`	`+ return (crsType() == SpatialCrsType.GEO) ? GEO.distance(leftGeom, rightGeom) : CARTESIAN.distance(leftGeom, rightGeom);`
`177`	`177`	`}`
`178`	`178`
`179`	`179`	`@Override`