-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Support geo label position as runtime field #86154
Support geo label position as runtime field #86154
Conversation
Hi @craigtaverner, I've created a changelog YAML for you. |
@elasticmachine run elasticsearch-ci/part-2 |
Pinging @elastic/clients-team (Team:Clients) |
server/src/main/java/org/elasticsearch/script/field/GeoPointDocValuesField.java
Outdated
Show resolved
Hide resolved
We simplified this to always use the middle of the first tree geometry. The use of the centroid can be done higher up in the stack. For painless, this means in the user written script. For the REST API we will add that logic on the server side.
Now that we no longer use the centroid for polygon labels, but the middle of the first triangle.
This time without rounding error
And more tests, including testing intersects for all label positions.
bfcd82b
to
57c977d
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.
Just have a small comment about how we handle an error. Otherwise let's get CI green and it can be approved.
...main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java
Show resolved
Hide resolved
4ae19a0
to
ccf30cf
Compare
The centroid is unlikely to be one of the points, so we choose a point closest to the middle of the bounding box.
The changes to expose GeoShapeQueryable.toQuantizeLuceneGeometry to external use also caused exception causes to be one level deeper. This fix just brings the underlying message up one level to get tests to pass. While we could have also changed tests to assert on causes a level deeper, this fix seems more backwards compatible, just in case anyone else is relying on causes.
ccf30cf
to
74dc54b
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.
LGTM
Thank you Craig!
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.
Scripting looks good.
There is a need to provide sensibly calculated label positions for polygons and lines in Kibana maps (as requested in #86044). A very convenient way of satisfy this need is through a runtime field that the rest API can make use of when labels are requested. This has the advantage of providing painless access to the label position as well. This PR provides the runtime field.