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

Geo shape query vs geo point #52382

Merged
merged 109 commits into from
Mar 18, 2020

Conversation

djptek
Copy link
Contributor

@djptek djptek commented Feb 14, 2020

Enable geo_shape query to work on geo_point fields for shapes: circle, polygon, multipolygon, rectangle

see: #48928

many thanks to @iverase for fa64b52

djptek added 30 commits November 15, 2019 19:05
 in Class org.elasticsearch.search.geo.GeoPointShapeQueryTests
…tion raised, see org.elasticsearch.index.query.QueryShardContext.toQuery
GeoShapeQueryTests refactored adding superclass and sibling to accomodate new test coverage as part of adaptation of geo_shape query to work on geo_point fields
Copy link
Contributor

@nknize nknize left a comment

Choose a reason for hiding this comment

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

Looking good! A couple thoughts:

  1. We need to add testing for GeometryCollection; but this can come in a follow up PR
  2. ShapeQueryBuilder.validateContentTypes returns a new List instance every time it's called. We need to be careful there. We already called it twice in buildShapeQuery so that needs to be changed in this PR. We should probably make a static variable in the derived builders that call this once so we don't shoot ourselves in the foot in the future.

djptek and others added 2 commits March 17, 2020 18:52
…atial/index/query/ShapeQueryBuilder.java

Co-Authored-By: Nick Knize <[email protected]>
@nknize
Copy link
Contributor

nknize commented Mar 17, 2020

@elasticmachine run elasticsearch-ci/2
@elasticmachine run elasticsearch-ci/default-distro

@djptek
Copy link
Contributor Author

djptek commented Mar 17, 2020

@elasticmachine run elasticsearch-ci/2
@elasticmachine run elasticsearch-ci/default-distro

@droberts195
Copy link
Contributor

@elasticmachine update branch

Copy link
Contributor

@nknize nknize left a comment

Choose a reason for hiding this comment

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

Thanks @djptek and @iverase. This LGTM!

@djptek djptek merged commit d1cbdfb into elastic:master Mar 18, 2020
djptek added a commit to djptek-legacy/elasticsearch that referenced this pull request Mar 18, 2020
Enable geo_shape query to work on geo_point fields for shapes: circle, polygon, multipolygon, rectangle

see: elastic#48928

Co-Authored-By:  @iverase
@djptek djptek deleted the geo-shape-query-vs-geo-point branch March 20, 2020 12:54
@djptek djptek restored the geo-shape-query-vs-geo-point branch March 20, 2020 13:06
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants