Support synthetic source for date fields when ignore_malformed is used (#109410)

lkts · web-flow · commit a9f31bd2aa6f · 2024-06-10T10:26:31.000-07:00
diff --git a/docs/changelog/109410.yaml b/docs/changelog/109410.yaml
@@ -0,0 +1,5 @@
+pr: 109410
+summary: Support synthetic source for date fields when `ignore_malformed` is used
+area: Mapping
+type: enhancement
+issues: []
diff --git a/docs/reference/mapping/types/date.asciidoc b/docs/reference/mapping/types/date.asciidoc
@@ -242,8 +242,7 @@ of official GA features.
 
 `date` fields support <<synthetic-source,synthetic `_source`>> in their
 default configuration. Synthetic `_source` cannot be used together with
-<<copy-to,`copy_to`>>, <<ignore-malformed,`ignore_malformed`>> set to true
-or with <<doc-values,`doc_values`>> disabled.
+<<copy-to,`copy_to`>> or with <<doc-values,`doc_values`>> disabled.
 
 Synthetic source always sorts `date` fields. For example:
 [source,console,id=synthetic-source-date-example]
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java
@@ -364,7 +364,16 @@ public DateFieldMapper build(MapperBuilderContext context) {
                 && ignoreMalformed.isConfigured() == false) {
                 ignoreMalformed.setValue(false);
             }
-            return new DateFieldMapper(name(), ft, multiFieldsBuilder.build(this, context), copyTo, nullTimestamp, resolution, this);
+            return new DateFieldMapper(
+                name(),
+                ft,
+                multiFieldsBuilder.build(this, context),
+                copyTo,
+                nullTimestamp,
+                resolution,
+                context.isSourceSynthetic(),
+                this
+            );
         }
     }
 
