Skip to content

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

Merged
talevy merged 17 commits intoelastic:masterfrom
talevy:geogrid-bounds
Jan 14, 2020
Merged

Adds support for geo-bounds filtering in geogrid aggregations#50002
talevy merged 17 commits intoelastic:masterfrom
talevy:geogrid-bounds

Conversation

@talevy
Copy link
Copy Markdown
Contributor

@talevy talevy commented Dec 9, 2019 •
edited
Loading

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
Copy Markdown
Collaborator

elasticmachine commented Dec 9, 2019

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
nknize
nknize suggested changes Dec 13, 2019
View reviewed changes
Copy link
Copy Markdown
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.

@talevy
Centralize BoundingBox logic to a dedicated class
cceda1f 
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
@talevy talevy mentioned this pull request Dec 16, 2019
Merged
@talevy
Adds support for geo-bounds filtering in geogrid aggregations
af29e5e 
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.
@talevy talevy requested a review from nknize January 6, 2020 21:41
@imotov imotov mentioned this pull request Jan 8, 2020
Closed
iverase
iverase reviewed Jan 9, 2020
View reviewed changes
Copy link
Copy Markdown
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.

@talevy talevy mentioned this pull request Jan 10, 2020
Closed
@talevy talevy requested a review from iverase January 10, 2020 16:30
iverase
iverase approved these changes Jan 14, 2020
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

talevy commented Jan 14, 2020

@elasticmachine update branch

thanks Ignacio!

@talevy
Copy link
Copy Markdown
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
This was referenced Jan 14, 2020
Merged
Merged
talevy added a commit that referenced this pull request Jan 14, 2020
@talevy
[7.x] Adds support for geo-bounds filtering in geogrid aggregations (#…
9ee2e11 
…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
@talevy
re-enable bwc tests and update geo-grid bounds version guard to 7.6 a…
26fed22 
…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
@talevy @SivagurunathanV
Adds support for geo-bounds filtering in geogrid aggregations (elasti…
ab8627b 
…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
@talevy @SivagurunathanV
re-enable bwc tests and update geo-grid bounds version guard to 7.6 a…
d976f31 
…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
This was referenced Feb 3, 2020
Closed
Closed
@thomasneirynck thomasneirynck mentioned this pull request Feb 5, 2020
Closed
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Feb 20, 2020
@russcam
Add support for bounds in geohash_grid aggregation
126d709 
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.
@russcam russcam mentioned this pull request Feb 20, 2020
Merged
Mpdreamz pushed a commit to elastic/elasticsearch-net that referenced this pull request Feb 21, 2020
@russcam
Add support for bounds in geohash_grid aggregation (#4403)
8ed67e3 
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
@russcam @github-actions
Add support for bounds in geohash_grid aggregation (#4403)
72f590a 
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
@github-actions @russcam
[Backport 7.6] Add support for bounds in geohash_grid aggregation (#4422
2dfc732 
)

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 <russ.cam@elastic.co>
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Feb 23, 2020
@russcam
Add support for bounds in geohash_grid aggregation (#4403)
501d90b 
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

Reviewers

@iverase iverase iverase approved these changes

@nknize nknize Awaiting requested review from nknize

Assignees

No one assigned

Labels

:Analytics/Geo Indexing, search aggregations of geo points and shapes >enhancement v7.6.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@talevy @elasticmachine @nknize @iverase @jakelandis