Allow doc-values only search on geo_point fields #83395
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| pr: 83395 | ||
| summary: Date script doc values | ||
| area: Mapping | ||
| type: enhancement | ||
| issues: [] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,10 +42,12 @@ | |
| import org.elasticsearch.script.ScriptCompiler; | ||
| import org.elasticsearch.script.field.DateMillisDocValuesField; | ||
| import org.elasticsearch.script.field.DateNanosDocValuesField; | ||
| import org.elasticsearch.script.field.SortedNumericDocValuesLongFieldScript; | ||
| import org.elasticsearch.script.field.ToScriptField; | ||
| import org.elasticsearch.search.DocValueFormat; | ||
| import org.elasticsearch.search.lookup.FieldValues; | ||
| import org.elasticsearch.search.lookup.SearchLookup; | ||
| import org.elasticsearch.search.runtime.LongScriptFieldDistanceFeatureQuery; | ||
|
|
||
| import java.io.IOException; | ||
| import java.text.NumberFormat; | ||
|
|
@@ -86,6 +88,11 @@ public long convert(Instant instant) { | |
| return instant.toEpochMilli(); | ||
| } | ||
|
|
||
| @Override | ||
| public long convert(TimeValue timeValue) { | ||
| return timeValue.millis(); | ||
| } | ||
|
|
||
| @Override | ||
| public Instant toInstant(long value) { | ||
| return Instant.ofEpochMilli(value); | ||
|
|
@@ -105,18 +112,18 @@ public long roundDownToMillis(long value) { | |
| public long roundUpToMillis(long value) { | ||
| return value; | ||
| } | ||
| }, | ||
| NANOSECONDS(DATE_NANOS_CONTENT_TYPE, NumericType.DATE_NANOSECONDS, DateNanosDocValuesField::new) { | ||
| @Override | ||
| public long convert(Instant instant) { | ||
| return toLong(instant); | ||
| } | ||
|
|
||
| @Override | ||
| public long convert(TimeValue timeValue) { | ||
| return timeValue.nanos(); | ||
| } | ||
|
|
||
| @Override | ||
| public Instant toInstant(long value) { | ||
| return DateUtils.toInstant(value); | ||
|
|
@@ -141,11 +148,6 @@ public long roundUpToMillis(long value) { | |
| return DateUtils.toMilliSeconds(value - 1L) + 1L; | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| private final String type; | ||
|
|
@@ -180,6 +182,11 @@ ToScriptField<SortedNumericDocValues> getDefaultToScriptField() { | |
| */ | ||
| public abstract Instant toInstant(long value); | ||
|
|
||
| /** | ||
| * Convert an {@linkplain TimeValue} into a long value in this resolution. | ||
| */ | ||
| public abstract long convert(TimeValue timeValue); | ||
|
|
||
| /** | ||
| * Decode the points representation of this field as milliseconds. | ||
| */ | ||
|
|
@@ -203,8 +210,6 @@ public static Resolution ofOrdinal(int ord) { | |
| } | ||
| throw new IllegalArgumentException("unknown resolution ordinal [" + ord + "]"); | ||
| } | ||
| } | ||
|
|
||
| private static DateFieldMapper toType(FieldMapper in) { | ||
|
|
@@ -596,10 +601,22 @@ public static long parseToLong( | |
|
|
||
| @Override | ||
| public Query distanceFeatureQuery(Object origin, String pivot, SearchExecutionContext context) { | ||
| failIfNotIndexedNorDocValuesFallback(context); | ||
| long originLong = parseToLong(origin, true, null, null, context::nowInMillis); | ||
| TimeValue pivotTime = TimeValue.parseTimeValue(pivot, "distance_feature.pivot"); | ||
| long pivotLong = resolution.convert(pivotTime); | ||
| // As we already apply boost in AbstractQueryBuilder::toQuery, we always passing a boost of 1.0 to distanceFeatureQuery | ||
| if (isIndexed()) { | ||
| 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 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. |
||
| ctx -> new SortedNumericDocValuesLongFieldScript(name(), context.lookup(), ctx), | ||
| name(), | ||
| originLong, | ||
| pivotLong | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we were silently not returning any result...
We have this kind of bug in some other places unfortunately as well