Skip to content

Conversation

@iverase
Copy link
Contributor

@iverase iverase commented Jul 10, 2023

We are currently creating empty geo_line with the flag non_overlapping equal to true which is causing issues when the driving geo_line for merging is this empty object. This PR make sure that empty geo_lines are created with the right flag.

fixes #97311

@iverase iverase added >bug :Analytics/Geo Indexing, search aggregations of geo points and shapes v8.10.0 v8.9.1 labels Jul 10, 2023
@iverase iverase requested a review from craigtaverner July 10, 2023 11:53
@elasticsearchmachine
Copy link
Collaborator

Hi @iverase, I've created a changelog YAML for you.

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 10, 2023
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Contributor

@craigtaverner craigtaverner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fascinating! I did not consider this edge case! An alternative implementation would be to change InternalGeoLine.reduce to not make the decision on non-overlapping based on it's own state, but on the state of all geo-lines being merged. We should only merge as non-overlapping if all geo-lines are non-overlapping. That would be a change to a single method.

@iverase iverase merged commit fdf980c into elastic:main Jul 10, 2023
@iverase iverase deleted the nonOverlapping branch July 10, 2023 23:54
@iverase
Copy link
Contributor Author

iverase commented Jul 10, 2023

I am fine with your proposed fix and thanks for the test.

@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.9
iverase added a commit to iverase/elasticsearch that referenced this pull request Jul 10, 2023
During reduce, only merge as non-overlapping if all geo-lines are non-overlapping. --------- Co-authored-by: Craig Taverner <craig@amanzi.com>
elasticsearchmachine pushed a commit that referenced this pull request Jul 11, 2023
During reduce, only merge as non-overlapping if all geo-lines are non-overlapping. --------- Co-authored-by: Craig Taverner <craig@amanzi.com>
@rjernst rjernst added v8.9.0 and removed v8.9.1 labels Jul 21, 2023
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) v8.9.0 v8.10.0

4 participants