[8.19] Do not respect synthetic_source_keep=arrays if type parses arrays (#127796) (#127999)

* Do not respect synthetic_source_keep=arrays if type parses arrays (#127796)

Types that parse arrays directly should not need to store values in _ignored_source if synthetic_source_keep=arrays. Since they have custom handling of arrays, it provides no benefit to store in _ignored_source when there are multiple values of the type.

(cherry picked from commit c04a956)

* Broke some stuff while merging

Author: parkertimmins

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

docs/reference/mapping/types/geo-point.asciidoc

@@ -220,7 +220,10 @@ any issues, but features in technical preview are not subject to the support SLA
 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:
+
 [source,console,id=synthetic-source-geo-point-example]
 ----
 PUT idx
@@ -236,15 +239,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}
   ]
 }
 ----

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)

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<String, Object> fieldMapping, Object value, TestContext testContext) {
-        var extractedFieldValues = (ExtractedFieldValues) value;
-        var values = extractedFieldValues.values();
-
+    protected Object expected(Map<String, Object> fieldMapping, Object values, TestContext testContext) {
         var rawNullValue = fieldMapping.get("null_value");
 
         GeoPoint nullValue;
@@ -80,9 +76,6 @@ protected Object expected(Map<String, Object> 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)) {
@@ -117,61 +110,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<String, Object> 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<String, Object> level, String field, ArrayList<Object> 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<String, Object>) m, field.substring(field.indexOf('.') + 1), extracted, documentHasObjectArrays);
-        }
-        if (entry instanceof List<?> l) {
-            for (var object : l) {
-                processLevel((Map<String, Object>) object, field.substring(field.indexOf('.') + 1), extracted, true);
-            }
-            return true;
-        }
-
-        assert false : "unexpected document structure";
-        return false;
-    }
-
-    private void processLeafLevel(Object value, ArrayList<Object> 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) {