Fix bug 94733 geo_line not respecting sort_order

docs/changelog/94734.yaml

@@ -0,0 +1,6 @@
+pr: 94734
+summary: Fix bug where `geo_line` does not respect `sort_order`
+area: Geo
+type: bug
+issues:
+ - 94733

x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/search/aggregations/InternalGeoLine.java

@@ -110,10 +110,6 @@ public InternalAggregation reduce(List<InternalAggregation> aggregations, Aggreg
 
         MergedGeoLines mergedGeoLines = new MergedGeoLines(internalGeoLines, finalSize, sortOrder);
         mergedGeoLines.merge();
-        // the final reduce should always be in ascending order
-        if (reduceContext.isFinalReduce() && SortOrder.DESC.equals(sortOrder)) {
-            new PathArraySorter(mergedGeoLines.getFinalPoints(), mergedGeoLines.getFinalSortValues(), SortOrder.ASC).sort();
-        }
         return new InternalGeoLine(
             name,
             mergedGeoLines.getFinalPoints(),

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/GeoLineAggregatorTests.java

@@ -323,13 +323,13 @@ private void testAggregator(SortOrder sortOrder) throws IOException {
                 int encodedLon = GeoEncodingUtils.encodeLongitude(point.getLon());
                 long lonLat = (((long) encodedLon) << 32) | encodedLat & 0xffffffffL;
                 points[i] = lonLat;
-                sortValues[i] = SortOrder.ASC.equals(sortOrder) ? i : numPoints - i;
+                sortValues[i] = i;
             }
             int lineSize = Math.min(numPoints, size);
             // re-sort line to be ascending
             long[] linePoints = Arrays.copyOf(points, lineSize);
             double[] lineSorts = Arrays.copyOf(sortValues, lineSize);
-            new PathArraySorter(linePoints, lineSorts, SortOrder.ASC).sort();
+            new PathArraySorter(linePoints, lineSorts, sortOrder).sort();
 
             lines.put(String.valueOf(groupOrd), new InternalGeoLine("_name", linePoints, lineSorts, null, complete, true, sortOrder, size));
 

x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/search/aggregations/InternalGeoLineTests.java

@@ -134,10 +134,6 @@ protected void assertReduced(InternalGeoLine reduced, List<InternalGeoLine> inpu
         long[] finalCappedPoints = Arrays.copyOf(finalList, Math.min(reduced.size(), mergedLength));
         double[] finalCappedSortVals = Arrays.copyOf(finalSortVals, Math.min(reduced.size(), mergedLength));
 
-        if (SortOrder.DESC.equals(reduced.sortOrder())) {
-            new PathArraySorter(finalCappedPoints, finalCappedSortVals, SortOrder.ASC).sort();
-        }
-
         assertArrayEquals(finalCappedSortVals, reduced.sortVals(), 0d);
         assertArrayEquals(finalCappedPoints, reduced.line());
     }

x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/spatial/60_geo_line.yml

@@ -11,6 +11,17 @@ setup:
               position:
                 type: geo_point
 
+  - do:
+      indices.create:
+        index: locations
+        body:
+          mappings:
+            properties:
+              location:
+                type: geo_point
+              rank:
+                type: double
+
   - do:
       indices.create:
         index: test1
@@ -49,6 +60,18 @@ setup:
           {"index":{}}
           {"position": "POINT(4.914722 52.371667)", "race_id": "Amsterdam", "timestamp": 10}
 
+  - do:
+      bulk:
+        index: locations
+        refresh: true
+        body: |
+          {"index":{}}
+          {"location": [13.37139831, 47.82930284], "rank": 2.0 }
+          {"index":{}}
+          {"location": [13.3784208402, 47.88832084022], "rank": 0.0 }
+          {"index":{}}
+          {"location": [13.371830148701, 48.2084200148], "rank": 1.2 }
+
   - do:
       bulk:
         index: test1
@@ -79,6 +102,128 @@ setup:
   - do:
       indices.refresh: { }
 
+---
+"Test geo_line aggregation on geo points sort by double and include_sort":
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: locations
+        size: 0
+        body:
+          aggs:
+            path:
+              geo_line:
+                include_sort: true
+                point:
+                  field: location
+                sort:
+                  field: rank
+  - match: { hits.total:  3 }
+  - match: { aggregations.path.type: "Feature" }
+  - match: { aggregations.path.geometry.type: "LineString" }
+  - length: { aggregations.path.geometry.coordinates: 3 }
+  - match: { aggregations.path.geometry.coordinates.0.0: 13.378421 }
+  - match: { aggregations.path.geometry.coordinates.0.1: 47.888321 }
+  - match: { aggregations.path.geometry.coordinates.1.0: 13.37183 }
+  - match: { aggregations.path.geometry.coordinates.1.1: 48.20842 }
+  - match: { aggregations.path.geometry.coordinates.2.0: 13.371398 }
+  - match: { aggregations.path.geometry.coordinates.2.1: 47.829303 }
+  - is_true: aggregations.path.properties.complete
+  - length: { aggregations.path.properties.sort_values: 3 }
+  - match: { aggregations.path.properties.sort_values.0: 0.0 }
+  - match: { aggregations.path.properties.sort_values.1: 1.2 }
+  - match: { aggregations.path.properties.sort_values.2: 2.0 }
+
+---
+"Test geo_line aggregation on geo points reverse sort by double and include_sort":
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: locations
+        size: 0
+        body:
+          aggs:
+            path:
+              geo_line:
+                include_sort: true
+                sort_order: "desc"
+                point:
+                  field: location
+                sort:
+                  field: rank
+  - match: { hits.total:  3 }
+  - match: { aggregations.path.type: "Feature" }
+  - match: { aggregations.path.geometry.type: "LineString" }
+  - length: { aggregations.path.geometry.coordinates: 3 }
+  - match: { aggregations.path.geometry.coordinates.0.0: 13.371398 }
+  - match: { aggregations.path.geometry.coordinates.0.1: 47.829303 }
+  - match: { aggregations.path.geometry.coordinates.1.0: 13.37183 }
+  - match: { aggregations.path.geometry.coordinates.1.1: 48.20842 }
+  - match: { aggregations.path.geometry.coordinates.2.0: 13.378421 }
+  - match: { aggregations.path.geometry.coordinates.2.1: 47.888321 }
+  - is_true: aggregations.path.properties.complete
+  - length: { aggregations.path.properties.sort_values: 3 }
+  - match: { aggregations.path.properties.sort_values.0: 2.0 }
+  - match: { aggregations.path.properties.sort_values.1: 1.2 }
+  - match: { aggregations.path.properties.sort_values.2: 0.0 }
+
+---
+"Test geo_line aggregation on geo points limit size":
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: locations
+        size: 0
+        body:
+          aggs:
+            path:
+              geo_line:
+                size: 2
+                point:
+                  field: location
+                sort:
+                  field: rank
+  - match: { hits.total:  3 }
+  - match: { aggregations.path.type: "Feature" }
+  - match: { aggregations.path.geometry.type: "LineString" }
+  - length: { aggregations.path.geometry.coordinates: 2 }
+  - match: { aggregations.path.geometry.coordinates.0.0: 13.378421 }
+  - match: { aggregations.path.geometry.coordinates.0.1: 47.888321 }
+  - match: { aggregations.path.geometry.coordinates.1.0: 13.37183 }
+  - match: { aggregations.path.geometry.coordinates.1.1: 48.20842 }
+  - is_false: aggregations.path.properties.complete
+
+---
+"Test geo_line aggregation on geo points limit and sort":
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        index: locations
+        size: 0
+        body:
+          aggs:
+            path:
+              geo_line:
+                size: 2
+                include_sort: true
+                sort_order: "desc"
+                point:
+                  field: location
+                sort:
+                  field: rank
+  - match: { hits.total:  3 }
+  - match: { aggregations.path.type: "Feature" }
+  - match: { aggregations.path.geometry.type: "LineString" }
+  - length: { aggregations.path.geometry.coordinates: 2 }
+  - match: { aggregations.path.geometry.coordinates.0.0: 13.371398 }
+  - match: { aggregations.path.geometry.coordinates.0.1: 47.829303 }
+  - match: { aggregations.path.geometry.coordinates.1.0: 13.37183 }
+  - match: { aggregations.path.geometry.coordinates.1.1: 48.20842 }
+  - is_false: aggregations.path.properties.complete
+  - length: { aggregations.path.properties.sort_values: 2 }
+  - match: { aggregations.path.properties.sort_values.0: 2.0 }
+  - match: { aggregations.path.properties.sort_values.1: 1.2 }
+
 ---
 "Test geo_line aggregation on geo points with no grouping":
   - do: