diff --git a/docs/changelog/127796.yaml b/docs/changelog/127796.yaml new file mode 100644 index 0000000000000..c87e777f83d40 --- /dev/null +++ b/docs/changelog/127796.yaml @@ -0,0 +1,6 @@ +pr: 127796 +summary: Do not respect synthetic_source_keep=arrays if type parses arrays +area: Mapping +type: enhancement +issues: + - 126155 diff --git a/docs/reference/elasticsearch/mapping-reference/geo-point.md b/docs/reference/elasticsearch/mapping-reference/geo-point.md index faeada7cf15a4..c26b8ecfa5eb7 100644 --- a/docs/reference/elasticsearch/mapping-reference/geo-point.md +++ b/docs/reference/elasticsearch/mapping-reference/geo-point.md @@ -218,7 +218,9 @@ of official GA features. Synthetic source may sort `geo_point` fields (first by latitude and then -longitude) and reduces them to their stored precision. For example: +longitude) and reduces them to their stored precision. Additionally, unlike most +types, arrays of `geo_point` fields will not preserve order if +`synthetic_source_keep` is set to `arrays`. For example: $$$synthetic-source-geo-point-example$$$ @@ -236,15 +238,18 @@ PUT idx }, "mappings": { "properties": { - "point": { "type": "geo_point" } + "point": { + "type": "geo_point", + "synthetic_source_keep": "arrays" + } } } } PUT idx/_doc/1 { "point": [ - {"lat":-90, "lon":-80}, - {"lat":10, "lon":30} + {"lat":10, "lon":30}, + {"lat":-90, "lon":-80} ] } ``` diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 06dec6a090352..f877502fe8754 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -460,7 +460,7 @@ static void parseObjectOrField(DocumentParserContext context, Mapper mapper) thr if (context.canAddIgnoredField() && (fieldMapper.syntheticSourceMode() == FieldMapper.SyntheticSourceMode.FALLBACK || sourceKeepMode == Mapper.SourceKeepMode.ALL - || (sourceKeepMode == Mapper.SourceKeepMode.ARRAYS && context.inArrayScope()) + || (sourceKeepMode == Mapper.SourceKeepMode.ARRAYS && context.inArrayScope() && parsesArrayValue(mapper) == false) || (context.isWithinCopyTo() == false && context.isCopyToDestinationField(mapper.fullPath())))) { context = context.addIgnoredFieldFromContext( IgnoredSourceFieldMapper.NameValue.fromContext(context, fieldMapper.fullPath(), null) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java index 530be9fbbc738..fdb1d89175fef 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/blockloader/GeoPointFieldBlockLoaderTests.java @@ -17,7 +17,6 @@ import org.elasticsearch.index.mapper.MappedFieldType; import java.nio.ByteOrder; -import java.util.ArrayList; import java.util.Comparator; import java.util.List; import java.util.Map; @@ -30,10 +29,7 @@ public GeoPointFieldBlockLoaderTests(BlockLoaderTestCase.Params params) { @Override @SuppressWarnings("unchecked") - protected Object expected(Map fieldMapping, Object value, TestContext testContext) { - var extractedFieldValues = (ExtractedFieldValues) value; - var values = extractedFieldValues.values(); - + protected Object expected(Map fieldMapping, Object values, TestContext testContext) { var nullValue = switch (fieldMapping.get("null_value")) { case String s -> convert(s, null, false); case null -> null; @@ -75,9 +71,6 @@ protected Object expected(Map fieldMapping, Object value, TestCo if (syntheticSourceKeep.equals("all")) { return exactValuesFromSource(values, nullValue, false); } - if (syntheticSourceKeep.equals("arrays") && extractedFieldValues.documentHasObjectArrays()) { - return exactValuesFromSource(values, nullValue, false); - } // synthetic source and doc_values are present if (hasDocValues(fieldMapping, true)) { @@ -112,61 +105,6 @@ private Object exactValuesFromSource(Object value, GeoPoint nullValue, boolean n return maybeFoldList(resultList); } - private record ExtractedFieldValues(Object values, boolean documentHasObjectArrays) {} - - @Override - protected Object getFieldValue(Map document, String fieldName) { - var extracted = new ArrayList<>(); - var documentHasObjectArrays = processLevel(document, fieldName, extracted, false); - - if (extracted.size() == 1) { - return new ExtractedFieldValues(extracted.get(0), documentHasObjectArrays); - } - - return new ExtractedFieldValues(extracted, documentHasObjectArrays); - } - - @SuppressWarnings("unchecked") - private boolean processLevel(Map level, String field, ArrayList extracted, boolean documentHasObjectArrays) { - if (field.contains(".") == false) { - var value = level.get(field); - processLeafLevel(value, extracted); - return documentHasObjectArrays; - } - - var nameInLevel = field.split("\\.")[0]; - var entry = level.get(nameInLevel); - if (entry instanceof Map m) { - return processLevel((Map) m, field.substring(field.indexOf('.') + 1), extracted, documentHasObjectArrays); - } - if (entry instanceof List l) { - for (var object : l) { - processLevel((Map) object, field.substring(field.indexOf('.') + 1), extracted, true); - } - return true; - } - - assert false : "unexpected document structure"; - return false; - } - - private void processLeafLevel(Object value, ArrayList extracted) { - if (value instanceof List l) { - if (l.size() > 0 && l.get(0) instanceof Double) { - // this must be a single point in array form - // we'll put it into a different form here to make our lives a bit easier while implementing `expected` - extracted.add(Map.of("type", "point", "coordinates", l)); - } else { - // this is actually an array of points but there could still be points in array form inside - for (var arrayValue : l) { - processLeafLevel(arrayValue, extracted); - } - } - } else { - extracted.add(value); - } - } - @SuppressWarnings("unchecked") private GeoPoint convert(Object value, GeoPoint nullValue, boolean needsMultifieldAdjustment) { if (value == null) {