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

Issue 54628 #58151

Closed
wants to merge 8 commits into from
Closed

Issue 54628 #58151

wants to merge 8 commits into from

Conversation

Shubhangi2901
Copy link

@Shubhangi2901 Shubhangi2901 commented Jun 15, 2020

PR for Enable geo_distance and geo_bounding_box queries on geo_shape field type

  • I have added validation in geoDistanceQueryBuilder and geoBoundingBoxQueryBuilder

@@ -312,10 +327,15 @@ public Query doToQuery(QueryShardContext context) {
throw new QueryShardException(context, "failed to find geo_point field [" + fieldName + "]");
}
}
if (!(fieldType instanceof GeoPointFieldType)) {
throw new QueryShardException(context, "field [" + fieldName + "] is not a geo_point field");
// if (!(fieldType instanceof GeoPointFieldType)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that above refers just to geo_point ("failed to find geo_point field ["). That might need to be updated.

@iverase
Copy link
Contributor

iverase commented Jun 16, 2020

Validation looks good. Next thing is to change the way we are building the queries by using AbstractGeometryFieldType#geometryQueryBuilder()

@Shubhangi2901
Copy link
Author

Hi @iverase,

I have updated GeoBoundingBox and GeoDistance to inherit and implement AbstractGeometryQueryBuilder as given in GeoShapeQueryBuilder. I have added the following functions from GeoShapeQuery:

  1. buildShapeQuery(QueryShardContext context, MappedFieldType fieldType)
  2. GeoDistanceQueryBuilder fromXContent(XContentParser parser)

Let me know if I have progressed in the right direction. Also if new issues should be created to update docs with new changes.

@iverase
Copy link
Contributor

iverase commented Jun 24, 2020

Thanks @Shubhangi2901!

I had a look into your changes and I don't think this queries should implement AbstractGeometryQueryBuilder as these are very specific queries and they do not do as much things as that abstraction. I think it is enough to just update the way we are building the queries, so for example for geo_circle queries will be enough to update the query logic to:

       final AbstractGeometryFieldMapper.AbstractGeometryFieldType ft =
            (AbstractGeometryFieldMapper.AbstractGeometryFieldType) fieldType;

        Circle circle = new Circle(center.lon(), center.lat(), this.distance);
        return ft.geometryQueryBuilder().process(circle, fieldType.name(), SpatialStrategy.RECURSIVE,
            ShapeRelation.INTERSECTS, shardContext);

Note that we need to update the test so all the possibilities are tested.

@nik9000 nik9000 added the :Analytics/Geo Indexing, search aggregations of geo points and shapes label Jul 7, 2020
@elasticmachine
Copy link
Collaborator

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

@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 7, 2020
@iverase
Copy link
Contributor

iverase commented Oct 27, 2020

I am going to close this PR in favour of #64224.

@iverase iverase closed this Oct 27, 2020
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 Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants