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.ClusterStateUpdateTask;
 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_DEPTH_L
 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 class DynamicMappingIT extends ESIntegTestCase {
         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,8 +13,8 @@ import org.apache.lucene.index.IndexableField;
 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;
@@ -22,6 +22,7 @@ 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 @@ public final class DocumentParser {
             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 @@ public final class DocumentParser {
         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 @@ public final class DocumentParser {
                 // 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 @@ public final class DocumentParser {
             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 @@ package org.elasticsearch.index.mapper;
 
 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 @@ import java.util.Map;
 
 /**
  * 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,10 +122,8 @@ final class DynamicFieldsBuilder {
 
     /**
      * 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());
@@ -132,7 +131,6 @@ final class DynamicFieldsBuilder {
 
     /**
      * 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 @@ final class DynamicFieldsBuilder {
 
     /**
      * 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 class DocumentParserTests extends MapperServiceTestCase {
         }));
         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 class DynamicMappingTests extends MapperServiceTestCase {
         }));
 
         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 class DynamicMappingTests extends MapperServiceTestCase {
         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 class DynamicMappingTests extends MapperServiceTestCase {
         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 class DynamicRuntimeTests extends MapperServiceTestCase {
             "{\"_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 class DynamicRuntimeTests extends MapperServiceTestCase {
             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())
+        );
+    }
 }