Don't parse unmapped array field when dynamic is set to false (#85082)

When parsing array fields from incoming documents and dynamic for the current context is set to false, we should not go ahead and parse all the children, but rather skip parsing. We had a TODO for a very long time in the code around this, which this commit addresses.
elastic · Mar 28, 2022 · c3b2175 · c3b2175
1 parent ff517c5
commit c3b2175
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 2 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java
@@ -438,8 +438,7 @@ private static void parseArray(DocumentParserContext context, ObjectMapper paren
             if (dynamic == ObjectMapper.Dynamic.STRICT) {
                 throw new StrictDynamicMappingException(parentMapper.fullPath(), lastFieldName);
             } else if (dynamic == ObjectMapper.Dynamic.FALSE) {
-                // TODO: shouldn't this skip, not parse?
-                parseNonDynamicArray(context, parentMapper, lastFieldName, lastFieldName);
+                context.parser().skipChildren();
             } else {
                 Mapper objectMapperFromTemplate = dynamic.getDynamicFieldsBuilder().createObjectMapperFromTemplate(context, lastFieldName);
                 if (objectMapperFromTemplate == null) {

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java
@@ -820,4 +820,86 @@ public void testDottedFieldDynamicFalse() throws IOException {
 
         assertNull(doc.dynamicMappingsUpdate());
     }
+
+    public void testArraysDynamicFalse() throws IOException {
+        DocumentMapper defaultMapper = createDocumentMapper(
+            dynamicMapping("false", b -> b.startObject("myarray").field("type", "keyword").endObject())
+        );
+
+        ParsedDocument doc = defaultMapper.parse(source(b -> {
+            b.array("unmapped", "unknown1", "unknown2");
+            b.array("myarray", "array1", "array2");
+        }));
+
+        assertThat(doc.rootDoc().getFields("myarray"), arrayWithSize(4));
+        assertThat(doc.rootDoc().getFields("unmapped"), arrayWithSize(0));
+        assertNull(doc.dynamicMappingsUpdate());
+    }
+
+    public void testArraysOfObjectsRootDynamicFalse() throws IOException {
+        DocumentMapper defaultMapper = createDocumentMapper(
+            dynamicMapping(
+                "false",
+                b -> b.startObject("objects")
+                    .startObject("properties")
+                    .startObject("subfield")
+                    .field("type", "keyword")
+                    .endObject()
+                    .endObject()
+                    .endObject()
+            )
+        );
+
+        ParsedDocument doc = defaultMapper.parse(source(b -> {
+            b.startArray("objects");
+            b.startObject().field("subfield", "sub").field("unmapped", "unmapped").endObject();
+            b.endArray();
+            b.startArray("unmapped");
+            b.startObject().field("subfield", "unmapped").endObject();
+            b.endArray();
+        }));
+
+        assertThat(doc.rootDoc().getFields("objects.subfield"), arrayWithSize(2));
+        assertThat(doc.rootDoc().getFields("objects.unmapped"), arrayWithSize(0));
+        assertThat(doc.rootDoc().getFields("unmapped.subfield"), arrayWithSize(0));
+        assertNull(doc.dynamicMappingsUpdate());
+    }
+
+    public void testArraysOfObjectsDynamicFalse() throws IOException {
+        DocumentMapper defaultMapper = createDocumentMapper(
+            dynamicMapping(
+                "true",
+                b -> b.startObject("objects")
+                    .field("dynamic", false)
+                    .startObject("properties")
+                    .startObject("subfield")
+                    .field("type", "keyword")
+                    .endObject()
+                    .endObject()
+                    .endObject()
+            )
+        );
+
+        ParsedDocument doc = defaultMapper.parse(source(b -> {
+            b.startArray("objects");
+            b.startObject().field("subfield", "sub").field("unmapped", "unmapped").endObject();
+            b.endArray();
+            b.field("myfield", 2);
+        }));
+
+        assertThat(doc.rootDoc().getFields("myfield"), arrayWithSize(2));
+        assertThat(doc.rootDoc().getFields("objects.subfield"), arrayWithSize(2));
+        assertThat(doc.rootDoc().getFields("objects.unmapped"), arrayWithSize(0));
+        assertEquals(XContentHelper.stripWhitespace("""
+            {
+              "_doc": {
+                "dynamic":"true",
+                "properties":{
+                  "myfield":{
+                    "type":"long"
+                  }
+                }
+              }
+            }"""), Strings.toString(doc.dynamicMappingsUpdate()));
+    }
 }