Skip to content

Commit 4e0978e

Browse files
prdoyleprdoyle
authored
Merge branch 'main' into health
2 parents ce015e7 + 4494fdc commit 4e0978e

File tree

30 files changed

+636
-307
lines changed

30 files changed

+636
-307
lines changed

‎docs/changelog/127532.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 127532
2+
summary: Fix case insensitive comparisons to ""
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 127431

‎modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
378378
if (hasDocValues() && (blContext.fieldExtractPreference() != FieldExtractPreference.STORED || isSyntheticSource)) {
379379
return new BlockDocValuesReader.DoublesBlockLoader(name(), l -> l / scalingFactor);
380380
}
381-
if (isSyntheticSource) {
381+
// Multi fields don't have fallback synthetic source.
382+
if (isSyntheticSource && blContext.parentField(name()) == null) {
382383
return new FallbackSyntheticSourceBlockLoader(fallbackSyntheticSourceBlockLoaderReader(), name()) {
383384
@Override
384385
public Builder builder(BlockFactory factory, int expectedCount) {

‎muted-tests.yml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -429,9 +429,6 @@ tests:
429429
- class: org.elasticsearch.xpack.esql.heap_attack.HeapAttackIT
430430
method: testLookupExplosionNoFetch
431431
issue: https://github.com/elastic/elasticsearch/issues/127365
432-
- class: org.elasticsearch.xpack.esql.qa.single_node.PushQueriesIT
433-
method: testPushCaseInsensitiveEqualityOnDefaults
434-
issue: https://github.com/elastic/elasticsearch/issues/127431
435432
- class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeIT
436433
method: test
437434
issue: https://github.com/elastic/elasticsearch/issues/127536

‎server/src/main/java/org/elasticsearch/index/mapper/BooleanFieldMapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
350350
return new BlockDocValuesReader.BooleansBlockLoader(name());
351351
}
352352

353-
if (isSyntheticSource) {
353+
// Multi fields don't have fallback synthetic source.
354+
if (isSyntheticSource && blContext.parentField(name()) == null) {
354355
return new FallbackSyntheticSourceBlockLoader(fallbackSyntheticSourceBlockLoaderReader(), name()) {
355356
@Override
356357
public Builder builder(BlockFactory factory, int expectedCount) {

‎server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -948,7 +948,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
948948
return new BlockDocValuesReader.LongsBlockLoader(name());
949949
}
950950

951-
if (isSyntheticSource) {
951+
// Multi fields don't have fallback synthetic source.
952+
if (isSyntheticSource && blContext.parentField(name()) == null) {
952953
return new FallbackSyntheticSourceBlockLoader(fallbackSyntheticSourceBlockLoaderReader(), name()) {
953954
@Override
954955
public Builder builder(BlockFactory factory, int expectedCount) {

‎server/src/main/java/org/elasticsearch/index/mapper/GeoPointFieldMapper.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ protected void index(DocumentParserContext context, GeoPoint geometry) throws IO
316316

317317
/**
318318
* Parser that pretends to be the main document parser, but exposes the provided geohash regardless of how the geopoint was provided
319-
* in the incoming document. We rely on the fact that consumers are only ever call {@link XContentParser#textOrNull()} and never
319+
* in the incoming document. We rely on the fact that consumers only ever read text from the parser and never
320320
* advance tokens, which is explicitly disallowed by this parser.
321321
*/
322322
static class GeoHashMultiFieldParser extends FilterXContentParserWrapper {
@@ -332,6 +332,11 @@ public String textOrNull() throws IOException {
332332
return value;
333333
}
334334

335+
@Override
336+
public String text() throws IOException {
337+
return value;
338+
}
339+
335340
@Override
336341
public Token currentToken() {
337342
return Token.VALUE_STRING;
@@ -545,8 +550,9 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
545550
// So we have two subcases:
546551
// - doc_values are enabled - _ignored_source field does not exist since we have doc_values. We will use
547552
// blockLoaderFromSource which reads "native" synthetic source.
548-
// - doc_values are disabled - we know that _ignored_source field is present and use a special block loader.
549-
if (isSyntheticSource && hasDocValues() == false) {
553+
// - doc_values are disabled - we know that _ignored_source field is present and use a special block loader unless it's a multi
554+
// field.
555+
if (isSyntheticSource && hasDocValues() == false && blContext.parentField(name()) == null) {
550556
return blockLoaderFromFallbackSyntheticSource(blContext);
551557
}
552558

‎server/src/main/java/org/elasticsearch/index/mapper/IpFieldMapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
467467
return new BlockStoredFieldsReader.BytesFromBytesRefsBlockLoader(name());
468468
}
469469

470-
if (isSyntheticSource) {
470+
// Multi fields don't have fallback synthetic source.
471+
if (isSyntheticSource && blContext.parentField(name()) == null) {
471472
return blockLoaderFromFallbackSyntheticSource(blContext);
472473
}
473474

‎server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -755,7 +755,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
755755
return new BlockStoredFieldsReader.BytesFromBytesRefsBlockLoader(name());
756756
}
757757

758-
if (isSyntheticSource) {
758+
// Multi fields don't have fallback synthetic source.
759+
if (isSyntheticSource && blContext.parentField(name()) == null) {
759760
return new FallbackSyntheticSourceBlockLoader(fallbackSyntheticSourceBlockLoaderReader(), name()) {
760761
@Override
761762
public Builder builder(BlockFactory factory, int expectedCount) {

‎server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1973,7 +1973,8 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) {
19731973
return type.blockLoaderFromDocValues(name());
19741974
}
19751975

1976-
if (isSyntheticSource) {
1976+
// Multi fields don't have fallback synthetic source.
1977+
if (isSyntheticSource && blContext.parentField(name()) == null) {
19771978
return type.blockLoaderFromFallbackSyntheticSource(name(), nullValue, coerce);
19781979
}
19791980

‎server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java‎

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,28 +35,29 @@ protected Object expected(Map<String, Object> fieldMapping, Object value, TestCo
3535
var values = extractedFieldValues.values();
3636

3737
var nullValue = switch (fieldMapping.get("null_value")) {
38-
case String s -> convert(s, null);
38+
case String s -> convert(s, null, false);
3939
case null -> null;
4040
default -> throw new IllegalStateException("Unexpected null_value format");
4141
};
4242

4343
if (params.preference() == MappedFieldType.FieldExtractPreference.DOC_VALUES && hasDocValues(fieldMapping, true)) {
4444
if (values instanceof List<?> == false) {
45-
var point = convert(values, nullValue);
45+
var point = convert(values, nullValue, testContext.isMultifield());
4646
return point != null ? point.getEncoded() : null;
4747
}
4848

4949
var resultList = ((List<Object>) values).stream()
50-
.map(v -> convert(v, nullValue))
50+
.map(v -> convert(v, nullValue, testContext.isMultifield()))
5151
.filter(Objects::nonNull)
5252
.map(GeoPoint::getEncoded)
5353
.sorted()
5454
.toList();
5555
return maybeFoldList(resultList);
5656
}
5757

58+
// stored source is used
5859
if (params.syntheticSource() == false) {
59-
return exactValuesFromSource(values, nullValue);
60+
return exactValuesFromSource(values, nullValue, false);
6061
}
6162

6263
// Usually implementation of block loader from source adjusts values read from source
@@ -67,25 +68,25 @@ protected Object expected(Map<String, Object> fieldMapping, Object value, TestCo
6768
// That is unless "synthetic_source_keep" forces fallback synthetic source again.
6869

6970
if (testContext.forceFallbackSyntheticSource()) {
70-
return exactValuesFromSource(values, nullValue);
71+
return exactValuesFromSource(values, nullValue, false);
7172
}
7273

7374
String syntheticSourceKeep = (String) fieldMapping.getOrDefault("synthetic_source_keep", "none");
7475
if (syntheticSourceKeep.equals("all")) {
75-
return exactValuesFromSource(values, nullValue);
76+
return exactValuesFromSource(values, nullValue, false);
7677
}
7778
if (syntheticSourceKeep.equals("arrays") && extractedFieldValues.documentHasObjectArrays()) {
78-
return exactValuesFromSource(values, nullValue);
79+
return exactValuesFromSource(values, nullValue, false);
7980
}
8081

8182
// synthetic source and doc_values are present
8283
if (hasDocValues(fieldMapping, true)) {
8384
if (values instanceof List<?> == false) {
84-
return toWKB(normalize(convert(values, nullValue)));
85+
return toWKB(normalize(convert(values, nullValue, false)));
8586
}
8687

8788
var resultList = ((List<Object>) values).stream()
88-
.map(v -> convert(v, nullValue))
89+
.map(v -> convert(v, nullValue, false))
8990
.filter(Objects::nonNull)
9091
.sorted(Comparator.comparingLong(GeoPoint::getEncoded))
9192
.map(p -> toWKB(normalize(p)))
@@ -94,16 +95,20 @@ protected Object expected(Map<String, Object> fieldMapping, Object value, TestCo
9495
}
9596

9697
// synthetic source but no doc_values so using fallback synthetic source
97-
return exactValuesFromSource(values, nullValue);
98+
return exactValuesFromSource(values, nullValue, false);
9899
}
99100

100101
@SuppressWarnings("unchecked")
101-
private Object exactValuesFromSource(Object value, GeoPoint nullValue) {
102+
private Object exactValuesFromSource(Object value, GeoPoint nullValue, boolean needsMultifieldAdjustment) {
102103
if (value instanceof List<?> == false) {
103-
return toWKB(convert(value, nullValue));
104+
return toWKB(convert(value, nullValue, needsMultifieldAdjustment));
104105
}
105106

106-
var resultList = ((List<Object>) value).stream().map(v -> convert(v, nullValue)).filter(Objects::nonNull).map(this::toWKB).toList();
107+
var resultList = ((List<Object>) value).stream()
108+
.map(v -> convert(v, nullValue, needsMultifieldAdjustment))
109+
.filter(Objects::nonNull)
110+
.map(this::toWKB)
111+
.toList();
107112
return maybeFoldList(resultList);
108113
}
109114

@@ -163,14 +168,17 @@ private void processLeafLevel(Object value, ArrayList<Object> extracted) {
163168
}
164169

165170
@SuppressWarnings("unchecked")
166-
private GeoPoint convert(Object value, GeoPoint nullValue) {
171+
private GeoPoint convert(Object value, GeoPoint nullValue, boolean needsMultifieldAdjustment) {
167172
if (value == null) {
168-
return nullValue;
173+
if (nullValue == null) {
174+
return null;
175+
}
176+
return possiblyAdjustMultifieldValue(nullValue, needsMultifieldAdjustment);
169177
}
170178

171179
if (value instanceof String s) {
172180
try {
173-
return new GeoPoint(s);
181+
return possiblyAdjustMultifieldValue(new GeoPoint(s), needsMultifieldAdjustment);
174182
} catch (Exception e) {
175183
return null;
176184
}
@@ -180,16 +188,30 @@ private GeoPoint convert(Object value, GeoPoint nullValue) {
180188
if (m.get("type") != null) {
181189
var coordinates = (List<Double>) m.get("coordinates");
182190
// Order is GeoJSON is lon,lat
183-
return new GeoPoint(coordinates.get(1), coordinates.get(0));
191+
return possiblyAdjustMultifieldValue(new GeoPoint(coordinates.get(1), coordinates.get(0)), needsMultifieldAdjustment);
184192
} else {
185-
return new GeoPoint((Double) m.get("lat"), (Double) m.get("lon"));
193+
return possiblyAdjustMultifieldValue(new GeoPoint((Double) m.get("lat"), (Double) m.get("lon")), needsMultifieldAdjustment);
186194
}
187195
}
188196

189197
// Malformed values are excluded
190198
return null;
191199
}
192200

201+
private GeoPoint possiblyAdjustMultifieldValue(GeoPoint point, boolean isMultifield) {
202+
// geo_point multifields are parsed from a geohash representation of the original point (GeoPointFieldMapper#index)
203+
// and it's not exact.
204+
// So if this is a multifield we need another adjustment here.
205+
// Note that this does not apply to block loader from source because in this case we parse raw original values.
206+
// Same thing happens with synthetic source since it is generated from the parent field data that didn't go through multi field
207+
// parsing logic.
208+
if (isMultifield) {
209+
return point.resetFromString(point.geohash());
210+
}
211+
212+
return point;
213+
}
214+
193215
private GeoPoint normalize(GeoPoint point) {
194216
if (point == null) {
195217
return null;

0 commit comments

Comments
 (0)