Skip to content
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

Try to simplify geometries that fail with TopologyException #115834

Merged
merged 2 commits into from
Oct 29, 2024

Conversation

iverase
Copy link
Contributor

@iverase iverase commented Oct 29, 2024

During some testing I noticed that some geometries where failing with a topolygy exception when computing the intersection. This error is not seeing when reading from source because we are swallowing the exceptions but it is visible when doing it from the stored value.

This geometries are valid and they can actually be simplified so lets make the clipping algorithm a best effort and return the original geometry in those cases so the simplification can handle it.

Note: I am working on adding a test but we need to clear the license of the geometry I am using and that might take time.

@iverase iverase added >bug :Analytics/Geo Indexing, search aggregations of geo points and shapes v9.0.0 v8.16.1 v8.17.0 labels Oct 29, 2024
@iverase iverase requested a review from craigtaverner October 29, 2024 09:56
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Oct 29, 2024
@elasticsearchmachine
Copy link
Collaborator

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

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.

Since we deal with envelope disjoint and contains separately, this seems like a safe enough approach as it should mostly be geometries that are likely intersecting. I guess the worst case scenario is a big geometry that intersects the window with a small corner, and simplifying just that corner would be best, and now we'll simplify the whole geometry. Still better than throwing the exception.

docs/changelog/115834.yaml Show resolved Hide resolved
@iverase iverase added v8.15.3 v8.15.4 auto-backport Automatically create backport pull requests when merged and removed v8.15.3 labels Oct 29, 2024
@iverase iverase merged commit 04682dc into elastic:main Oct 29, 2024
15 of 16 checks passed
@iverase iverase deleted the ignoreTopologyExceptions branch October 29, 2024 11:19
iverase added a commit to iverase/elasticsearch that referenced this pull request Oct 29, 2024
…115834)

This geometries are valid and they can actually be simplified so lets make the clipping algorithm a best effort and 
return the original geometry in those cases so the simplification can handle it.
iverase added a commit to iverase/elasticsearch that referenced this pull request Oct 29, 2024
…115834)

This geometries are valid and they can actually be simplified so lets make the clipping algorithm a best effort and 
return the original geometry in those cases so the simplification can handle it.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.15
8.16
8.x

iverase added a commit to iverase/elasticsearch that referenced this pull request Oct 29, 2024
…115834)

This geometries are valid and they can actually be simplified so lets make the clipping algorithm a best effort and 
return the original geometry in those cases so the simplification can handle it.
iverase added a commit that referenced this pull request Oct 30, 2024
…#115841)

This geometries are valid and they can actually be simplified so lets make the clipping algorithm a best effort and 
return the original geometry in those cases so the simplification can handle it.
elasticsearchmachine pushed a commit that referenced this pull request Oct 30, 2024
…#115842)

This geometries are valid and they can actually be simplified so lets make the clipping algorithm a best effort and 
return the original geometry in those cases so the simplification can handle it.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
iverase added a commit that referenced this pull request Oct 30, 2024
…#115843)

This geometries are valid and they can actually be simplified so lets make the clipping algorithm a best effort and 
return the original geometry in those cases so the simplification can handle it.
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Nov 4, 2024
…115834)

This geometries are valid and they can actually be simplified so lets make the clipping algorithm a best effort and 
return the original geometry in those cases so the simplification can handle it.
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 auto-backport Automatically create backport pull requests when merged >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.15.4 v8.16.1 v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants