Skip to content

When running geo_line aggregation, the sort_order has no effect #94733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
craigtaverner opened this issue Mar 24, 2023 · 1 comment · Fixed by #94734
Closed

When running geo_line aggregation, the sort_order has no effect #94733

craigtaverner opened this issue Mar 24, 2023 · 1 comment · Fixed by #94734
Assignees
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@craigtaverner
Copy link
Contributor

craigtaverner commented Mar 24, 2023

Elasticsearch Version

all supported versions

Installed Plugins

No response

Java Version

bundled

OS Version

MacOSX (Darwin Kernel Version 22.3.0 arm64)

Problem Description

The documentation for geo_line claims that the option sort_order can be either ASC (default) or DESC:

This option accepts one of two values: "ASC", "DESC".
The line is sorted in ascending order by the sort key when set to "ASC", and in descending with "DESC".

However, when testing this, no matter which you choose the actual order of points in the result is always ASC by the value of the sort field.

Steps to Reproduce

Create an index with a geo_point and a

PUT /locations
{
  "mappings": {
    "properties": {
      "location":    { "type": "geo_point" },
      "rank":  { "type": "double"  }
    }
  }
}

Add data with appropriate rank values:

POST /locations/_bulk?refresh
{"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 }

Query with ASC sort order:

POST /locations/_search?filter_path=aggregations
{
  "aggs": {
    "line": {
      "geo_line": {
        "point": {"field": "location"},
        "sort": {"field": "rank"},
        "include_sort": true,
        "sort_order": "ASC"
      }
    }
  }
}

And get expected result:

{
  "aggregations": {
    "line": {
      "type" : "Feature",
      "geometry" : {
        "type" : "LineString",
        "coordinates" : [
          [
            13.378421,
            47.888321
          ],
          [
            13.37183,
            48.20842
          ],
          [
            13.371398,
            47.829303
          ]
        ]
      },
      "properties" : {
        "complete" : true,
        "sort_values": [0.0, 1.2, 2.0]
      }
    }
  }
}

Query with DESC sort order:

POST /locations/_search?filter_path=aggregations
{
  "aggs": {
    "line": {
      "geo_line": {
        "point": {"field": "location"},
        "sort": {"field": "rank"},
        "include_sort": true,
        "sort_order": "DESC"
      }
    }
  }
}

But you get exactly the same result as the previous query. However, we expect the sort order to have been reversed and we expect the following result instead:

{
  "aggregations": {
    "line": {
      "type" : "Feature",
      "geometry" : {
        "type" : "LineString",
        "coordinates" : [
          [
            13.371398,
            47.829303
          ],
          [
            13.37183,
            48.20842
          ],
          [
            13.378421,
            47.888321
          ]
        ]
      },
      "properties" : {
        "complete" : true,
        "sort_values": [2.0, 1.2, 0.0]
      }
    }
  }
}

Logs (if relevant)

No response

@craigtaverner craigtaverner added >bug needs:triage Requires assignment of a team area label :Analytics/Geo Indexing, search aggregations of geo points and shapes Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) and removed needs:triage Requires assignment of a team area label labels Mar 24, 2023
@craigtaverner craigtaverner self-assigned this Mar 24, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

craigtaverner added a commit to craigtaverner/elasticsearch that referenced this issue Mar 24, 2023
The code was re-sorting to ASC explicitly in the reduce phase.
No explanation was given. Removing this extra sort causes the
new yaml test to pass. Two failing java tests were fixed by no
longer explicitly expected ASC data when order was DESC.
craigtaverner added a commit that referenced this issue Mar 28, 2023
* Move unused geoline tests into correct location

The original geo_line PR included two yaml test files, one of which
was never used. This commit moves that test into the correct place,
and fixes a syntax error in the test.

* Fix geo_line sort order (#94733)

The code was re-sorting to ASC explicitly in the reduce phase.
No explanation was given. Removing this extra sort causes the
new yaml test to pass. Two failing java tests were fixed by no
longer explicitly expected ASC data when order was DESC.

* Update docs/changelog/94734.yaml

* Improve changelog text

* Added test for size in geo_line

* Add geo_line sort and limit test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants