Add the ability to remove a runtime field (#68992)

Currently, existing runtime fields can be updated, but they cannot be removed. That allows to correct potential mistakes, but once a runtime field is added to the index mappings, it is not possible to remove it.

With this commit we introduce the ability to remove an existing runtime field by providing a null value for it through the put mapping API. If a field with such name does not exist, such specific instruction will have no effect on other existing runtime fields.

Note that the removal of runtime fields makes the recently introduced assertRefreshItNotNeeded assertion trip, because when each local node merges mappings back in, the runtime fields that were previously removed by the master node, get added back again locally. This is only a problem for the assertion that verifies that the removed refresh operation is never needed. We worked around this by tweaking the assertion to ignore runtime fields completely, for simplicity, by assertion on the serialized merged mappings and incoming mappings without the corresponding runtime section.

Co-authored-by: Adam Locke <adam.locke@elastic.co>

docs/reference/mapping/runtime.asciidoc

@@ -139,18 +139,30 @@ PUT my-index
 }
 ----
 
+You can update or remove runtime fields at any time. To replace an existing runtime field, add a new runtime field to the mappings with the same name. To remove a runtime field from the mappings, set the value of the runtime field to `null`:
+
+ [source,console]
+ ----
+ PUT my-index/_mapping
+ {
+   "runtime": {
+     "day_of_week": null
+   }
+ }
+ ----
+
 [[runtime-updating-scripts]]
-.Updating runtime scripts
+.Updating and removing runtime fields
 ****
 
-Updating a script while a dependent query is running can return
+Updating or removing a runtime field while a dependent query is running can return
 inconsistent results. Each shard might have access to different versions of the
 script, depending on when the mapping change takes effect.
 
 Existing queries or visualizations in {kib} that rely on runtime fields can
-fail if you change the field type. For example, a bar chart visualization
+fail if you remove or update the field. For example, a bar chart visualization
 that uses a runtime field of type `ip` will fail if the type is changed
-to `boolean`.
+to `boolean`, or if the runtime field is removed.
 
 ****
 

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

@@ -186,7 +186,7 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
             DocumentMapper previousMapper;
             synchronized (this) {
                 previousMapper = this.mapper;
-                assert assertRefreshIsNotNeeded(previousMapper, type, incomingMappingSource, incomingMapping);
+                assert assertRefreshIsNotNeeded(previousMapper, type, incomingMapping);
                 this.mapper = newDocumentMapper(incomingMapping, MergeReason.MAPPING_RECOVERY);
             }
             String op = previousMapper != null ? "updated" : "added";
