Support geohash, geotile and geohex grid types in ST_INTERSECTS

[craigtaverner]

In the work at #129581 we added new geo-grid types: GEOHASH, GEOTILE and GEOHEX, as well as support functions for creating these from geo_point, long and keyword.

However, one of the key use cases we wish to replicate from the Query DSL is the grid search, and this involves the need to include the grid id inside a search predicate, in particular the ST_INTERSECTS function. For example:

FROM airports
| WHERE ST_INTERSECTS(location, TO_GEOTILE("3/4/3"))
| STATS
    count = COUNT(*),
    centroid = ST_CENTROID_AGG(location)

[elasticsearchmachine]

Pinging @elastic/es-analytical-engine (Team:Analytics)

[elasticsearchmachine]

Hi @craigtaverner, I've created a changelog YAML for you.

[github-actions]

🔍 Preview links for changed docs

• docs/reference/query-languages/esql/kibana/docs/functions/to_geohash.md
• docs/reference/query-languages/esql/kibana/docs/functions/to_geohex.md
• docs/reference/query-languages/esql/kibana/docs/functions/to_geotile.md
• docs/reference/query-languages/esql/kibana/docs/functions/st_geohash.md
• docs/reference/query-languages/esql/kibana/docs/functions/st_geohex.md
• docs/reference/query-languages/esql/kibana/docs/functions/st_geotile.md

[ivancea]

80% review here

[ivancea]

The capability is in snapshot mode, but the docs will be productive, as well as the type resolution. Is that right? If they will all be snapshot, should we add some snapshot check in resolveType()? (I didn't see it at least). Or maybe set it as preview instead of snapshot, and add an appliesTo to the docs?

[craigtaverner]

So we had a discussion about this in the previous PR. Making the docs generation snapshot aware is a much bigger lift. The previous PR leaks the types into several functions (TO_STRING, TO_LONG, and TO_GEOSHAPE in particular, but many more). The plan is to move this out of snapshot as soon as this new ST_INTERSECTS PR is merged (and give a couple of days to bake).

[craigtaverner]

We have applies_to for functions, but not for types. This is a subtle issue really, and we should discuss whether we should invest in applies_to support, or even snapshot support, for types. Right now neither option exists.

[ivancea]

Would an applies_to with preview (Or any other tag) work here, even if it's not "snaspshot" explicitly (We can add it in the description (?)).
Thinking not just about the snapshot period, but about the next release, as we'll have to add an appliesTo for the new version anyway (If I'm understanding the new docs correctly)

[github-actions]

ℹ️ Important: Docs version tagging

👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version.

We use applies_to tags to mark version-specific features and changes.

Expand for a quick overview

When to use applies_to tags:

✅ At the page level to indicate which products/deployments the content applies to (mandatory)
✅ When features change state (e.g. preview, ga) in a specific version
✅ When availability differs across deployments and environments

What NOT to do:

❌ Don't remove or replace information that applies to an older version
❌ Don't add new information that applies to a specific version without an applies_to tag
❌ Don't forget that applies_to tags can be used at the page, section, and inline level

🤔 Need help?

• Check out the cumulative docs guidelines
• Reach out in the #docs Slack channel

[ivancea]

nit: Should this method be abstract and implemented in the specific classes? Both to move special logics out of this base class, and to avoid forgetting it (Or not finding the places to update) when adding grid to WITHIN and CONTAINS

[craigtaverner]

Sounds like a nice idea. But at the moment only CONTAINS has a concrete sub-class, and all others (INTERSECTS, DISJOINT and WITHIN) are operating entirely in the base class. This is because CONTAINS has some special non-standard behaviour forced on us by Lucene. We could create new sub-classes specifically for dealing with the new geo-grids (and therefor remove one switch statement for the grids code path), but I think the performance optimization is negligable at this point. For performance, the best value for money would be to support Lucene pushdown. Perhaps once I add support for CONTAINS/WITHIN or shapes, it will make the move to multiple sub-classes more worth while?

[ivancea]

We already have the subclasses (SpatialIntersects and SpatialDisjoint, as well as the others). I was thinking about moving the method there, to have all the logic related with each of them in the functions.
Are you talking about other subclasses?

[craigtaverner]

Ah, this method compareGeometryAndGrid is in the SpatialRelatesFunction class which has only one subclass for CONTAINS. It does not have a reference to the original SpatialIntersects or other function classes, so cannot call methods on that. I could think of two ways of getting a function into those classes, either making it static (in which case we still need a switch here) or by passing a reference down into this class. Both approaches seem a little too complex. I think I'll defer considering options here until we support either shapes or contains/within, which might motivate for a refactoring, but even then I suspect the current approach might still be a little cleaner.

[ivancea]

LGTM!

[ivancea]

We already have the subclasses (SpatialIntersects and SpatialDisjoint, as well as the others). I was thinking about moving the method there, to have all the logic related with each of them in the functions.
Are you talking about other subclasses?