Skip to content

Conversation

@talevy
Copy link
Contributor

@talevy talevy commented Dec 9, 2019

It is fairly common to filter the geo point candidates in
geohash_grid and geotile_grid aggregations according to some
viewable bounding box. This change introduces the option of
specifying this filter directly in the tiling aggregation.

This is even more relevant to geo_shape where the bounds will restrict
the shape to be within the bounds

this optional bounds parameter is parsed in an equivalent fashion to
the bounds specified in the geo_bounding_box query.

example:

POST /museums/_search?size=0
{
    "aggregations" : {
        "tiles-in-bounds" : {
            "geohash_grid" : {
                "field" : "location",
                "precision" : 8,
                "bounds": {
                  "top_left" : "53.4375, 4.21875",
                  "bottom_right" : "52.03125, 5.625"
                }
            }
        }
    }
}

@talevy talevy added WIP :Analytics/Geo Indexing, search aggregations of geo points and shapes labels Dec 9, 2019
@elasticmachine
Copy link
Collaborator

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

@talevy talevy added >enhancement and removed WIP labels Dec 11, 2019
@talevy talevy requested review from iverase and nknize December 11, 2019 02:13
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.

Overall it looks good. Left a few minor thoughts and suggestions. At minimum I think we should use the Lucene test utilities where we can. Separate from that I think we can introduce a little more randomization to the tests.

Both geo_bounding_box query and geo_bounds aggregation have
a very similar definition of a "bounding box". A lot of this
logic (serialization, xcontent-parsing, etc) can be centralized
instead of having separated efforts to do the same things
It is fairly common to filter the geo point candidates in
geohash_grid and geotile_grid aggregations according to some
viewable bounding box. This change introduces the option of
specifying this filter directly in the tiling aggregation.
Copy link
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

I think this is almost there, just want to think if we should use different implementations for the bounded and unbounded case when iterating doc values.

Copy link
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

lgtm

@talevy
Copy link
Contributor Author

talevy commented Jan 14, 2020

@elasticmachine update branch

thanks Ignacio!

@talevy
Copy link
Contributor Author

talevy commented Jan 14, 2020

@elasticmachine update branch

@talevy talevy added the v8.0.0 label Jan 14, 2020
@talevy talevy merged commit 6c86606 into elastic:master Jan 14, 2020
@talevy talevy deleted the geogrid-bounds branch January 14, 2020 16:29
talevy added a commit that referenced this pull request Jan 14, 2020
…50996)

* Adds support for geo-bounds filtering in geogrid aggregations (#50002)

It is fairly common to filter the geo point candidates in
geohash_grid and geotile_grid aggregations according to some
viewable bounding box. This change introduces the option of
specifying this filter directly in the tiling aggregation.

This is even more relevant to `geo_shape` where the bounds will restrict
the shape to be within the bounds

this optional `bounds` parameter is parsed in an equivalent fashion to
the bounds specified in the geo_bounding_box query.
talevy added a commit that referenced this pull request Jan 14, 2020
…fter #50996 (#50997)

after #50996 (backport of #50002) merged, the version guard 
on geo-grid `bounds` parameter can be updated to 7.6

re-enables bwc tests
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
…c#50002)

It is fairly common to filter the geo point candidates in
geohash_grid and geotile_grid aggregations according to some
viewable bounding box. This change introduces the option of
specifying this filter directly in the tiling aggregation.

This is even more relevant to `geo_shape` where the bounds will restrict
the shape to be within the bounds

this optional `bounds` parameter is parsed in an equivalent fashion to 
the bounds specified in the geo_bounding_box query.
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
…fter elastic#50996 (elastic#50997)

after elastic#50996 (backport of elastic#50002) merged, the version guard 
on geo-grid `bounds` parameter can be updated to 7.6

re-enables bwc tests
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Feb 20, 2020
Relates: #4341, elastic/elasticsearch#50002

This commit adds support for supplying bounds on a geohash_grid
aggregation, similar to how bounds are supplied on
geo_bounding_box query.
Mpdreamz pushed a commit to elastic/elasticsearch-net that referenced this pull request Feb 21, 2020
Relates: #4341, elastic/elasticsearch#50002

This commit adds support for supplying bounds on a geohash_grid
aggregation, similar to how bounds are supplied on
geo_bounding_box query.
github-actions bot pushed a commit to elastic/elasticsearch-net that referenced this pull request Feb 21, 2020
Relates: #4341, elastic/elasticsearch#50002

This commit adds support for supplying bounds on a geohash_grid
aggregation, similar to how bounds are supplied on
geo_bounding_box query.
Mpdreamz pushed a commit to elastic/elasticsearch-net that referenced this pull request Feb 21, 2020
)

Relates: #4341, elastic/elasticsearch#50002

This commit adds support for supplying bounds on a geohash_grid
aggregation, similar to how bounds are supplied on
geo_bounding_box query.

Co-authored-by: Russ Cam <[email protected]>
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Feb 23, 2020
Relates: #4341, elastic/elasticsearch#50002

This commit adds support for supplying bounds on a geohash_grid
aggregation, similar to how bounds are supplied on
geo_bounding_box query.

(cherry picked from commit 8ed67e3)
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 >enhancement v7.6.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants