Skip to content

Commit f3ccde6

Browse files
authored
Use FallbackSyntheticSourceBlockLoader for point and geo_point (elastic#125816)
1 parent 7b46621 commit f3ccde6

27 files changed

+859
-87
lines changed

docs/changelog/125816.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 125816
2+
summary: Use `FallbackSyntheticSourceBlockLoader` for point and `geo_point`
3+
area: Mapping
4+
type: enhancement
5+
issues: []

server/src/main/java/org/elasticsearch/index/mapper/AbstractPointGeometryFieldMapper.java

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import java.util.function.Function;
2323
import java.util.function.Supplier;
2424

25-
import static org.elasticsearch.index.mapper.MappedFieldType.FieldExtractPreference.DOC_VALUES;
26-
2725
/** Base class for spatial fields that only support indexing points */
2826
public abstract class AbstractPointGeometryFieldMapper<T> extends AbstractGeometryFieldMapper<T> {
2927

@@ -148,7 +146,6 @@ protected void parseAndConsumeFromObject(
148146
}
149147

150148
public abstract static class AbstractPointFieldType<T extends SpatialPoint> extends AbstractGeometryFieldType<T> {
151-
152149
protected AbstractPointFieldType(
153150
String name,
154151
boolean indexed,
@@ -165,13 +162,5 @@ protected AbstractPointFieldType(
165162
protected Object nullValueAsSource(T nullValue) {
166163
return nullValue == null ? null : nullValue.toWKT();
167164
}
168-
169-
@Override
170-
public BlockLoader blockLoader(BlockLoaderContext blContext) {
171-
if (blContext.fieldExtractPreference() == DOC_VALUES && hasDocValues()) {
172-
return new BlockDocValuesReader.LongsBlockLoader(name());
173-
}
174-
return blockLoaderFromSource(blContext);
175-
}
176165
}
177166
}

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

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@
6767
import java.util.Set;
6868
import java.util.function.Function;
6969

70+
import static org.elasticsearch.index.mapper.MappedFieldType.FieldExtractPreference.DOC_VALUES;
71+
7072
/**
7173
* Field Mapper for geo_point types.
7274
*
@@ -224,7 +226,8 @@ public FieldMapper build(MapperBuilderContext context) {
224226
scriptValues(),
225227
meta.get(),
226228
metric.get(),
227-
indexMode
229+
indexMode,
230+
context.isSourceSynthetic()
228231
);
229232
hasScript = script.get() != null;
230233
onScriptError = onScriptErrorParam.get();
@@ -370,6 +373,7 @@ public static class GeoPointFieldType extends AbstractPointFieldType<GeoPoint> i
370373

371374
private final FieldValues<GeoPoint> scriptValues;
372375
private final IndexMode indexMode;
376+
private final boolean isSyntheticSource;
373377

374378
private GeoPointFieldType(
375379
String name,
@@ -381,17 +385,19 @@ private GeoPointFieldType(
381385
FieldValues<GeoPoint> scriptValues,
382386
Map<String, String> meta,
383387
TimeSeriesParams.MetricType metricType,
384-
IndexMode indexMode
388+
IndexMode indexMode,
389+
boolean isSyntheticSource
385390
) {
386391
super(name, indexed, stored, hasDocValues, parser, nullValue, meta);
387392
this.scriptValues = scriptValues;
388393
this.metricType = metricType;
389394
this.indexMode = indexMode;
395+
this.isSyntheticSource = isSyntheticSource;
390396
}
391397

392398
// only used in test
393399
public GeoPointFieldType(String name, TimeSeriesParams.MetricType metricType, IndexMode indexMode) {
394-
this(name, true, false, true, null, null, null, Collections.emptyMap(), metricType, indexMode);
400+
this(name, true, false, true, null, null, null, Collections.emptyMap(), metricType, indexMode, false);
395401
}
396402

397403
// only used in test
@@ -524,6 +530,28 @@ public Query distanceFeatureQuery(Object origin, String pivot, SearchExecutionCo
524530
public TimeSeriesParams.MetricType getMetricType() {
525531
return metricType;
526532
}
533+
534+
@Override
535+
public BlockLoader blockLoader(BlockLoaderContext blContext) {
536+
if (blContext.fieldExtractPreference() == DOC_VALUES && hasDocValues()) {
537+
return new BlockDocValuesReader.LongsBlockLoader(name());
538+
}
539+
540+
// There are two scenarios possible once we arrive here:
541+
//
542+
// * Stored source - we'll just use blockLoaderFromSource
543+
// * Synthetic source. However, because of the fieldExtractPreference() check above it is still possible that doc_values are
544+
// present here.
545+
// So we have two subcases:
546+
// - doc_values are enabled - _ignored_source field does not exist since we have doc_values. We will use
547+
// 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) {
550+
return blockLoaderFromFallbackSyntheticSource(blContext);
551+
}
552+
553+
return blockLoaderFromSource(blContext);
554+
}
527555
}
528556

529557
/** GeoPoint parser implementation */

server/src/test/java/org/elasticsearch/index/mapper/blockloader/BooleanFieldBlockLoaderTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ public BooleanFieldBlockLoaderTests(Params params) {
2323

2424
@Override
2525
@SuppressWarnings("unchecked")
26-
protected Object expected(Map<String, Object> fieldMapping, Object value) {
26+
protected Object expected(Map<String, Object> fieldMapping, Object value, TestContext testContext) {
2727
var nullValue = switch (fieldMapping.get("null_value")) {
2828
case Boolean b -> b;
2929
case String s -> Boolean.parseBoolean(s);

server/src/test/java/org/elasticsearch/index/mapper/blockloader/DateFieldBlockLoaderTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public DateFieldBlockLoaderTests(Params params) {
2929

3030
@Override
3131
@SuppressWarnings("unchecked")
32-
protected Object expected(Map<String, Object> fieldMapping, Object value) {
32+
protected Object expected(Map<String, Object> fieldMapping, Object value, TestContext testContext) {
3333
var format = (String) fieldMapping.get("format");
3434
var nullValue = fieldMapping.get("null_value") != null ? format(fieldMapping.get("null_value"), format) : null;
3535

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.index.mapper.blockloader;
11+
12+
import org.apache.lucene.util.BytesRef;
13+
import org.elasticsearch.common.geo.GeoPoint;
14+
import org.elasticsearch.geometry.Point;
15+
import org.elasticsearch.geometry.utils.WellKnownBinary;
16+
import org.elasticsearch.index.mapper.BlockLoaderTestCase;
17+
import org.elasticsearch.index.mapper.MappedFieldType;
18+
19+
import java.nio.ByteOrder;
20+
import java.util.ArrayList;
21+
import java.util.Comparator;
22+
import java.util.List;
23+
import java.util.Map;
24+
import java.util.Objects;
25+
26+
public class GeoPointFieldBlockLoaderTests extends BlockLoaderTestCase {
27+
public GeoPointFieldBlockLoaderTests(BlockLoaderTestCase.Params params) {
28+
super("geo_point", params);
29+
}
30+
31+
@Override
32+
@SuppressWarnings("unchecked")
33+
protected Object expected(Map<String, Object> fieldMapping, Object value, TestContext testContext) {
34+
var extractedFieldValues = (ExtractedFieldValues) value;
35+
var values = extractedFieldValues.values();
36+
37+
var nullValue = switch (fieldMapping.get("null_value")) {
38+
case String s -> convert(s, null);
39+
case null -> null;
40+
default -> throw new IllegalStateException("Unexpected null_value format");
41+
};
42+
43+
if (params.preference() == MappedFieldType.FieldExtractPreference.DOC_VALUES && hasDocValues(fieldMapping, true)) {
44+
if (values instanceof List<?> == false) {
45+
var point = convert(values, nullValue);
46+
return point != null ? point.getEncoded() : null;
47+
}
48+
49+
var resultList = ((List<Object>) values).stream()
50+
.map(v -> convert(v, nullValue))
51+
.filter(Objects::nonNull)
52+
.map(GeoPoint::getEncoded)
53+
.sorted()
54+
.toList();
55+
return maybeFoldList(resultList);
56+
}
57+
58+
if (params.syntheticSource() == false) {
59+
return exactValuesFromSource(values, nullValue);
60+
}
61+
62+
// Usually implementation of block loader from source adjusts values read from source
63+
// so that they look the same as doc_values would (like reducing precision).
64+
// geo_point does not do that and because of that we need to handle all these cases below.
65+
// If we are reading from stored source or fallback synthetic source we get the same exact data as source.
66+
// But if we are using "normal" synthetic source we get lesser precision data from doc_values.
67+
// That is unless "synthetic_source_keep" forces fallback synthetic source again.
68+
69+
if (testContext.forceFallbackSyntheticSource()) {
70+
return exactValuesFromSource(values, nullValue);
71+
}
72+
73+
String syntheticSourceKeep = (String) fieldMapping.getOrDefault("synthetic_source_keep", "none");
74+
if (syntheticSourceKeep.equals("all")) {
75+
return exactValuesFromSource(values, nullValue);
76+
}
77+
if (syntheticSourceKeep.equals("arrays") && extractedFieldValues.documentHasObjectArrays()) {
78+
return exactValuesFromSource(values, nullValue);
79+
}
80+
81+
// synthetic source and doc_values are present
82+
if (hasDocValues(fieldMapping, true)) {
83+
if (values instanceof List<?> == false) {
84+
return toWKB(normalize(convert(values, nullValue)));
85+
}
86+
87+
var resultList = ((List<Object>) values).stream()
88+
.map(v -> convert(v, nullValue))
89+
.filter(Objects::nonNull)
90+
.sorted(Comparator.comparingLong(GeoPoint::getEncoded))
91+
.map(p -> toWKB(normalize(p)))
92+
.toList();
93+
return maybeFoldList(resultList);
94+
}
95+
96+
// synthetic source but no doc_values so using fallback synthetic source
97+
return exactValuesFromSource(values, nullValue);
98+
}
99+
100+
@SuppressWarnings("unchecked")
101+
private Object exactValuesFromSource(Object value, GeoPoint nullValue) {
102+
if (value instanceof List<?> == false) {
103+
return toWKB(convert(value, nullValue));
104+
}
105+
106+
var resultList = ((List<Object>) value).stream().map(v -> convert(v, nullValue)).filter(Objects::nonNull).map(this::toWKB).toList();
107+
return maybeFoldList(resultList);
108+
}
109+
110+
private record ExtractedFieldValues(Object values, boolean documentHasObjectArrays) {}
111+
112+
@Override
113+
protected Object getFieldValue(Map<String, Object> document, String fieldName) {
114+
var extracted = new ArrayList<>();
115+
var documentHasObjectArrays = processLevel(document, fieldName, extracted, false);
116+
117+
if (extracted.size() == 1) {
118+
return new ExtractedFieldValues(extracted.get(0), documentHasObjectArrays);
119+
}
120+
121+
return new ExtractedFieldValues(extracted, documentHasObjectArrays);
122+
}
123+
124+
@SuppressWarnings("unchecked")
125+
private boolean processLevel(Map<String, Object> level, String field, ArrayList<Object> extracted, boolean documentHasObjectArrays) {
126+
if (field.contains(".") == false) {
127+
var value = level.get(field);
128+
processLeafLevel(value, extracted);
129+
return documentHasObjectArrays;
130+
}
131+
132+
var nameInLevel = field.split("\\.")[0];
133+
var entry = level.get(nameInLevel);
134+
if (entry instanceof Map<?, ?> m) {
135+
return processLevel((Map<String, Object>) m, field.substring(field.indexOf('.') + 1), extracted, documentHasObjectArrays);
136+
}
137+
if (entry instanceof List<?> l) {
138+
for (var object : l) {
139+
processLevel((Map<String, Object>) object, field.substring(field.indexOf('.') + 1), extracted, true);
140+
}
141+
return true;
142+
}
143+
144+
assert false : "unexpected document structure";
145+
return false;
146+
}
147+
148+
private void processLeafLevel(Object value, ArrayList<Object> extracted) {
149+
if (value instanceof List<?> l) {
150+
if (l.size() > 0 && l.get(0) instanceof Double) {
151+
// this must be a single point in array form
152+
// we'll put it into a different form here to make our lives a bit easier while implementing `expected`
153+
extracted.add(Map.of("type", "point", "coordinates", l));
154+
} else {
155+
// this is actually an array of points but there could still be points in array form inside
156+
for (var arrayValue : l) {
157+
processLeafLevel(arrayValue, extracted);
158+
}
159+
}
160+
} else {
161+
extracted.add(value);
162+
}
163+
}
164+
165+
@SuppressWarnings("unchecked")
166+
private GeoPoint convert(Object value, GeoPoint nullValue) {
167+
if (value == null) {
168+
return nullValue;
169+
}
170+
171+
if (value instanceof String s) {
172+
try {
173+
return new GeoPoint(s);
174+
} catch (Exception e) {
175+
return null;
176+
}
177+
}
178+
179+
if (value instanceof Map<?, ?> m) {
180+
if (m.get("type") != null) {
181+
var coordinates = (List<Double>) m.get("coordinates");
182+
// Order is GeoJSON is lon,lat
183+
return new GeoPoint(coordinates.get(1), coordinates.get(0));
184+
} else {
185+
return new GeoPoint((Double) m.get("lat"), (Double) m.get("lon"));
186+
}
187+
}
188+
189+
// Malformed values are excluded
190+
return null;
191+
}
192+
193+
private GeoPoint normalize(GeoPoint point) {
194+
if (point == null) {
195+
return null;
196+
}
197+
return point.resetFromEncoded(point.getEncoded());
198+
}
199+
200+
private BytesRef toWKB(GeoPoint point) {
201+
if (point == null) {
202+
return null;
203+
}
204+
205+
return new BytesRef(WellKnownBinary.toWKB(new Point(point.getX(), point.getY()), ByteOrder.LITTLE_ENDIAN));
206+
}
207+
}

server/src/test/java/org/elasticsearch/index/mapper/blockloader/KeywordFieldBlockLoaderTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public KeywordFieldBlockLoaderTests(Params params) {
2727

2828
@SuppressWarnings("unchecked")
2929
@Override
30-
protected Object expected(Map<String, Object> fieldMapping, Object value) {
30+
protected Object expected(Map<String, Object> fieldMapping, Object value, TestContext testContext) {
3131
var nullValue = (String) fieldMapping.get("null_value");
3232

3333
var ignoreAbove = fieldMapping.get("ignore_above") == null

0 commit comments

Comments
 (0)