Support geohash, geotile and geohex grid types #129581
Conversation
craigtaverner
added
>enhancement
:Analytics/Geo
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
|
Hi @craigtaverner, I've created a changelog YAML for you. |
Added EsqlCapabily to block older BWC Changed license IT to new API
| long geoGridId = getGridId(point, gridId, gridType); | ||
| return gridId == geoGridId; | ||
| } else { | ||
| throw new IllegalArgumentException( |
There was a problem hiding this comment.
This should be easy to implement though. It is ok to do it in a follow up
There was a problem hiding this comment.
More over, I think we should do it generic, no point in distinguish a point from another geometry, we have code that that this already.
There was a problem hiding this comment.
We need to carefully consider how to deal with the breaking change that this proposes, because we remove some functions altogether.
Additionally, the new functions added by this aren't snapshot-only, either, and will be made available immediately. It's better to have them bake a little before un-snapshotting them in a dedicated PR.
Finally, this adds multiple data types and wires them immediately - I much prefer we do that in a dedicated PR, this is a big change and can easily introduce bugs.
| def(StGeohashToLong.class, StGeohashToLong::new, "st_geohash_to_long"), | ||
| def(StGeohashToString.class, StGeohashToString::new, "st_geohash_to_string"), | ||
| def(StGeotile.class, StGeotile::new, "st_geotile"), | ||
| def(StGeotileToLong.class, StGeotileToLong::new, "st_geotile_to_long"), | ||
| def(StGeotileToString.class, StGeotileToString::new, "st_geotile_to_string"), | ||
| def(StGeohex.class, StGeohex::new, "st_geohex"), | ||
| def(StGeohexToLong.class, StGeohexToLong::new, "st_geohex_to_long"), | ||
| def(StGeohexToString.class, StGeohexToString::new, "st_geohex_to_string") }, |
There was a problem hiding this comment.
This change is breaking. These functions were not snapshot-only (would have had to be placed in the snapshotFunctions below for that).
The PR at #125143 added support for
ST_GEOHASH,ST_GEOTILEandST_GEOHEX. However, since it usedlongas the internal type for the grid id, there was need for many additional functions for converting thelongto and fromkeywordas well as generatinggeo_shapecell bounds for map display. Each pf these involved many more files (for the functions, their docs and the generated evaluators). With inspiration from PostGIS we decided to take a different direction, and instead use a new internal type for each grid:geohashfor theST_GEOHASHfunction, created from literal using eitherTO_GEOHASH(hash)orhash::geohashgeotilefor theST_GEOTILEfunction, created from literal using eitherTO_GEOTILE(tile)ortile::geotilegeohexfor theST_GEOHEXfunction, created from literal using eitherTO_GEOHEX(h3)orh3::geohexThis also leads to much stricter type checking as we can no longer use the
longas a plainlongin all functions that accept longs, or inadvertently using a geohash in a geotile function. However, the addition of new types involves a lot of boilerplate, especially considering the large number of functions that operate on all types, and need to be informed of the existence of these three new types.One of the main goals of this work was to also support the concept of a geogrid search. This means "find all documents that intersect a grid cell", similar to the Query DSL version. This is achieved by enabling the use of these three new types in the
ST_INTERSECTSfunction, so a query like this would satisfy the requirement: