Browse Source

Fix ST_CENTROID_AGG when no records are aggregated (#114888) (#114901)

This was returning an invalid result `POINT(NaN NaN)` and now instead returns `null`.
Craig Taverner 1 year ago
parent
commit
5a0934679d

+ 6 - 0
docs/changelog/114888.yaml

@@ -0,0 +1,6 @@
+pr: 114888
+summary: Fix ST_CENTROID_AGG when no records are aggregated
+area: ES|QL
+type: bug
+issues:
+ - 106025

+ 9 - 5
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/spatial/CentroidPointAggregator.java

@@ -58,7 +58,7 @@ abstract class CentroidPointAggregator {
     }
 
     public static Block evaluateFinal(CentroidState state, DriverContext driverContext) {
-        return driverContext.blockFactory().newConstantBytesRefBlockWith(state.encodeCentroidResult(), 1);
+        return state.toBlock(driverContext.blockFactory());
     }
 
     public static void combineStates(GroupingCentroidState current, int groupId, GroupingCentroidState state, int statePosition) {
@@ -181,10 +181,14 @@ abstract class CentroidPointAggregator {
             this.count += count;
         }
 
-        protected BytesRef encodeCentroidResult() {
-            double x = xSum.value() / count;
-            double y = ySum.value() / count;
-            return encode(x, y);
+        protected Block toBlock(BlockFactory blockFactory) {
+            if (count > 0) {
+                double x = xSum.value() / count;
+                double y = ySum.value() / count;
+                return blockFactory.newConstantBytesRefBlockWith(encode(x, y), 1);
+            } else {
+                return blockFactory.newConstantNullBlock(1);
+            }
         }
     }
 

+ 39 - 2
x-pack/plugin/esql/qa/testFixtures/src/main/resources/spatial.csv-spec

@@ -616,6 +616,42 @@ location:geo_point  | city_location:geo_point  |  count:long
 POINT (0 0)         | POINT (0 0)              |  1
 ;
 
+airportCityLocationPointIntersectionCentroidGroups
+required_capability: st_intersects
+
+FROM airports_mp
+| WHERE ST_INTERSECTS(location, city_location)
+| STATS location=ST_CENTROID_AGG(location), city_location=ST_CENTROID_AGG(city_location), count=COUNT() BY country
+;
+
+location:geo_point  | city_location:geo_point  |  count:long | country:k
+POINT (0 0)         | POINT (0 0)              |  1          | Atlantis
+;
+
+airportCityLocationPointIntersectionNullCentroid
+required_capability: st_intersects
+required_capability: spatial_centroid_no_records
+
+FROM airports
+| WHERE ST_INTERSECTS(location, city_location)
+| STATS location=ST_CENTROID_AGG(location), city_location=ST_CENTROID_AGG(city_location), count=COUNT()
+;
+
+location:geo_point  | city_location:geo_point  |  count:long
+null                | null                     |  0
+;
+
+airportCityLocationPointIntersectionNullCentroidGroups
+required_capability: st_intersects
+
+FROM airports
+| WHERE ST_INTERSECTS(location, city_location)
+| STATS location=ST_CENTROID_AGG(location), city_location=ST_CENTROID_AGG(city_location), count=COUNT() BY country
+;
+
+location:geo_point  | city_location:geo_point  |  count:long | country:k
+;
+
 ###############################################
 # Tests for ST_DISJOINT on GEO_POINT type
 
@@ -1948,14 +1984,15 @@ wkt:keyword   | pt:cartesian_point
 
 cartesianCentroidFromAirportsAfterPointContainsPolygonPredicate
 required_capability: st_contains_within
+required_capability: spatial_centroid_no_records
 
 FROM airports_web
 | WHERE ST_CONTAINS(location, TO_CARTESIANSHAPE("POLYGON((4700000 1600000, 4800000 1600000, 4800000 1700000, 4700000 1700000, 4700000 1600000))"))
 | STATS centroid=ST_CENTROID_AGG(location), count=COUNT()
 ;
 
-centroid:cartesian_point     | count:long
-POINT (NaN NaN)              | 0
+centroid:cartesian_point | count:long
+null                     | 0
 ;
 
 cartesianPointContainsPolygonPredicate

+ 5 - 0
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

@@ -180,6 +180,11 @@ public class EsqlCapabilities {
          */
         SPATIAL_DISTANCE_PUSHDOWN_ENHANCEMENTS,
 
+        /**
+         * Fix for spatial centroid when no records are found.
+         */
+        SPATIAL_CENTROID_NO_RECORDS,
+
         /**
          * Fix to GROK and DISSECT that allows extracting attributes with the same name as the input
          * https://github.com/elastic/elasticsearch/issues/110184