@@ -202,13 +202,20 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
     }
 
     private boolean assertRefreshIsNotNeeded(DocumentMapper currentMapper,
-                                          String type,
-                                          CompressedXContent incomingMappingSource,
-                                          Mapping incomingMapping) {
+                                             String type,
+                                             Mapping incomingMapping) {
         Mapping mergedMapping = mergeMappings(currentMapper, incomingMapping, MergeReason.MAPPING_RECOVERY);
+        //skip the runtime section or removed runtime fields will make the assertion fail
+        ToXContent.MapParams params = new ToXContent.MapParams(Collections.singletonMap(RootObjectMapper.TOXCONTENT_SKIP_RUNTIME, "true"));
         CompressedXContent mergedMappingSource;
         try {
-            mergedMappingSource = new CompressedXContent(mergedMapping, XContentType.JSON, ToXContent.EMPTY_PARAMS);
+            mergedMappingSource = new CompressedXContent(mergedMapping, XContentType.JSON, params);
+        } catch (Exception e) {
+            throw new AssertionError("failed to serialize source for type [" + type + "]", e);
+        }
+        CompressedXContent incomingMappingSource;
+        try {
+            incomingMappingSource = new CompressedXContent(incomingMapping, XContentType.JSON, params);
         } catch (Exception e) {
             throw new AssertionError("failed to serialize source for type [" + type + "]", e);
         }

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

@@ -41,6 +41,16 @@ import static org.elasticsearch.index.mapper.TypeParsers.parseDateTimeFormatter;
 public class RootObjectMapper extends ObjectMapper {
     private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(RootObjectMapper.class);
 
+    /**
+     * Parameter used when serializing {@link RootObjectMapper} and request that the runtime section is skipped.
+     * This is only needed internally when we compare different versions of mappings and assert that they are the same.
+     * Runtime fields break these assertions as they can be removed: the master node sends the merged mappings without the runtime fields
+     * that needed to be removed. Then each local node as part of its assertions merges the incoming mapping with the current mapping,
+     *  and the previously removed runtime fields appear again, which is not desirable. The expectation is that those two versions of the
+     *  mappings are the same, besides runtime fields.
+     */
+    static final String TOXCONTENT_SKIP_RUNTIME = "skip_runtime";
+
     public static class Defaults {
         public static final DateFormatter[] DYNAMIC_DATE_TIME_FORMATTERS =
                 new DateFormatter[]{
@@ -57,7 +67,7 @@ public class RootObjectMapper extends ObjectMapper {
         protected Explicit<DateFormatter[]> dynamicDateTimeFormatters = new Explicit<>(Defaults.DYNAMIC_DATE_TIME_FORMATTERS, false);
         protected Explicit<Boolean> dateDetection = new Explicit<>(Defaults.DATE_DETECTION, false);
         protected Explicit<Boolean> numericDetection = new Explicit<>(Defaults.NUMERIC_DETECTION, false);
-        protected final Map<String, RuntimeFieldType> runtimeFieldTypes = new HashMap<>();
+        protected Map<String, RuntimeFieldType> runtimeFieldTypes;
 
         public Builder(String name, Version indexCreatedVersion) {
             super(name, indexCreatedVersion);
@@ -79,8 +89,8 @@ public class RootObjectMapper extends ObjectMapper {
             return this;
         }
 
-        public RootObjectMapper.Builder addRuntime(RuntimeFieldType runtimeFieldType) {
-            this.runtimeFieldTypes.put(runtimeFieldType.name(), runtimeFieldType);
+        public RootObjectMapper.Builder setRuntime(Map<String, RuntimeFieldType> runtimeFields) {
+            this.runtimeFieldTypes = runtimeFields;
             return this;
         }
 
@@ -93,7 +103,8 @@ public class RootObjectMapper extends ObjectMapper {
         protected ObjectMapper createMapper(String name, String fullPath, Explicit<Boolean> enabled, Nested nested, Dynamic dynamic,
                 Map<String, Mapper> mappers, Version indexCreatedVersion) {
             assert nested.isNested() == false;
-            return new RootObjectMapper(name, enabled, dynamic, mappers, runtimeFieldTypes,
+            return new RootObjectMapper(name, enabled, dynamic, mappers,
+                    runtimeFieldTypes == null ? Collections.emptyMap() : runtimeFieldTypes,
                     dynamicDateTimeFormatters,
                     dynamicTemplates,
                     dateDetection, numericDetection, indexCreatedVersion);
@@ -204,7 +215,7 @@ public class RootObjectMapper extends ObjectMapper {
                 return true;
             } else if (fieldName.equals("runtime")) {
                 if (fieldNode instanceof Map) {
-                    RuntimeFieldType.parseRuntimeFields((Map<String, Object>) fieldNode, parserContext, builder::addRuntime);
+                    builder.setRuntime(RuntimeFieldType.parseRuntimeFields((Map<String, Object>) fieldNode, parserContext, true));
                     return true;
                 } else {
                     throw new ElasticsearchParseException("runtime must be a map type");
@@ -326,7 +337,13 @@ public class RootObjectMapper extends ObjectMapper {
             }
         }
         assert this.runtimeFieldTypes != mergeWithObject.runtimeFieldTypes;
-        this.runtimeFieldTypes.putAll(mergeWithObject.runtimeFieldTypes);
+        for (Map.Entry<String, RuntimeFieldType> runtimeField : mergeWithObject.runtimeFieldTypes.entrySet()) {
+            if (runtimeField.getValue() == null) {
+                this.runtimeFieldTypes.remove(runtimeField.getKey());
+            } else {
+                this.runtimeFieldTypes.put(runtimeField.getKey(), runtimeField.getValue());
+            }
+        }
     }
 
     void addRuntimeFields(Collection<RuntimeFieldType> runtimeFields) {
@@ -364,7 +381,7 @@ public class RootObjectMapper extends ObjectMapper {
             builder.field("numeric_detection", numericDetection.value());
         }
 
-        if (runtimeFieldTypes.size() > 0) {
+        if (runtimeFieldTypes.size() > 0 && params.paramAsBoolean(TOXCONTENT_SKIP_RUNTIME, false) == false) {
             builder.startObject("runtime");
             List<RuntimeFieldType> sortedRuntimeFieldTypes = runtimeFieldTypes.values().stream().sorted(
                 Comparator.comparing(RuntimeFieldType::name)).collect(Collectors.toList());

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

@@ -12,10 +12,10 @@ import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
-import java.util.function.Consumer;
 
 /**
  * Base implementation for a runtime field that can be defined as part of the runtime section of the index mappings
@@ -50,14 +50,30 @@ public abstract class RuntimeFieldType extends MappedFieldType implements ToXCon
             throws MapperParsingException;
     }
 
-    public static void parseRuntimeFields(Map<String, Object> node,
-                                          Mapper.TypeParser.ParserContext parserContext,
-                                          Consumer<RuntimeFieldType> runtimeFieldTypeConsumer) {
+    /**
+     * Parse runtime fields from the provided map, using the provided parser context.
+     * @param node the map that holds the runtime fields configuration
+     * @param parserContext the parser context that holds info needed when parsing mappings
+     * @param supportsRemoval whether a null value for a runtime field should be properly parsed and
+     *                        translated to the removal of such runtime field
+     * @return the parsed runtime fields
+     */
+    public static Map<String, RuntimeFieldType> parseRuntimeFields(Map<String, Object> node,
+                                                                   Mapper.TypeParser.ParserContext parserContext,
+                                                                   boolean supportsRemoval) {
+        Map<String, RuntimeFieldType> runtimeFields = new HashMap<>();
         Iterator<Map.Entry<String, Object>> iterator = node.entrySet().iterator();
         while (iterator.hasNext()) {
             Map.Entry<String, Object> entry = iterator.next();
             String fieldName = entry.getKey();
-            if (entry.getValue() instanceof Map) {
+            if (entry.getValue() == null) {
+                if (supportsRemoval) {
+                    runtimeFields.put(fieldName, null);
+                } else {
+                    throw new MapperParsingException("Runtime field [" + fieldName + "] was set to null but its removal is not supported " +
+                        "in this context");
+                }
+            } else if (entry.getValue() instanceof Map) {
                 @SuppressWarnings("unchecked")
                 Map<String, Object> propNode = new HashMap<>(((Map<String, Object>) entry.getValue()));
                 Object typeNode = propNode.get("type");
@@ -72,14 +88,15 @@ public abstract class RuntimeFieldType extends MappedFieldType implements ToXCon
                     throw new MapperParsingException("No handler for type [" + type +
                         "] declared on runtime field [" + fieldName + "]");
                 }
-                runtimeFieldTypeConsumer.accept(typeParser.parse(fieldName, propNode, parserContext));
+                runtimeFields.put(fieldName, typeParser.parse(fieldName, propNode, parserContext));
                 propNode.remove("type");
                 MappingParser.checkNoRemainingFields(fieldName, propNode);
                 iterator.remove();
             } else {
                 throw new MapperParsingException("Expected map for runtime field [" + fieldName + "] definition but got a "
-                    + fieldName.getClass().getName());
+                    + entry.getValue().getClass().getName());
             }
         }
+        return Collections.unmodifiableMap(runtimeFields);
     }
 }

server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java

@@ -104,7 +104,7 @@ public class SearchExecutionContext extends QueryRewriteContext {
     private boolean mapUnmappedFieldAsString;
     private NestedScope nestedScope;
     private final ValuesSourceRegistry valuesSourceRegistry;
-    private final Map<String, MappedFieldType> runtimeMappings;
+    private final Map<String, RuntimeFieldType> runtimeMappings;
 
     /**
      * Build a {@linkplain SearchExecutionContext}.
@@ -197,7 +197,7 @@ public class SearchExecutionContext extends QueryRewriteContext {
                                    Index fullyQualifiedIndex,
                                    BooleanSupplier allowExpensiveQueries,
                                    ValuesSourceRegistry valuesSourceRegistry,
-                                   Map<String, MappedFieldType> runtimeMappings) {
+                                   Map<String, RuntimeFieldType> runtimeMappings) {
         super(xContentRegistry, namedWriteableRegistry, client, nowInMillis);
         this.shardId = shardId;
         this.shardRequestIndex = shardRequestIndex;
@@ -597,13 +597,11 @@ public class SearchExecutionContext extends QueryRewriteContext {
         return fullyQualifiedIndex;
     }
 
-    private static Map<String, MappedFieldType> parseRuntimeMappings(Map<String, Object> runtimeMappings, MapperService mapperService) {
-        Map<String, MappedFieldType> runtimeFieldTypes = new HashMap<>();
-        if (runtimeMappings.isEmpty() == false) {
-            RuntimeFieldType.parseRuntimeFields(new HashMap<>(runtimeMappings), mapperService.parserContext(),
-                runtimeFieldType -> runtimeFieldTypes.put(runtimeFieldType.name(), runtimeFieldType));
+    private static Map<String, RuntimeFieldType> parseRuntimeMappings(Map<String, Object> runtimeMappings, MapperService mapperService) {
+        if (runtimeMappings.isEmpty()) {
+            return Collections.emptyMap();
         }
-        return Collections.unmodifiableMap(runtimeFieldTypes);
+        return RuntimeFieldType.parseRuntimeFields(new HashMap<>(runtimeMappings), mapperService.parserContext(), false);
     }
 
     /**

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

@@ -14,6 +14,8 @@ import org.elasticsearch.test.ESTestCase;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.emptyMap;
@@ -197,7 +199,8 @@ public class FieldAliasMapperValidationTests extends ESTestCase {
                                                      List<FieldAliasMapper> fieldAliasMappers,
                                                      List<RuntimeFieldType> runtimeFields) {
         RootObjectMapper.Builder builder = new RootObjectMapper.Builder("_doc", Version.CURRENT);
-        runtimeFields.forEach(builder::addRuntime);
+        Map<String, RuntimeFieldType> runtimeFieldTypes = runtimeFields.stream().collect(Collectors.toMap(MappedFieldType::name, r -> r));
+        builder.setRuntime(runtimeFieldTypes);
         Mapping mapping = new Mapping(builder.build(new ContentPath()), new MetadataFieldMapper[0], Collections.emptyMap());
         return new MappingLookup(mapping, fieldMappers, objectMappers, fieldAliasMappers, null, null, null);
     }

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

@@ -24,6 +24,8 @@ import java.io.StringReader;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
 
 import static java.util.Collections.emptyList;
 import static org.hamcrest.CoreMatchers.instanceOf;
@@ -34,7 +36,8 @@ public class MappingLookupTests extends ESTestCase {
                                                      List<ObjectMapper> objectMappers,
                                                      List<RuntimeFieldType> runtimeFields) {
         RootObjectMapper.Builder builder = new RootObjectMapper.Builder("_doc", Version.CURRENT);
-        runtimeFields.forEach(builder::addRuntime);
+        Map<String, RuntimeFieldType> runtimeFieldTypes = runtimeFields.stream().collect(Collectors.toMap(MappedFieldType::name, r -> r));
+        builder.setRuntime(runtimeFieldTypes);
         Mapping mapping = new Mapping(builder.build(new ContentPath()), new MetadataFieldMapper[0], Collections.emptyMap());
         return new MappingLookup(mapping, fieldMappers, objectMappers, emptyList(), null, null, null);
     }

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

@@ -103,7 +103,8 @@ public class ObjectMapperMergeTests extends ESTestCase {
             (RootObjectMapper) new RootObjectMapper.Builder(type, Version.CURRENT).enabled(false).build(new ContentPath());
         //the root is disabled, and we are not trying to re-enable it, but we do want to be able to add runtime fields
         final RootObjectMapper mergeWith =
-            new RootObjectMapper.Builder(type, Version.CURRENT).addRuntime(new TestRuntimeField("test", "long")).build(new ContentPath());
+            new RootObjectMapper.Builder(type, Version.CURRENT).setRuntime(
+                Collections.singletonMap("test", new TestRuntimeField("test", "long"))).build(new ContentPath());
 
         RootObjectMapper merged = (RootObjectMapper) rootObjectMapper.merge(mergeWith);
         assertFalse(merged.isEnabled());

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

@@ -648,6 +648,20 @@ public class RootObjectMapperTests extends MapperServiceTestCase {
                     "\"field\":{\"type\":\"keyword\"}}}}",
                 mapperService.documentMapper().mappingSource().toString());
         }
+        {
+            //remove a runtime field
+            String mapping = Strings.toString(runtimeMapping(
+                builder -> builder.nullField("field3")));
+            merge(mapperService, mapping);
+            assertEquals("{\"_doc\":" +
+                    "{\"runtime\":{" +
+                    "\"field\":{\"type\":\"test\",\"prop2\":\"second version\"}," +
+                    "\"field2\":{\"type\":\"test\"}}," +
+                    "\"properties\":{" +
+                    "\"concrete\":{\"type\":\"keyword\"}," +
+                    "\"field\":{\"type\":\"keyword\"}}}}",
+                mapperService.documentMapper().mappingSource().toString());
+        }
     }
 
     public void testRuntimeSectionNonRuntimeType() throws IOException {
@@ -669,9 +683,9 @@ public class RootObjectMapperTests extends MapperServiceTestCase {
     }
 
     public void testRuntimeSectionWrongFormat() throws IOException {
-        XContentBuilder mapping = runtimeMapping(builder -> builder.field("field", "value"));
+        XContentBuilder mapping = runtimeMapping(builder -> builder.field("field", 123));
         MapperParsingException e = expectThrows(MapperParsingException.class, () -> createMapperService(mapping));
-        assertEquals("Failed to parse mapping: Expected map for runtime field [field] definition but got a java.lang.String",
+        assertEquals("Failed to parse mapping: Expected map for runtime field [field] definition but got a java.lang.Integer",
             e.getMessage());
     }
 

server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java

@@ -46,6 +46,7 @@ import org.elasticsearch.index.mapper.IndexFieldMapper;
 import org.elasticsearch.index.mapper.KeywordFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.Mapper;
+import org.elasticsearch.index.mapper.MapperParsingException;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.Mapping;
 import org.elasticsearch.index.mapper.MappingLookup;
@@ -71,7 +72,9 @@ import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -320,7 +323,8 @@ public class SearchExecutionContextTests extends ESTestCase {
     private static MappingLookup createMappingLookup(List<MappedFieldType> concreteFields, List<RuntimeFieldType> runtimeFields) {
         List<FieldMapper> mappers = concreteFields.stream().map(MockFieldMapper::new).collect(Collectors.toList());
         RootObjectMapper.Builder builder = new RootObjectMapper.Builder("_doc", Version.CURRENT);
-        runtimeFields.forEach(builder::addRuntime);
+        Map<String, RuntimeFieldType> runtimeFieldTypes = runtimeFields.stream().collect(Collectors.toMap(MappedFieldType::name, r -> r));
+        builder.setRuntime(runtimeFieldTypes);
         Mapping mapping = new Mapping(builder.build(new ContentPath()), new MetadataFieldMapper[0], Collections.emptyMap());
         return new MappingLookup(mapping, mappers, Collections.emptyList(), Collections.emptyList(), null, null, null);
     }
@@ -353,6 +357,30 @@ public class SearchExecutionContextTests extends ESTestCase {
         assertThat(context.simpleMatchToIndexNames("*"), equalTo(Set.of("cat", "dog", "pig")));
     }
 
+    public void testSearchRequestRuntimeFieldsWrongFormat() {
+        Map<String, Object> runtimeMappings = new HashMap<>();
+        runtimeMappings.put("field", Arrays.asList("test1", "test2"));
+        MapperParsingException exception = expectThrows(MapperParsingException.class, () -> createSearchExecutionContext(
+            "uuid",
+            null,
+            createMappingLookup(List.of(new MockFieldMapper.FakeFieldType("pig"), new MockFieldMapper.FakeFieldType("cat")), List.of()),
+            runtimeMappings,
+            Collections.singletonList(new TestRuntimeField.Plugin())));
+        assertEquals("Expected map for runtime field [field] definition but got a java.util.Arrays$ArrayList", exception.getMessage());
+    }
+
+    public void testSearchRequestRuntimeFieldsRemoval() {
+        Map<String, Object> runtimeMappings = new HashMap<>();
+        runtimeMappings.put("field", null);
+        MapperParsingException exception = expectThrows(MapperParsingException.class, () -> createSearchExecutionContext(
+            "uuid",
+            null,
+            createMappingLookup(List.of(new MockFieldMapper.FakeFieldType("pig"), new MockFieldMapper.FakeFieldType("cat")), List.of()),
+            runtimeMappings,
+            Collections.singletonList(new TestRuntimeField.Plugin())));
+        assertEquals("Runtime field [field] was set to null but its removal is not supported in this context", exception.getMessage());
+    }
+
     public static SearchExecutionContext createSearchExecutionContext(String indexUuid, String clusterAlias) {
         return createSearchExecutionContext(indexUuid, clusterAlias, MappingLookup.EMPTY, Map.of(), List.of());
     }