Dynamic runtime to not dynamically create objects (#74234)

When we introduced dynamic:runtime (#65489) we decided to have it create objects dynamically under properties, as the runtime section did not (and still does not) support object fields. That proved to be a poor choice, because the runtime section is flat, supports dots in field names, and does not really need objects. Also, these end up causing unnecessary mapping conflicts.

With this commit we adapt dynamic:runtime to not dynamically create objects.

Closes #70268

docs/reference/mapping/dynamic/field-mapping.asciidoc

@@ -22,12 +22,12 @@ h| JSON data type h| `"dynamic":"true"` h| `"dynamic":"runtime"`
  |`true` or `false` 2*| `boolean`
  |`double` | `float` | `double`
  |`integer` 2*| `long`
- |`object`^1^  2*| `object`
+ |`object` | `object` | No field added
  |`array` 2*|  Depends on the first non-`null` value in the array
  |`string` that passes <<date-detection,date detection>> 2*| `date`
  |`string` that passes <<numeric-detection,numeric detection>> | `float` or `long` | `double` or `long`
  |`string` that doesn't pass `date` detection or `numeric` detection | `text` with a `.keyword` sub-field | `keyword`
-3+| ^1^Objects are always mapped as part of the `properties` section, even when the `dynamic` parameter is set to `runtime`. | |
+3+|
 |===
 // end::dynamic-field-mapping-types-tag[]
 

server/src/internalClusterTest/java/org/elasticsearch/index/mapper/DynamicMappingIT.java

@@ -20,13 +20,17 @@
 import org.elasticsearch.cluster.metadata.MappingMetadata;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Randomness;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentType;
+import org.elasticsearch.common.xcontent.support.XContentMapValues;
+import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.index.query.GeoBoundingBoxQueryBuilder;
 import org.elasticsearch.plugins.Plugin;
+import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.InternalSettingsPlugin;
 import org.hamcrest.Matchers;
@@ -44,6 +48,8 @@
 import static org.elasticsearch.index.mapper.MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchHits;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
@@ -327,4 +333,78 @@ public void testBulkRequestWithNotFoundDynamicTemplate() throws Exception {
         assertThat(bulkItemResponses.getItems()[1].getFailureMessage(),
             containsString("Can't find dynamic template for dynamic template name [bar_foo] of field [address.location]"));
     }
+
+    public void testDynamicRuntimeNoConflicts() {
+        assertAcked(client().admin().indices().prepareCreate("test").setMapping("{\"_doc\":{\"dynamic\":\"runtime\"}}").get());
+
+        List<IndexRequest> docs = new ArrayList<>();
+        docs.add(new IndexRequest("test").source("one.two.three", new int[]{1, 2, 3}));
+        docs.add(new IndexRequest("test").source("one.two", 1.2));
+        docs.add(new IndexRequest("test").source("one", "one"));
+        docs.add(new IndexRequest("test").source("{\"one\":{\"two\": { \"three\": \"three\"}}}", XContentType.JSON));
+        Collections.shuffle(docs, random());
+        BulkRequest bulkRequest = new BulkRequest();
+        for (IndexRequest doc : docs) {
+            bulkRequest.add(doc);
+        }
+        BulkResponse bulkItemResponses = client().bulk(bulkRequest).actionGet();
+        assertFalse(bulkItemResponses.buildFailureMessage(), bulkItemResponses.hasFailures());
+
+        GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("test").get();
+        Map<String, Object> sourceAsMap = getMappingsResponse.getMappings().get("test").sourceAsMap();
+        assertFalse(sourceAsMap.containsKey("properties"));
+        @SuppressWarnings("unchecked")
+        Map<String, Object> runtime = (Map<String, Object>)sourceAsMap.get("runtime");
+        //depending on the order of the documents field types may differ, but there are no mapping conflicts
+        assertThat(runtime.keySet(), containsInAnyOrder("one", "one.two", "one.two.three"));
+    }
+
+    public void testDynamicRuntimeObjectFields() {
+        assertAcked(client().admin().indices().prepareCreate("test").setMapping("{\"_doc\":{\"properties\":{" +
+            "\"obj\":{\"properties\":{\"runtime\":{\"type\":\"object\",\"dynamic\":\"runtime\"}}}}}}").get());
+
+        List<IndexRequest> docs = new ArrayList<>();
+        docs.add(new IndexRequest("test").source("obj.one", 1));
+        docs.add(new IndexRequest("test").source("anything", 1));
+        docs.add(new IndexRequest("test").source("obj.runtime.one.two", "test"));
+        docs.add(new IndexRequest("test").source("obj.runtime.one", "one"));
+        docs.add(new IndexRequest("test").source("{\"obj\":{\"runtime\":{\"one\":{\"two\": \"test\"}}}}", XContentType.JSON));
+        Collections.shuffle(docs, random());
+        BulkRequest bulkRequest = new BulkRequest();
+        for (IndexRequest doc : docs) {
+            bulkRequest.add(doc);
+        }
+        BulkResponse bulkItemResponses = client().bulk(bulkRequest).actionGet();
+        assertFalse(bulkItemResponses.buildFailureMessage(), bulkItemResponses.hasFailures());
+
+        MapperParsingException exception = expectThrows(MapperParsingException.class,
+            () -> client().prepareIndex("test").setSource("obj.runtime", "value").get());
+        assertEquals("object mapping for [obj.runtime] tried to parse field [obj.runtime] as object, but found a concrete value",
+            exception.getMessage());
+
+        assertEquals("{\"test\":{\"mappings\":" +
+                "{\"runtime\":{\"obj.runtime.one\":{\"type\":\"keyword\"},\"obj.runtime.one.two\":{\"type\":\"keyword\"}}," +
+                "\"properties\":{\"anything\":{\"type\":\"long\"}," +
+                "\"obj\":{\"properties\":{\"one\":{\"type\":\"long\"}," +
+                "\"runtime\":{\"type\":\"object\",\"dynamic\":\"runtime\"}}}}}}}",
+            Strings.toString(client().admin().indices().prepareGetMappings("test").get()));
+
+        assertAcked(client().admin().indices().preparePutMapping("test").setSource("{\"_doc\":{\"properties\":{\"obj\":{\"properties\":" +
+            "{\"runtime\":{\"properties\":{\"dynamic\":{\"type\":\"object\", \"dynamic\":true}}}}}}}}", XContentType.JSON));
+
+        assertEquals(RestStatus.CREATED, client().prepareIndex("test").setSource("obj.runtime.dynamic.leaf", 1).get().status());
+        GetMappingsResponse getMappingsResponse = client().admin().indices().prepareGetMappings("test").get();
+        Map<String, Object> sourceAsMap = getMappingsResponse.getMappings().get("test").sourceAsMap();
+        assertThat(
+            XContentMapValues.extractRawValues("properties.obj.properties.runtime.properties.dynamic.properties.leaf.type", sourceAsMap),
+            contains("long"));
+    }
+
+    private static Map<String, Object> getMappedField(Map<String, Object> sourceAsMap, String name) {
+        @SuppressWarnings("unchecked")
+        Map<String, Object> properties = (Map<String, Object>)sourceAsMap.get("properties");
+        @SuppressWarnings("unchecked")
+        Map<String, Object> mappedField = (Map<String, Object>)properties.get(name);
+        return mappedField;
+    }
 }

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

@@ -13,15 +13,16 @@
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.Query;
 import org.elasticsearch.Version;
+import org.elasticsearch.common.Explicit;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.core.Tuple;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
+import org.elasticsearch.core.Tuple;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.analysis.IndexAnalyzers;
 import org.elasticsearch.index.fielddata.IndexFieldDataCache;
@@ -568,12 +569,19 @@ private static void parseObject(final ParseContext context, ObjectMapper mapper,
             ObjectMapper.Dynamic dynamic = dynamicOrDefault(parentMapper, context);
             if (dynamic == ObjectMapper.Dynamic.STRICT) {
                 throw new StrictDynamicMappingException(mapper.fullPath(), currentFieldName);
-            } else if ( dynamic == ObjectMapper.Dynamic.FALSE) {
+            } else if (dynamic == ObjectMapper.Dynamic.FALSE) {
                 // not dynamic, read everything up to end object
                 context.parser().skipChildren();
             } else {
-                Mapper dynamicObjectMapper = dynamic.getDynamicFieldsBuilder().createDynamicObjectMapper(context, currentFieldName);
-                context.addDynamicMapper(dynamicObjectMapper);
+                Mapper dynamicObjectMapper;
+                if (dynamic == ObjectMapper.Dynamic.RUNTIME) {
+                    //with dynamic:runtime all leaf fields will be runtime fields unless explicitly mapped,
+                    //hence we don't dynamically create empty objects under properties, but rather carry around an artificial object mapper
+                    dynamicObjectMapper = new NoOpObjectMapper(currentFieldName, context.path().pathAsText(currentFieldName));
+                } else {
+                    dynamicObjectMapper = dynamic.getDynamicFieldsBuilder().createDynamicObjectMapper(context, currentFieldName);
+                    context.addDynamicMapper(dynamicObjectMapper);
+                }
                 context.path().add(currentFieldName);
                 parseObjectOrField(context, dynamicObjectMapper);
                 context.path().remove();
@@ -759,7 +767,8 @@ private static Tuple<Integer, ObjectMapper> getDynamicParentMapper(ParseContext
         int pathsAdded = 0;
         ObjectMapper parent = mapper;
         for (int i = 0; i < paths.length-1; i++) {
-            String currentPath = context.path().pathAsText(paths[i]);
+            String name = paths[i];
+            String currentPath = context.path().pathAsText(name);
             Mapper existingFieldMapper = context.mappingLookup().getMapper(currentPath);
             if (existingFieldMapper != null) {
                 throw new MapperParsingException(
@@ -771,13 +780,14 @@ private static Tuple<Integer, ObjectMapper> getDynamicParentMapper(ParseContext
                 // One mapping is missing, check if we are allowed to create a dynamic one.
                 ObjectMapper.Dynamic dynamic = dynamicOrDefault(parent, context);
                 if (dynamic == ObjectMapper.Dynamic.STRICT) {
-                    throw new StrictDynamicMappingException(parent.fullPath(), paths[i]);
+                    throw new StrictDynamicMappingException(parent.fullPath(), name);
                 } else if (dynamic == ObjectMapper.Dynamic.FALSE) {
                     // Should not dynamically create any more mappers so return the last mapper
                     return new Tuple<>(pathsAdded, parent);
+                } else if (dynamic == ObjectMapper.Dynamic.RUNTIME) {
+                        mapper = new NoOpObjectMapper(name, currentPath);
                 } else {
-                    //objects are created under properties even with dynamic: runtime, as the runtime section only holds leaf fields
-                    final Mapper fieldMapper = dynamic.getDynamicFieldsBuilder().createDynamicObjectMapper(context, paths[i]);
+                    final Mapper fieldMapper = dynamic.getDynamicFieldsBuilder().createDynamicObjectMapper(context, name);
                     if (fieldMapper instanceof ObjectMapper == false) {
                         assert context.sourceToParse().dynamicTemplates().containsKey(currentPath) :
                             "dynamic templates [" + context.sourceToParse().dynamicTemplates() + "]";
@@ -957,4 +967,10 @@ protected String contentType() {
             throw new UnsupportedOperationException();
         }
     }
+
+    private static class NoOpObjectMapper extends ObjectMapper {
+        NoOpObjectMapper(String name, String fullPath) {
+            super(name, fullPath, new Explicit<>(true, false), Nested.NO, Dynamic.RUNTIME, Collections.emptyMap(), Version.CURRENT);
+        }
+    }
 }

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

@@ -10,10 +10,10 @@
 
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.CheckedBiConsumer;
-import org.elasticsearch.core.CheckedRunnable;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.core.CheckedRunnable;
 import org.elasticsearch.index.mapper.ObjectMapper.Dynamic;
 import org.elasticsearch.script.ScriptCompiler;
 
@@ -23,8 +23,9 @@
 
 /**
  * Encapsulates the logic for dynamically creating fields as part of document parsing.
- * Objects are always created the same, but leaf fields can be mapped under properties, as concrete fields that get indexed,
+ * Fields can be mapped under properties, as concrete fields that get indexed,
  * or as runtime fields that are evaluated at search-time and have no indexing overhead.
+ * Objects get dynamically mapped only under dynamic:true.
  */
 final class DynamicFieldsBuilder {
     private static final Concrete CONCRETE = new Concrete(DocumentParser::parseObjectOrField);
@@ -121,18 +122,15 @@ void createDynamicFieldFromValue(final ParseContext context,
 
     /**
      * Returns a dynamically created object mapper, eventually based on a matching dynamic template.
-     * Note that objects are always mapped under properties.
      */
     Mapper createDynamicObjectMapper(ParseContext context, String name) {
-        //dynamic:runtime maps objects under properties, exactly like dynamic:true
         Mapper mapper = createObjectMapperFromTemplate(context, name);
         return mapper != null ? mapper :
             new ObjectMapper.Builder(name, context.indexSettings().getIndexVersionCreated()).enabled(true).build(context.path());
     }
 
     /**
      * Returns a dynamically created object mapper, based exclusively on a matching dynamic template, null otherwise.
-     * Note that objects are always mapped under properties.
      */
     Mapper createObjectMapperFromTemplate(ParseContext context, String name) {
         Mapper.Builder templateBuilder = findTemplateBuilderForObject(context, name);
@@ -311,7 +309,7 @@ void newDynamicBinaryField(ParseContext context, String name) throws IOException
 
     /**
      * Dynamically creates runtime fields, in the runtime section.
-     * Used for leaf fields, when their parent object is mapped as dynamic:runtime.
+     * Used for sub-fields of objects that are mapped as dynamic:runtime.
      * @see Dynamic
      */
     private static final class Runtime implements Strategy {

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

@@ -556,8 +556,7 @@ public void testPropagateDynamicRuntimeWithDynamicMapper() throws Exception {
         }));
         assertNull(doc.rootDoc().getField("foo.bar.baz"));
         assertEquals("{\"_doc\":{\"dynamic\":\"false\"," +
-            "\"runtime\":{\"foo.bar.baz\":{\"type\":\"keyword\"},\"foo.baz\":{\"type\":\"keyword\"}}," +
-            "\"properties\":{\"foo\":{\"dynamic\":\"runtime\",\"properties\":{\"bar\":{\"type\":\"object\"}}}}}}",
+            "\"runtime\":{\"foo.bar.baz\":{\"type\":\"keyword\"},\"foo.baz\":{\"type\":\"keyword\"}}}}",
             Strings.toString(doc.dynamicMappingsUpdate()));
     }
 

server/src/test/java/org/elasticsearch/index/mapper/DynamicMappingTests.java

@@ -7,11 +7,11 @@
  */
 package org.elasticsearch.index.mapper;
 
-import org.elasticsearch.core.CheckedConsumer;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.core.CheckedConsumer;
 
 import java.io.IOException;
 import java.time.Instant;
@@ -302,8 +302,7 @@ public void testDynamicRuntimeFieldWithinObject() throws Exception {
         }));
 
         assertEquals("{\"_doc\":{\"dynamic\":\"runtime\"," +
-            "\"runtime\":{\"foo.bar.baz\":{\"type\":\"long\"}}," +
-            "\"properties\":{\"foo\":{\"properties\":{\"bar\":{\"type\":\"object\"}}}}}}",
+            "\"runtime\":{\"foo.bar.baz\":{\"type\":\"long\"}}}}",
             Strings.toString(doc.dynamicMappingsUpdate()));
     }
 
@@ -326,8 +325,7 @@ public void testDynamicRuntimeMappingDynamicObject() throws Exception {
         assertEquals("{\"_doc\":{\"dynamic\":\"runtime\"," +
                 "\"runtime\":{\"object.foo.bar.baz\":{\"type\":\"long\"}}," +
                 "\"properties\":{\"dynamic_object\":{\"dynamic\":\"true\"," +
-                "\"properties\":{\"foo\":{" + "\"properties\":{\"bar\":{" + "\"properties\":{\"baz\":" + "{\"type\":\"long\"}}}}}}}," +
-                "\"object\":{\"properties\":{\"foo\":{\"properties\":{\"bar\":{\"type\":\"object\"}}}}}}}}",
+                "\"properties\":{\"foo\":{\"properties\":{\"bar\":{\"properties\":{\"baz\":{\"type\":\"long\"}}}}}}}}}}",
             Strings.toString(doc.dynamicMappingsUpdate()));
     }
 
@@ -350,8 +348,7 @@ public void testDynamicMappingDynamicRuntimeObject() throws Exception {
         assertEquals("{\"_doc\":{\"dynamic\":\"true\",\"" +
                 "runtime\":{\"runtime_object.foo.bar.baz\":{\"type\":\"keyword\"}}," +
                 "\"properties\":{\"object\":{\"properties\":{\"foo\":{\"properties\":{\"bar\":{\"properties\":{" +
-                "\"baz\":{\"type\":\"text\",\"fields\":{\"keyword\":{\"type\":\"keyword\",\"ignore_above\":256}}}}}}}}}," +
-                "\"runtime_object\":{\"dynamic\":\"runtime\",\"properties\":{\"foo\":{\"properties\":{\"bar\":{\"type\":\"object\"}}}}}}}}",
+                "\"baz\":{\"type\":\"text\",\"fields\":{\"keyword\":{\"type\":\"keyword\",\"ignore_above\":256}}}}}}}}}}}}",
             Strings.toString(doc.dynamicMappingsUpdate()));
     }
 

server/src/test/java/org/elasticsearch/index/mapper/DynamicRuntimeTests.java

@@ -9,10 +9,6 @@
 package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.index.mapper.DocumentMapper;
-import org.elasticsearch.index.mapper.MapperServiceTestCase;
-import org.elasticsearch.index.mapper.ObjectMapper;
-import org.elasticsearch.index.mapper.ParsedDocument;
 
 import java.io.IOException;
 
@@ -72,7 +68,7 @@ public void testWithObjects() throws IOException {
             "{\"_doc\":{\"dynamic\":\"false\","
                 + "\"runtime\":{\"dynamic_runtime.child.field4\":{\"type\":\"keyword\"},"
                 + "\"dynamic_runtime.field3\":{\"type\":\"keyword\"}},"
-                + "\"properties\":{\"dynamic_runtime\":{\"dynamic\":\"runtime\",\"properties\":{\"child\":{\"type\":\"object\"}}},"
+                + "\"properties\":{"
                 + "\"dynamic_true\":{\"dynamic\":\"true\",\"properties\":{\"child\":{\"properties\":{"
                 + "\"field2\":{\"type\":\"text\",\"fields\":{\"keyword\":{\"type\":\"keyword\",\"ignore_above\":256}}}}},"
                 + "\"field1\":{\"type\":\"text\",\"fields\":{\"keyword\":{\"type\":\"keyword\",\"ignore_above\":256}}}}}}}}",
@@ -109,4 +105,21 @@ public void testWithDynamicTemplate() throws IOException {
             Strings.toString(parsedDoc.dynamicMappingsUpdate())
         );
     }
+
+    public void testDotsInFieldNames() throws IOException {
+        DocumentMapper documentMapper = createDocumentMapper(topMapping(b -> b.field("dynamic", ObjectMapper.Dynamic.RUNTIME)));
+        ParsedDocument doc = documentMapper.parse(source(b -> {
+            b.field("one.two.three.four", "1234");
+            b.field("one.two.three", 123);
+            b.array("one.two", 1.2, 1.2, 1.2);
+            b.field("one", "one");
+        }));
+        assertEquals("{\"_doc\":{\"dynamic\":\"runtime\",\"runtime\":{" +
+                "\"one\":{\"type\":\"keyword\"}," +
+                "\"one.two\":{\"type\":\"double\"}," +
+                "\"one.two.three\":{\"type\":\"long\"}," +
+                "\"one.two.three.four\":{\"type\":\"keyword\"}}}}",
+            Strings.toString(doc.dynamicMappingsUpdate())
+        );
+    }
 }