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

https://github.com/glennvill/elasticsearch.git #85776

Closed

Conversation

glennvill
Copy link

fixed query builder so that top can be the same as bottom

As the same bounding box on a geo_shape query works. Lucene supports it so this fix is a must

@elasticsearchmachine elasticsearchmachine added v8.3.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Apr 11, 2022
@gmarouli gmarouli added the :Analytics/Geo Indexing, search aggregations of geo points and shapes label Apr 11, 2022
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Apr 11, 2022
@elasticmachine
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.

Missed the second check. Also, there really should be tests for this.

@@ -119,8 +119,6 @@ public GeoBoundingBoxQueryBuilder setCorners(double top, double left, double bot
// all corners are valid after above checks - make sure they are in the right relation
if (top < bottom) {
throw new IllegalArgumentException("top is below bottom corner: " + top + " vs. " + bottom);
} else if (top == bottom) {
throw new IllegalArgumentException("top cannot be the same as bottom: " + top + " == " + bottom);
} else if (left == right) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You missed the left == right check which should also be removed.

@craigtaverner
Copy link
Contributor

It appears this work was already done by @jchrys on the same date (11th April) in the PR at #85788, which was reviewed by @iverase and merged into 8.3.0.

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 external-contributor Pull request authored by a developer outside the Elasticsearch team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants