Allow doc-values only search on geo_point fields#83395
Allow doc-values only search on geo_point fields#83395ywelsch merged 8 commits intoelastic:masterfrom
Conversation
|
Hi @ywelsch, I've created a changelog YAML for you. |
|
Pinging @elastic/es-search (Team:Search) |
romseygeek
left a comment
There was a problem hiding this comment.
LGTM, thanks @ywelsch
server/src/main/java/org/elasticsearch/script/field/SortedNumericDocValuesLongFieldScript.java
Show resolved
Hide resolved
|
Thank you @romseygeek! |
|
|
||
| @Override | ||
| public Query distanceFeatureQuery(Object origin, String pivot, SearchExecutionContext context) { | ||
| failIfNotIndexedNorDocValuesFallback(context); |
There was a problem hiding this comment.
were we silently not returning any result when the field was not indexed? Or failing later with some other exception?
There was a problem hiding this comment.
I think we were silently not returning any result...
We have this kind of bug in some other places unfortunately as well
| return LongPoint.newDistanceFeatureQuery(name(), 1.0f, originLong, pivotLong); | ||
| } else { | ||
| return new LongScriptFieldDistanceFeatureQuery( | ||
| new Script(""), |
There was a problem hiding this comment.
this makes me a bit nervous. We use the original script only in equals/hashcode, and we use the empty script already for script-less runtime fields that load from _source: https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/index/mapper/AbstractScriptFieldType.java#L212 . I am not entirely sure what this may cause down the line.
There was a problem hiding this comment.
The script is actually used nowhere. The reason it's there is because AbstractScriptFieldQuery requires a non-null script. I think we could also make it nullable there and make it clear to callers that a script is optional. I don't see your worry though, how this would interact badly in the future.
There was a problem hiding this comment.
Luca is right, it's used in equals/hashcode which is further used by the query cache. So if we're not careful here we could end up with a source-only field and a doc-values field stepping on each other's cache values. I don't think that would actually be a problem here because the source-only field for this fieldname should return the same values as the doc-values field (we also check the query class in equals) but I need to think harder about it
There was a problem hiding this comment.
ah, thanks for pointing that out @romseygeek. I wasn't aware of this dependency.
Similar to #82409, but for geo_point fields.
Allows searching on geo_point fields when those fields are not indexed (index: false) but just doc values are enabled.
Also adds distance feature query support for date fields (bringing date field to feature parity with runtime fields)
This enables searches on archive data, which has access to doc values but not index structures. When combined with searchable snapshots, it allows downloading only data for a given (doc value) field to quickly filter down to a select set of documents.
Relates #81210 and #52728