Core: Revert back to joda's multi date formatters (#36814)

This commit partially reverts #36447 by using the ability of Joda time's DateTimeFormatterBuilder to append multiple parsers instead of using the MergedDateFormatter. The MergedDateFormatter will be removed in a future change, as it is not as performant due to creating potentially many exceptions during heavy date parsing. This change is a stop-gap until that followup is ready. closes #36602
elastic · Dec 19, 2018 · 439630f · 439630f
1 parent 9031f8c
commit 439630f
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 15 deletions.
diff --git a/server/src/main/java/org/elasticsearch/common/joda/Joda.java b/server/src/main/java/org/elasticsearch/common/joda/Joda.java
@@ -232,6 +232,27 @@ public static JodaDateFormatter forPattern(String input) {
             formatter = StrictISODateTimeFormat.yearMonth();
         } else if ("strictYearMonthDay".equals(input) || "strict_year_month_day".equals(input)) {
             formatter = StrictISODateTimeFormat.yearMonthDay();
+        } else if (Strings.hasLength(input) && input.contains("||")) {
+            String[] formats = Strings.delimitedListToStringArray(input, "||");
+            DateTimeParser[] parsers = new DateTimeParser[formats.length];
+
+            if (formats.length == 1) {
+                formatter = forPattern(input).parser;
+            } else {
+                DateTimeFormatter dateTimeFormatter = null;
+                for (int i = 0; i < formats.length; i++) {
+                    JodaDateFormatter currentFormatter = forPattern(formats[i]);
+                    DateTimeFormatter currentParser = currentFormatter.parser;
+                    if (dateTimeFormatter == null) {
+                        dateTimeFormatter = currentFormatter.printer;
+                    }
+                    parsers[i] = currentParser.getParser();
+                }
+
+                DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder()
+                    .append(dateTimeFormatter.withZone(DateTimeZone.UTC).getPrinter(), parsers);
+                formatter = builder.toFormatter();
+            }
         } else {
             try {
                 maybeLogJodaDeprecation(input);

diff --git a/server/src/main/java/org/elasticsearch/common/time/DateFormatter.java b/server/src/main/java/org/elasticsearch/common/time/DateFormatter.java
@@ -129,19 +129,17 @@ static DateFormatter forPattern(String input) {
         if (Strings.hasLength(input) == false) {
             throw new IllegalArgumentException("No date pattern provided");
         }
+        if (input.startsWith("8") == false) {
+            return Joda.forPattern(input);
+        }
+
+        // force java 8 date format
         List<DateFormatter> formatters = new ArrayList<>();
-        for (String pattern : Strings.delimitedListToStringArray(input, "||")) {
-            if (Strings.hasLength(input) == false) {
+        for (String pattern : Strings.delimitedListToStringArray(input.substring(1), "||")) {
+            if (Strings.hasLength(pattern) == false) {
                 throw new IllegalArgumentException("Cannot have empty element in multi date format pattern: " + input);
             }
-            final DateFormatter formatter;
-            if (pattern.startsWith("8")) {
-                // force java 8 date format
-                formatter = DateFormatters.forPattern(pattern.substring(1));
-            } else {
-                formatter = Joda.forPattern(pattern);
-            }
-            formatters.add(formatter);
+            formatters.add(DateFormatters.forPattern(pattern));
         }
 
         if (formatters.size() == 1) {

diff --git a/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java b/server/src/test/java/org/elasticsearch/common/joda/JavaJodaTimeDuellingTests.java
@@ -471,7 +471,7 @@ public void testSamePrinterOutput() {
 
     public void testSeveralTimeFormats() {
         DateFormatter jodaFormatter = DateFormatter.forPattern("year_month_day||ordinal_date");
-        DateFormatter javaFormatter = DateFormatter.forPattern("8year_month_day||8ordinal_date");
+        DateFormatter javaFormatter = DateFormatter.forPattern("8year_month_day||ordinal_date");
         assertSameDate("2018-12-12", "year_month_day||ordinal_date", jodaFormatter, javaFormatter);
         assertSameDate("2018-128", "year_month_day||ordinal_date", jodaFormatter, javaFormatter);
     }

diff --git a/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java b/server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java
@@ -19,7 +19,6 @@
 
 package org.elasticsearch.common.time;
 
-import org.elasticsearch.common.joda.JodaDateFormatter;
 import org.elasticsearch.test.ESTestCase;
 
 import java.time.Instant;
@@ -204,6 +203,6 @@ public void testForceJava8() {
         assertThat(formatter, instanceOf(DateFormatters.MergedDateFormatter.class));
         DateFormatters.MergedDateFormatter mergedFormatter = (DateFormatters.MergedDateFormatter) formatter;
         assertThat(mergedFormatter.formatters.get(0), instanceOf(JavaDateFormatter.class));
-        assertThat(mergedFormatter.formatters.get(1), instanceOf(JodaDateFormatter.class));
+        assertThat(mergedFormatter.formatters.get(1), instanceOf(JavaDateFormatter.class));
     }
 }
diff --git a/server/src/test/java/org/elasticsearch/common/time/JavaDateMathParserTests.java b/server/src/test/java/org/elasticsearch/common/time/JavaDateMathParserTests.java
@@ -239,7 +239,7 @@ public void testTimestamps() {
         assertDateMathEquals("1418248078000||/m", "2014-12-10T21:47:00.000");
 
         // also check other time units
-        DateMathParser parser = DateFormatter.forPattern("8epoch_second||8dateOptionalTime").toDateMathParser();
+        DateMathParser parser = DateFormatter.forPattern("8epoch_second||dateOptionalTime").toDateMathParser();
         long datetime = parser.parse("1418248078", () -> 0);
         assertDateEquals(datetime, "1418248078", "2014-12-10T21:47:58.000");
 

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java
@@ -174,7 +174,7 @@ public void testIgnoreMalformed() throws Exception {
                         .endObject()),
                 XContentType.JSON));
         MapperParsingException e = expectThrows(MapperParsingException.class, runnable);
-        assertThat(e.getCause().getMessage(), containsString("failed to parse date field [2016-03-99]"));
+        assertThat(e.getCause().getMessage(), containsString("Cannot parse \"2016-03-99\""));
 
         mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type")
                 .startObject("properties").startObject("field").field("type", "date")