-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
ES|QL ST_DISTANCE Function #108764
ES|QL ST_DISTANCE Function #108764
Conversation
Hi @craigtaverner, I've created a changelog YAML for you. |
1b998f0
to
5e25ab3
Compare
11b5e45
to
f11e70c
Compare
Hi @craigtaverner, I've updated the changelog YAML for you. |
And note that we might want to include instead some of the related intelligence from Circle2D::HaversineDistance class
So we moved the common code to a separate SpatialTypeResolver, and made a simpler TernarySpatialFunction based on a simple TernaryScalarFunction. This had additional consequences, simplifying the points-only cases. The main reason for this change was to support StDWithinTests which need to test a lot of things that involve varying all three input types, generating expected error strings, etc. The original hack of just adding to BinarySpatialFunction worked for the actual integration tests, but clearly did not satisfy all the use cases tested by the unit tests. We also restricted ST_DWITHIN to take only a double as the third argument, because otherwise the number of evaluators would explode, since we need a separate evaluator for each Block type, and Integer and Double use different block types.
2f495be
to
a6c2fe1
Compare
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.
The PR is suppose to add a distance function, but it seems you are adding two functions here. I disagree on introducing two functions for the same thing. Let's keep it simple and only add ST_DISTANCE?
...rc/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StDistance.java
Show resolved
Hide resolved
...rc/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StDistance.java
Show resolved
Hide resolved
Hi @craigtaverner, I've updated the changelog YAML for you. |
...rc/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/spatial/StDistance.java
Outdated
Show resolved
Hide resolved
…expression/function/scalar/spatial/StDistance.java Co-authored-by: Ignacio Vera <[email protected]>
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
@elasticsearchmachine update branch |
We should put this into a separate PR
The work done on |
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
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.
Thanks Craig, LGTM!
@elasticsearchmachine update branch |
Initial work in support of #108212
This PR covers the addition of one new spatial function:
ST_DISTANCE(geomA, geomB)
which returns a double valueThis function can be used for calcualting distance, filtering results by distance and sorting by distance:
Distance calculations
Filtering results
Sorting results
Example
We can use it for all three capabilities in the same query:
Which results in something like:
This function was conceptually based on the similarly named PostGIS function, ST_DISTANCE, but also satisfies the needs of a related PostGIS function, ST_DWITHIN.
There are a few differences between ES|QL and PostGIS:
ST_DISTANCE
function cannot benefit directly from the spatial index, and is usually used in combination with a bounding box query to achieve higher performance results. PostGIS recommends the usage ofST_DWITHIN
for index-based distance filtering inWHERE
clauses. In ES|QL we can optimiseST_DISTANCE
for filtering, and so do not need to implementST_DWITHIN
at all.geo_point
data types we perform a haversin distance. This is because ES|QL prioritises compatibility with Elasticsearch's_search
API (and the underlying Lucene implementation) over perfect compatibility with OGC or PostGIS.Things not done in the PR and planned for followup work:
ST_DISTANCE
functions intoST_DWITHIN
functions and then push theST_DWITHIN
down to lucene, using geo_distance