-
Notifications
You must be signed in to change notification settings - Fork 25.3k
ES|QL Support for ST_GEOHASH, ST_GEOTILE and ST_GEOHEX #125143
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
Conversation
e3b2faf
to
5c3d0ee
Compare
5c3d0ee
to
2e7e642
Compare
} | ||
|
||
protected static BytesRef calculateGeotile(Point point, int precision, Rectangle bounds) { | ||
// For points, filtering the point is as good as filtering the tile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. The current code was a quick-and dirty placeholder for the draft PR. I'll definitely switch over to the better code you suggest.
2e7e642
to
afd6a94
Compare
bcd5ec4
to
9f851d1
Compare
I was trying queries like:
In case of geohex and geotile we provide a good error although looking into the stack trace it feels to me that we are not checking the input while parsing and we only fail when trying to apply the function to the point. More over, for geohash it does not throw an error. Let's improve the checking of input parameters. |
9f851d1
to
e362e57
Compare
eadd44d
to
ff82ace
Compare
This is because we want to move the ST_GEOXXX functions to produce longs only, for performance reasons, and need the convenience functions to display these either as strings or as actual shapes. This is both useful to the users, as well as easier to do manual tests where we visualize the results on a map.
This involved copying the H3 utils from spatial plugin to esql. We should rather find a common location or dependency for these.
And refine tests to better cover bounds parameter, fixing missing type information in the docs
FROM airports | ||
| EVAL points = ["POINT(0.0 30.0)", "POINT(12.0 60.0)"] | ||
| EVAL geohash = ST_GEOHASH(location, 2, TO_GEOPOINT(points)) | ||
| WHERE geohash IS NOT NULL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I understand now. It is strange how the bounds are defined here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Support for bounds defined by points is now removed.
Now only geo_shape bounds are supported
Also enhance tests to validate against actual bounds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I think we need to strength the test coverage for multi-value case but we can do it in a follow up
💔 Backport failed
You can use sqren/backport to manually backport by running |
Added support for the three primary scalar grid functions: * `ST_GEOHASH(geom, precision)` * `ST_GEOTILE(geom, precision)` * `ST_GEOHEX(geom, precision)` As well as versions of these three that take an optional `geo_shape` boundary (must be a `BBOX` ie. `Rectangle`). And also supporting conversion functions that convert the grid-id from long to string and back to long. This work represents the core of the feature to support geo-grid aggregations in ES|QL.
Added support for the three primary scalar grid functions: * `ST_GEOHASH(geom, precision)` * `ST_GEOTILE(geom, precision)` * `ST_GEOHEX(geom, precision)` As well as versions of these three that take an optional `geo_shape` boundary (must be a `BBOX` ie. `Rectangle`). And also supporting conversion functions that convert the grid-id from long to string and back to long. This work represents the core of the feature to support geo-grid aggregations in ES|QL.
… (#128938) * ES|QL Support for ST_GEOHASH, ST_GEOTILE and ST_GEOHEX (#125143) Added support for the three primary scalar grid functions: * `ST_GEOHASH(geom, precision)` * `ST_GEOTILE(geom, precision)` * `ST_GEOHEX(geom, precision)` As well as versions of these three that take an optional `geo_shape` boundary (must be a `BBOX` ie. `Rectangle`). And also supporting conversion functions that convert the grid-id from long to string and back to long. This work represents the core of the feature to support geo-grid aggregations in ES|QL. * Remove license checking mechanism from tests (unsupported in 8.x) * Add 8.x level generated docs * Link new spatial functions into the docs * Remove unused imports after removal of license level tests * Fix list syntax * Fix asciidoc links
Added support for the three primary scalar grid functions: * `ST_GEOHASH(geom, precision)` * `ST_GEOTILE(geom, precision)` * `ST_GEOHEX(geom, precision)` As well as versions of these three that take an optional `geo_shape` boundary (must be a `BBOX` ie. `Rectangle`). And also supporting conversion functions that convert the grid-id from long to string and back to long. This work represents the core of the feature to support geo-grid aggregations in ES|QL.
#125143 added 9 spatial grid functions and released them into Serverless. We think this is not the best long-term approach and the functions in #129581 are likely better. As a first step, rmove the spatial grid functions added in #125143 from release builds so they don't get released into 8.19/9.1. --------- Co-authored-by: Craig Taverner <[email protected]>
elastic#125143 added 9 spatial grid functions and released them into Serverless. We think this is not the best long-term approach and the functions in elastic#129581 are likely better. As a first step, rmove the spatial grid functions added in elastic#125143 from release builds so they don't get released into 8.19/9.1. --------- Co-authored-by: Craig Taverner <[email protected]> (cherry picked from commit efb1397) # Conflicts: # docs/reference/query-languages/esql/_snippets/lists/spatial-functions.md # docs/reference/query-languages/esql/functions-operators/spatial-functions.md
#125143 added 9 spatial grid functions and released them into Serverless. We think this is not the best long-term approach and the functions in #129581 are likely better. As a first step, remove the spatial grid functions added in #125143 from release builds so they don't get released into 8.19/9.1. --------- Co-authored-by: Craig Taverner <[email protected]> (cherry picked from commit efb1397) # Conflicts: # docs/reference/query-languages/esql/_snippets/lists/spatial-functions.md # docs/reference/query-languages/esql/functions-operators/spatial-functions.md
elastic#125143 added 9 spatial grid functions and released them into Serverless. We think this is not the best long-term approach and the functions in elastic#129581 are likely better. As a first step, rmove the spatial grid functions added in elastic#125143 from release builds so they don't get released into 8.19/9.1. --------- Co-authored-by: Craig Taverner <[email protected]>
In support of the feature request at #123903
Checklist:
GeoTileBoundedPredicate
for correctgeotile
bounds filteringGeohashBoundedPredicate
for correctgeohash
bounds filteringGeoHexCellIdSource.GeoHexPredicate
andH3SphericalUtil
classes into esql module and use them for correct H3 bounds filteringlong
grid-ids instead ofkeyword
for performance and add new functions forkeyword
alternativeslong
andkeyword
versions of the grid-ids| WHERE ST_GEOHEX(location, 1) == ST_GEOHEX_TO_LONG("812bbffffffffff")
)_TO_GEOSHAPE
functionsgeo_shape
of an envelope (no hidden envelope calculation)_TO_GEOSHAPE
functions, including enhanced WKB (include grid-id)GeoGridQueryBuilder
, and copy to ES|QL)geo_shape
fields (including license checks)Fixes #123903