@@ -850,6 +859,7 @@ public DocValueFormat docValueFormat(@Nullable String format, ZoneId timeZone) {
     private final Long nullValue;
     private final String nullValueAsString;
     private final Resolution resolution;
+    private final boolean isSourceSynthetic;
 
     private final boolean ignoreMalformedByDefault;
     private final IndexVersion indexCreatedVersion;
@@ -865,6 +875,7 @@ private DateFieldMapper(
         CopyTo copyTo,
         Long nullValue,
         Resolution resolution,
+        boolean isSourceSynthetic,
         Builder builder
     ) {
         super(simpleName, mappedFieldType, multiFields, copyTo, builder.script.get() != null, builder.onScriptError.get());
@@ -877,6 +888,7 @@ private DateFieldMapper(
         this.nullValueAsString = builder.nullValue.getValue();
         this.nullValue = nullValue;
         this.resolution = resolution;
+        this.isSourceSynthetic = isSourceSynthetic;
         this.ignoreMalformedByDefault = builder.ignoreMalformed.getDefaultValue();
         this.indexCreatedVersion = builder.indexCreatedVersion;
         this.script = builder.script.get();
@@ -915,6 +927,10 @@ protected void parseCreateField(DocumentParserContext context) throws IOExceptio
             } catch (IllegalArgumentException | ElasticsearchParseException | DateTimeException | ArithmeticException e) {
                 if (ignoreMalformed) {
                     context.addIgnoredField(mappedFieldType.name());
+                    if (isSourceSynthetic) {
+                        // Save a copy of the field so synthetic source can load it
+                        context.doc().add(IgnoreMalformedStoredValues.storedField(name(), context.parser()));
+                    }
                     return;
                 } else {
                     throw e;
@@ -976,11 +992,6 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
                 "field [" + name() + "] of type [" + typeName() + "] doesn't support synthetic source because it doesn't have doc values"
             );
         }
-        if (ignoreMalformed) {
-            throw new IllegalArgumentException(
-                "field [" + name() + "] of type [" + typeName() + "] doesn't support synthetic source because it ignores malformed dates"
-            );
-        }
         if (copyTo.copyToFields().isEmpty() != true) {
             throw new IllegalArgumentException(
                 "field [" + name() + "] of type [" + typeName() + "] doesn't support synthetic source because it declares copy_to"
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java
@@ -15,7 +15,6 @@
 import org.elasticsearch.common.time.DateUtils;
 import org.elasticsearch.core.CheckedConsumer;
 import org.elasticsearch.core.Strings;
-import org.elasticsearch.core.Tuple;
 import org.elasticsearch.index.IndexVersion;
 import org.elasticsearch.index.IndexVersions;
 import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType;
@@ -35,6 +34,7 @@
 import java.util.Comparator;
 import java.util.List;
 import java.util.function.Function;
+import java.util.stream.Stream;
 
 import static org.elasticsearch.index.mapper.DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER;
 import static org.hamcrest.Matchers.containsString;
@@ -152,7 +152,13 @@ protected List<ExampleMalformedValue> exampleMalformedValues() {
         return List.of(
             exampleMalformedValue("2016-03-99").mapping(mappingWithFormat("strict_date_optional_time||epoch_millis"))
                 .errorMatches("failed to parse date field [2016-03-99] with format [strict_date_optional_time||epoch_millis]"),
-            exampleMalformedValue("-522000000").mapping(mappingWithFormat("date_optional_time")).errorMatches("long overflow")
+            exampleMalformedValue("-522000000").mapping(mappingWithFormat("date_optional_time")).errorMatches("long overflow"),
+            exampleMalformedValue("2020").mapping(mappingWithFormat("strict_date"))
+                .errorMatches("failed to parse date field [2020] with format [strict_date]"),
+            exampleMalformedValue("hello world").mapping(mappingWithFormat("strict_date_optional_time"))
+                .errorMatches("failed to parse date field [hello world]"),
+            exampleMalformedValue("true").mapping(mappingWithFormat("strict_date_optional_time"))
+                .errorMatches("failed to parse date field [true]")
         );
     }
 
@@ -561,7 +567,6 @@ public void testScriptAndPrecludedParameters() {
 
     @Override
     protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed) {
-        assumeFalse("synthetic _source for date and date_millis doesn't support ignore_malformed", ignoreMalformed);
         return new SyntheticSourceSupport() {
             private final DateFieldMapper.Resolution resolution = randomFrom(DateFieldMapper.Resolution.values());
             private final Object nullValue = usually()
@@ -577,36 +582,62 @@ protected SyntheticSourceSupport syntheticSourceSupport(boolean ignoreMalformed)
             @Override
             public SyntheticSourceExample example(int maxValues) {
                 if (randomBoolean()) {
-                    Tuple<Object, String> v = generateValue();
+                    Value v = generateValue();
+                    if (v.malformedOutput != null) {
+                        return new SyntheticSourceExample(v.input, v.malformedOutput, null, this::mapping);
+                    }
+
                     return new SyntheticSourceExample(
-                        v.v1(),
-                        v.v2(),
-                        resolution.convert(Instant.from(formatter.parse(v.v2()))),
+                        v.input,
+                        v.output,
+                        resolution.convert(Instant.from(formatter.parse(v.output))),
                         this::mapping
                     );
                 }
-                List<Tuple<Object, String>> values = randomList(1, maxValues, this::generateValue);
-                List<Object> in = values.stream().map(Tuple::v1).toList();
-                List<String> outList = values.stream()
+
+                List<Value> values = randomList(1, maxValues, this::generateValue);
+                List<Object> in = values.stream().map(Value::input).toList();
+
+                List<String> outputFromDocValues = values.stream()
+                    .filter(v -> v.malformedOutput == null)
                     .sorted(
-                        Comparator.comparing(v -> Instant.from(formatter.parse(v.v1() == null ? nullValue.toString() : v.v1().toString())))
+                        Comparator.comparing(
+                            v -> Instant.from(formatter.parse(v.input == null ? nullValue.toString() : v.input.toString()))
+                        )
                     )
-                    .map(Tuple::v2)
+                    .map(Value::output)
                     .toList();
+
+                Stream<Object> malformedOutput = values.stream().filter(v -> v.malformedOutput != null).map(Value::malformedOutput);
+
+                // Malformed values are always last in the implementation.
+                List<Object> outList = Stream.concat(outputFromDocValues.stream(), malformedOutput).toList();
                 Object out = outList.size() == 1 ? outList.get(0) : outList;
 
-                List<Long> outBlockList = outList.stream().map(v -> resolution.convert(Instant.from(formatter.parse(v)))).toList();
+                List<Long> outBlockList = outputFromDocValues.stream()
+                    .map(v -> resolution.convert(Instant.from(formatter.parse(v))))
+                    .toList();
                 Object outBlock = outBlockList.size() == 1 ? outBlockList.get(0) : outBlockList;
                 return new SyntheticSourceExample(in, out, outBlock, this::mapping);
             }
 
-            private Tuple<Object, String> generateValue() {
+            private record Value(Object input, String output, Object malformedOutput) {}
+
+            private Value generateValue() {
                 if (nullValue != null && randomBoolean()) {
-                    return Tuple.tuple(null, outValue(nullValue));
+                    return new Value(null, outValue(nullValue), null);
+                }
+                // Different malformed values are tested in #exampleMalformedValues().
+                // Here we only verify behavior of arrays that contain malformed
+                // values since there are modifications specific to synthetic source.
+                if (ignoreMalformed && randomBoolean()) {
+                    var malformedInput = randomAlphaOfLengthBetween(1, 10);
+                    return new Value(malformedInput, null, malformedInput);
                 }
+
                 Object in = randomValue();
                 String out = outValue(in);
-                return Tuple.tuple(in, out);
+                return new Value(in, out, null);
             }
 
             private Object randomValue() {
@@ -637,6 +668,9 @@ private void mapping(XContentBuilder b) throws IOException {
                 if (nullValue != null) {
                     b.field("null_value", nullValue);
                 }
+                if (ignoreMalformed) {
+                    b.field("ignore_malformed", true);
+                }
             }
 
             @Override
@@ -653,16 +687,6 @@ public List<SyntheticSourceInvalidExample> invalidExample() throws IOException {
                             b -> b.field("type", fieldType).field("doc_values", false)
                         )
                     );
-                    examples.add(
-                        new SyntheticSourceInvalidExample(
-                            equalTo(
-                                "field [field] of type ["
-                                    + fieldType
-                                    + "] doesn't support synthetic source because it ignores malformed dates"
-                            ),
-                            b -> b.field("type", fieldType).field("ignore_malformed", true)
-                        )
-                    );
                 }
                 return examples;
             }
diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java
@@ -1129,8 +1129,16 @@ public final void testSyntheticSource() throws IOException {
 
     public void testSyntheticSourceIgnoreMalformedExamples() throws IOException {
         assumeTrue("type doesn't support ignore_malformed", supportsIgnoreMalformed());
-        CheckedConsumer<XContentBuilder, IOException> mapping = syntheticSourceSupport(true).example(1).mapping();
+        // We need to call this in order to hit the assumption inside so that
+        // it tells us when field supports ignore_malformed but doesn't support it together with synthetic source.
+        // E.g. `assumeFalse(ignoreMalformed)`
+        syntheticSourceSupport(true);
+
         for (ExampleMalformedValue v : exampleMalformedValues()) {
+            CheckedConsumer<XContentBuilder, IOException> mapping = b -> {
+                v.mapping.accept(b);
+                b.field("ignore_malformed", true);
+            };
             assertSyntheticSource(new SyntheticSourceExample(v.value, v.value, v.value, mapping));
         }
     }
diff --git a/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java b/x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
