Skip to content

Commit fb5d364

Browse files
iveraseiverase
authored
Optimize indexing points with index and doc values set to true (#120271)
Introducing at LonPoint and XYPoint that can add doc values sto improve indexing perfromance.
1 parent cfeeb1a commit fb5d364

File tree

11 files changed

+189
-66
lines changed

11 files changed

+189
-66
lines changed

‎docs/changelog/120271.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 120271
2+
summary: Optimize indexing points with index and doc values set to true
3+
area: Geo
4+
type: enhancement
5+
issues: []

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

Lines changed: 75 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,20 @@
88
*/
99
package org.elasticsearch.index.mapper;
1010

11+
import org.apache.lucene.document.Field;
12+
import org.apache.lucene.document.FieldType;
1113
import org.apache.lucene.document.LatLonDocValuesField;
1214
import org.apache.lucene.document.LatLonPoint;
1315
import org.apache.lucene.document.ShapeField;
1416
import org.apache.lucene.document.StoredField;
1517
import org.apache.lucene.geo.GeoEncodingUtils;
1618
import org.apache.lucene.geo.LatLonGeometry;
19+
import org.apache.lucene.index.DocValuesType;
1720
import org.apache.lucene.index.LeafReaderContext;
1821
import org.apache.lucene.search.IndexOrDocValuesQuery;
1922
import org.apache.lucene.search.Query;
23+
import org.apache.lucene.util.BytesRef;
24+
import org.apache.lucene.util.NumericUtils;
2025
import org.elasticsearch.ElasticsearchParseException;
2126
import org.elasticsearch.common.Explicit;
2227
import org.elasticsearch.common.geo.GeoFormatterFactory;
@@ -282,17 +287,24 @@ public FieldMapper.Builder getMergeBuilder() {
282287

283288
@Override
284289
protected void index(DocumentParserContext context, GeoPoint geometry) throws IOException {
285-
if (fieldType().isIndexed()) {
286-
context.doc().add(new LatLonPoint(fieldType().name(), geometry.lat(), geometry.lon()));
287-
}
288-
if (fieldType().hasDocValues()) {
290+
final boolean indexed = fieldType().isIndexed();
291+
final boolean hasDocValues = fieldType().hasDocValues();
292+
final boolean store = fieldType().isStored();
293+
if (indexed && hasDocValues) {
294+
context.doc().add(new LatLonPointWithDocValues(fieldType().name(), geometry.lat(), geometry.lon()));
295+
} else if (hasDocValues) {
289296
context.doc().add(new LatLonDocValuesField(fieldType().name(), geometry.lat(), geometry.lon()));
290-
} else if (fieldType().isStored() || fieldType().isIndexed()) {
291-
context.addToFieldNames(fieldType().name());
297+
} else if (indexed) {
298+
context.doc().add(new LatLonPoint(fieldType().name(), geometry.lat(), geometry.lon()));
292299
}
293-
if (fieldType().isStored()) {
300+
if (store) {
294301
context.doc().add(new StoredField(fieldType().name(), geometry.toString()));
295302
}
303+
if (hasDocValues == false && (indexed || store)) {
304+
// When the field doesn't have doc values so that we can run exists queries, we also need to index the field name separately.
305+
context.addToFieldNames(fieldType().name());
306+
}
307+
296308
// TODO phase out geohash (which is currently used in the CompletionSuggester)
297309
// we only expose the geohash value and disallow advancing tokens, hence we can reuse the same parser throughout multiple sub-fields
298310
DocumentParserContext parserContext = context.switchParser(new GeoHashMultiFieldParser(context.parser(), geometry.geohash()));
@@ -622,4 +634,60 @@ protected void writeValue(XContentBuilder b, long value) throws IOException {
622634

623635
return super.syntheticSourceSupport();
624636
}
637+
638+
/**
639+
* Utility class that allows adding index and doc values in one field
640+
*/
641+
public static class LatLonPointWithDocValues extends Field {
642+
643+
public static final FieldType TYPE = new FieldType();
644+
645+
static {
646+
TYPE.setDimensions(2, Integer.BYTES);
647+
TYPE.setDocValuesType(DocValuesType.SORTED_NUMERIC);
648+
TYPE.freeze();
649+
}
650+
651+
// holds the doc value value.
652+
private final long docValue;
653+
654+
public LatLonPointWithDocValues(String name, double latitude, double longitude) {
655+
super(name, TYPE);
656+
final byte[] bytes;
657+
if (fieldsData == null) {
658+
bytes = new byte[8];
659+
fieldsData = new BytesRef(bytes);
660+
} else {
661+
bytes = ((BytesRef) fieldsData).bytes;
662+
}
663+
664+
final int latitudeEncoded = GeoEncodingUtils.encodeLatitude(latitude);
665+
final int longitudeEncoded = GeoEncodingUtils.encodeLongitude(longitude);
666+
NumericUtils.intToSortableBytes(latitudeEncoded, bytes, 0);
667+
NumericUtils.intToSortableBytes(longitudeEncoded, bytes, Integer.BYTES);
668+
docValue = (((long) latitudeEncoded) << 32) | (longitudeEncoded & 0xFFFFFFFFL);
669+
}
670+
671+
@Override
672+
public Number numericValue() {
673+
return docValue;
674+
}
675+
676+
@Override
677+
public String toString() {
678+
StringBuilder result = new StringBuilder();
679+
result.append(getClass().getSimpleName());
680+
result.append(" <");
681+
result.append(name);
682+
result.append(':');
683+
684+
byte[] bytes = ((BytesRef) fieldsData).bytes;
685+
result.append(GeoEncodingUtils.decodeLatitude(bytes, 0));
686+
result.append(',');
687+
result.append(GeoEncodingUtils.decodeLongitude(bytes, Integer.BYTES));
688+
689+
result.append('>');
690+
return result.toString();
691+
}
692+
}
625693
}

‎server/src/main/java/org/elasticsearch/search/suggest/completion/context/GeoContextMapping.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ public Set<String> parseContext(LuceneDocument document) {
184184
if (field instanceof StringField) {
185185
spare.resetFromString(field.stringValue());
186186
geohashes.add(spare.geohash());
187-
} else if (field instanceof LatLonDocValuesField) {
187+
} else if (field instanceof LatLonDocValuesField || field instanceof GeoPointFieldMapper.LatLonPointWithDocValues) {
188188
spare.resetFromEncoded(field.numericValue().longValue());
189189
geohashes.add(spare.geohash());
190190
} else if (field instanceof LatLonPoint) {

‎server/src/test/java/org/elasticsearch/index/mapper/CompletionFieldMapperTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ public void testGeoHashWithSubCompletionAndStringInsert() throws Exception {
401401
ParsedDocument parsedDocument = defaultMapper.parse(source(b -> b.field("field", "drm3btev3e86")));
402402

403403
LuceneDocument indexableFields = parsedDocument.rootDoc();
404-
assertThat(indexableFields.getFields("field"), hasSize(2));
404+
assertThat(indexableFields.getFields("field"), hasSize(1));
405405
assertThat(indexableFields.getFields("field.analyzed"), containsInAnyOrder(suggestField("drm3btev3e86")));
406406
// unable to assert about geofield content, covered in a REST test
407407
}

‎server/src/test/java/org/elasticsearch/index/mapper/CopyToMapperTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -726,10 +726,10 @@ public void testCopyToGeoPoint() throws Exception {
726726
LuceneDocument doc = docMapper.parse(new SourceToParse("1", json, XContentType.JSON)).rootDoc();
727727

728728
List<IndexableField> fields = doc.getFields("geopoint");
729-
assertThat(fields.size(), equalTo(2));
729+
assertThat(fields.size(), equalTo(1));
730730

731731
fields = doc.getFields("geopoint_copy");
732-
assertThat(fields.size(), equalTo(2));
732+
assertThat(fields.size(), equalTo(1));
733733
}
734734
}
735735
// check failure for object/array type representations

‎server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java‎

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
package org.elasticsearch.index.mapper;
1111

1212
import org.apache.lucene.document.Field;
13-
import org.apache.lucene.document.LatLonDocValuesField;
14-
import org.apache.lucene.document.LatLonPoint;
1513
import org.apache.lucene.document.LongField;
1614
import org.apache.lucene.document.StringField;
1715
import org.apache.lucene.index.IndexableField;
@@ -1120,15 +1118,13 @@ public void testWithDynamicTemplates() throws Exception {
11201118

11211119
ParsedDocument doc = mapper.parse(source("1", b -> b.field(field, "41.12,-71.34"), null, Map.of(field, "points")));
11221120
List<IndexableField> fields = doc.rootDoc().getFields(field);
1123-
assertThat(fields, hasSize(2));
1124-
assertThat(fields.get(0).fieldType(), sameInstance(LatLonPoint.TYPE));
1125-
assertThat(fields.get(1).fieldType(), sameInstance(LatLonDocValuesField.TYPE));
1121+
assertThat(fields, hasSize(1));
1122+
assertThat(fields.get(0).fieldType(), sameInstance(GeoPointFieldMapper.LatLonPointWithDocValues.TYPE));
11261123

11271124
doc = mapper.parse(source("1", b -> b.field(field, new double[] { -71.34, 41.12 }), null, Map.of(field, "points")));
11281125
fields = doc.rootDoc().getFields(field);
1129-
assertThat(fields, hasSize(2));
1130-
assertThat(fields.get(0).fieldType(), sameInstance(LatLonPoint.TYPE));
1131-
assertThat(fields.get(1).fieldType(), sameInstance(LatLonDocValuesField.TYPE));
1126+
assertThat(fields, hasSize(1));
1127+
assertThat(fields.get(0).fieldType(), sameInstance(GeoPointFieldMapper.LatLonPointWithDocValues.TYPE));
11321128

11331129
doc = mapper.parse(source("1", b -> {
11341130
b.startObject(field);
@@ -1137,16 +1133,13 @@ public void testWithDynamicTemplates() throws Exception {
11371133
b.endObject();
11381134
}, null, Map.of(field, "points")));
11391135
fields = doc.rootDoc().getFields(field);
1140-
assertThat(fields, hasSize(2));
1141-
assertThat(fields.get(0).fieldType(), sameInstance(LatLonPoint.TYPE));
1142-
assertThat(fields.get(1).fieldType(), sameInstance(LatLonDocValuesField.TYPE));
1136+
assertThat(fields, hasSize(1));
1137+
assertThat(fields.get(0).fieldType(), sameInstance(GeoPointFieldMapper.LatLonPointWithDocValues.TYPE));
11431138
doc = mapper.parse(source("1", b -> b.field(field, new String[] { "41.12,-71.34", "43,-72.34" }), null, Map.of(field, "points")));
11441139
fields = doc.rootDoc().getFields(field);
1145-
assertThat(fields, hasSize(4));
1146-
assertThat(fields.get(0).fieldType(), sameInstance(LatLonPoint.TYPE));
1147-
assertThat(fields.get(1).fieldType(), sameInstance(LatLonDocValuesField.TYPE));
1148-
assertThat(fields.get(2).fieldType(), sameInstance(LatLonPoint.TYPE));
1149-
assertThat(fields.get(3).fieldType(), sameInstance(LatLonDocValuesField.TYPE));
1140+
assertThat(fields, hasSize(2));
1141+
assertThat(fields.get(0).fieldType(), sameInstance(GeoPointFieldMapper.LatLonPointWithDocValues.TYPE));
1142+
assertThat(fields.get(1).fieldType(), sameInstance(GeoPointFieldMapper.LatLonPointWithDocValues.TYPE));
11501143

11511144
doc = mapper.parse(source("1", b -> {
11521145
b.startArray(field);
@@ -1162,21 +1155,18 @@ public void testWithDynamicTemplates() throws Exception {
11621155
b.endArray();
11631156
}, null, Map.of(field, "points")));
11641157
fields = doc.rootDoc().getFields(field);
1165-
assertThat(fields, hasSize(4));
1166-
assertThat(fields.get(0).fieldType(), sameInstance(LatLonPoint.TYPE));
1167-
assertThat(fields.get(1).fieldType(), sameInstance(LatLonDocValuesField.TYPE));
1168-
assertThat(fields.get(2).fieldType(), sameInstance(LatLonPoint.TYPE));
1169-
assertThat(fields.get(3).fieldType(), sameInstance(LatLonDocValuesField.TYPE));
1158+
assertThat(fields, hasSize(2));
1159+
assertThat(fields.get(0).fieldType(), sameInstance(GeoPointFieldMapper.LatLonPointWithDocValues.TYPE));
1160+
assertThat(fields.get(1).fieldType(), sameInstance(GeoPointFieldMapper.LatLonPointWithDocValues.TYPE));
11701161

11711162
doc = mapper.parse(source("1", b -> {
11721163
b.startObject("address");
11731164
b.field("home", "43,-72.34");
11741165
b.endObject();
11751166
}, null, Map.of("address.home", "points")));
11761167
fields = doc.rootDoc().getFields("address.home");
1177-
assertThat(fields, hasSize(2));
1178-
assertThat(fields.get(0).fieldType(), sameInstance(LatLonPoint.TYPE));
1179-
assertThat(fields.get(1).fieldType(), sameInstance(LatLonDocValuesField.TYPE));
1168+
assertThat(fields, hasSize(1));
1169+
assertThat(fields.get(0).fieldType(), sameInstance(GeoPointFieldMapper.LatLonPointWithDocValues.TYPE));
11801170
}
11811171

11821172
public void testDynamicTemplatesNotFound() throws Exception {
@@ -3003,7 +2993,7 @@ public void testSubobjectsFalseDocsWithInnerObjectMappedAsFieldThatCanParseNativ
30032993
assertNotNull(location);
30042994
assertNull(parsedDocument.rootDoc().getField("metrics.service.location.lat"));
30052995
assertNull(parsedDocument.rootDoc().getField("metrics.service.location.lon"));
3006-
assertTrue(location instanceof LatLonPoint);
2996+
assertTrue(location instanceof GeoPointFieldMapper.LatLonPointWithDocValues);
30072997
Mapper locationMapper = mapper.mappers().getMapper("metrics.service.location");
30082998
assertNotNull(locationMapper);
30092999
assertTrue(locationMapper instanceof GeoPointFieldMapper);
@@ -3108,7 +3098,10 @@ public void testSubobjectsFalseDocsWithInnerObjectThatCanBeParsedNatively() thro
31083098
"""));
31093099
assertNull(parsedDocument.rootDoc().getField("metrics.service.location.lat"));
31103100
assertNull(parsedDocument.rootDoc().getField("metrics.service.location.lon"));
3111-
assertThat(parsedDocument.rootDoc().getField("metrics.service.location"), instanceOf(LatLonPoint.class));
3101+
assertThat(
3102+
parsedDocument.rootDoc().getField("metrics.service.location"),
3103+
instanceOf(GeoPointFieldMapper.LatLonPointWithDocValues.class)
3104+
);
31123105
assertThat(mapper.mappers().getMapper("metrics.service.location"), instanceOf(GeoPointFieldMapper.class));
31133106
}
31143107

@@ -3222,7 +3215,7 @@ public void testSubobjectsFalseDocsWithGeoPointFromDynamicTemplate() throws Exce
32223215
}
32233216
"""));
32243217

3225-
assertThat(parsedDocument.rootDoc().getField("location"), instanceOf(LatLonPoint.class));
3218+
assertThat(parsedDocument.rootDoc().getField("location"), instanceOf(GeoPointFieldMapper.LatLonPointWithDocValues.class));
32263219
RootObjectMapper root = parsedDocument.dynamicMappingsUpdate().getRoot();
32273220
assertEquals(1, root.mappers.size());
32283221
assertThat(root.getMapper("location"), instanceOf(GeoPointFieldMapper.class));

‎server/src/test/java/org/elasticsearch/index/mapper/DynamicTemplatesTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -678,7 +678,7 @@ public void testTemplateWithoutMatchPredicates() throws Exception {
678678
XContentMeteringParserDecorator.NOOP
679679
)
680680
);
681-
assertThat(doc.rootDoc().getFields("foo"), hasSize(2));
681+
assertThat(doc.rootDoc().getFields("foo"), hasSize(1));
682682
assertThat(doc.rootDoc().getFields("bar"), hasSize(1));
683683
}
684684

‎server/src/test/java/org/elasticsearch/index/mapper/GeoPointFieldMapperTests.java‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import org.apache.lucene.document.LatLonDocValuesField;
1212
import org.apache.lucene.document.LatLonPoint;
1313
import org.apache.lucene.geo.GeoEncodingUtils;
14-
import org.apache.lucene.index.DocValuesType;
1514
import org.apache.lucene.index.IndexableField;
1615
import org.apache.lucene.util.BytesRef;
1716
import org.elasticsearch.cluster.metadata.IndexMetadata;
@@ -194,16 +193,16 @@ public void testMetricAndMultiValues() throws Exception {
194193
new Object[] { new Double[] { pointA.getX(), pointA.getY() }, new Double[] { pointB.getX(), pointB.getY() } },
195194
new Object[] { pointA.getY() + "," + pointA.getX(), pointB.getY() + "," + pointB.getX() },
196195
new Object[] { GeoJson.toMap(pointA), GeoJson.toMap(pointB) } };
197-
IndexableField expectedPointA = new LatLonPoint("field", pointA.getY(), pointA.getX());
198-
IndexableField expectedPointB = new LatLonPoint("field", pointB.getY(), pointB.getX());
196+
IndexableField expectedPointA = new GeoPointFieldMapper.LatLonPointWithDocValues("field", pointA.getY(), pointA.getX());
197+
IndexableField expectedPointB = new GeoPointFieldMapper.LatLonPointWithDocValues("field", pointB.getY(), pointB.getX());
199198

200199
// Verify that metric and non-metric mappers behave the same on single valued fields
201200
for (Object[] values : data) {
202201
for (DocumentMapper mapper : new DocumentMapper[] { nonMetricMapper, metricMapper }) {
203202
ParsedDocument doc = mapper.parse(source(b -> b.field("field", values[0])));
204203
assertThat(doc.rootDoc().getField("field"), notNullValue());
205204
IndexableField field = doc.rootDoc().getField("field");
206-
assertThat(field, instanceOf(LatLonPoint.class));
205+
assertThat(field, instanceOf(GeoPointFieldMapper.LatLonPointWithDocValues.class));
207206
assertThat(field.toString(), equalTo(expectedPointA.toString()));
208207
}
209208
}
@@ -214,15 +213,11 @@ public void testMetricAndMultiValues() throws Exception {
214213
{
215214
ParsedDocument doc = nonMetricMapper.parse(source(b -> b.field("field", values)));
216215
assertThat(doc.rootDoc().getField("field"), notNullValue());
217-
Object[] fields = doc.rootDoc()
218-
.getFields()
219-
.stream()
220-
.filter(f -> f.name().equals("field") && f.fieldType().docValuesType() == DocValuesType.NONE)
221-
.toArray();
216+
Object[] fields = doc.rootDoc().getFields().stream().filter(f -> f.name().equals("field")).toArray();
222217
assertThat(fields.length, equalTo(2));
223-
assertThat(fields[0], instanceOf(LatLonPoint.class));
218+
assertThat(fields[0], instanceOf(GeoPointFieldMapper.LatLonPointWithDocValues.class));
224219
assertThat(fields[0].toString(), equalTo(expectedPointA.toString()));
225-
assertThat(fields[1], instanceOf(LatLonPoint.class));
220+
assertThat(fields[1], instanceOf(GeoPointFieldMapper.LatLonPointWithDocValues.class));
226221
assertThat(fields[1].toString(), equalTo(expectedPointB.toString()));
227222
}
228223
// Metric mapper rejects multi-valued data
@@ -328,7 +323,7 @@ public void testLonLatArrayDynamic() throws Exception {
328323
public void testLonLatArrayStored() throws Exception {
329324
DocumentMapper mapper = createDocumentMapper(fieldMapping(b -> b.field("type", "geo_point").field("store", true)));
330325
ParsedDocument doc = mapper.parse(source(b -> b.startArray("field").value(1.3).value(1.2).endArray()));
331-
assertThat(doc.rootDoc().getFields("field"), hasSize(3));
326+
assertThat(doc.rootDoc().getFields("field"), hasSize(2));
332327
}
333328

334329
public void testLonLatArrayArrayStored() throws Exception {

‎server/src/test/java/org/elasticsearch/index/mapper/GeoPointScriptMapperTests.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,9 @@ protected GeoPointFieldScript.Factory multipleValuesScript() {
7474

7575
@Override
7676
protected void assertMultipleValues(List<IndexableField> fields) {
77-
assertEquals(4, fields.size());
78-
assertEquals("LatLonPoint <field:-1.000000024214387,0.9999999403953552>", fields.get(0).toString());
79-
assertEquals("LatLonDocValuesField <field:-1.000000024214387,0.9999999403953552>", fields.get(1).toString());
80-
assertEquals("LatLonPoint <field:-2.000000006519258,1.9999999646097422>", fields.get(2).toString());
81-
assertEquals("LatLonDocValuesField <field:-2.000000006519258,1.9999999646097422>", fields.get(3).toString());
77+
assertEquals(2, fields.size());
78+
assertEquals("LatLonPointWithDocValues <field:-1.000000024214387,0.9999999403953552>", fields.get(0).toString());
79+
assertEquals("LatLonPointWithDocValues <field:-2.000000006519258,1.9999999646097422>", fields.get(1).toString());
8280
}
8381

8482
@Override

0 commit comments

Comments
 (0)