Multiple date formats in date_histogram aggregation

[Mpdreamz]

The following worked in 6.x

"date_histogram": {
    "extended_bounds": {
        "max": "2016-06-06T12:01:02.123",
        "min": "2014-06-06T12:01:02.123"
    },
    "field": "startedOn",
    "format": "yyyy-MM-dd'T'HH:mm:ss||date_optional_time",
    "interval": "month",
    "min_doc_count": 2,
    "missing": "2015-06-06T12:01:02.123",
    "order": {
         "_count": "asc"
    }
}

In 7.0.0-beta1 it fails with the following:

{
  "error" : {
    "root_cause" : [
      {
        "type" : "parse_exception",
        "reason" : "failed to parse date field [2014-06-06T12:01:02.123] with format [yyyy-MM-dd'T'HH:mm:ss||date_optional_time]: [Text '2014-06-06T12:01:02.123' could not be parsed, unparsed text found at index 19]",
        "stack_trace" : "ElasticsearchParseException[failed to parse date field [2014-06-06T12:01:02.123] with format [yyyy-MM-dd'T'HH:mm:ss||date_optional_time]: [Text '2014-06-06T12:01:02.123' could not be parsed, unparsed text found at index 19]]; nested: DateTimeParseException[Text '2014-06-06T12:01:02.123' could not be parsed, unparsed text found at index 19];\r\n\tat org.elasticsearch.common.time.JavaDateMathParser.parseDateTime(JavaDateMathParser.java:233)\r\n\tat org.elasticsearch.common.time.JavaDateMathParser.parse(JavaDateMathParser.java:75)\r\n\tat org.elasticsearch.search.DocValueFormat$DateTime.parseLong(DocValueFormat.java:229)\r\n\tat org.elasticsearch.search.aggregations.bucket.histogram.ExtendedBounds.parseAndValidate(ExtendedBounds.java:156)\r\n\tat org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder.innerBuild(DateHistogramAggregationBuilder.java:455)\r\n\tat org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder.doBuild(ValuesSourceAggregationBuilder.java:315)\r\n\tat org.elasticsearch.search.aggregations.support.ValuesSourceAggregationBuilder.doBuild(ValuesSourceAggregationBuilder.java:39)\r\n\tat org.elasticsearch.search.aggregations.AbstractAggregationBuilder.build(AbstractAggregationBuilder.java:139)\r\n\tat org.elasticsearch.search.aggregations.AggregatorFactories$Builder.build(AggregatorFactories.java:333)\r\n\tat org.elasticsearch.search.SearchService.parseSource(SearchService.java:800)\r\n\tat org.elasticsearch.search.SearchService.createContext(SearchService.java:607)\r\n\tat org.elasticsearch.search.SearchService.createAndPutContext(SearchService.java:582)\r\n\tat org.elasticsearch.search.SearchService.executeQueryPhase(SearchService.java:385)\r\n\tat org.elasticsearch.search.SearchService.access$100(SearchService.java:123)\r\n\tat org.elasticsearch.search.SearchService$2.onResponse(SearchService.java:357)\r\n\tat org.elasticsearch.search.SearchService$2.onResponse(SearchService.java:353)\r\n\tat org.elasticsearch.search.SearchService$4.doRun(SearchService.java:1068)\r\n\tat org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37)\r\n\tat org.elasticsearch.common.util.concurrent.TimedRunnable.doRun(TimedRunnable.java:41)\r\n\tat org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:751)\r\n\tat org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:37)\r\n\tat java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)\r\n\tat java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)\r\n\tat java.base/java.lang.Thread.run(Thread.java:834)\r\nCaused by: java.time.format.DateTimeParseException: Text '2014-06-06T12:01:02.123' could not be parsed, unparsed text found at index 19\r\n\tat java.base/java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:2049)\r\n\tat java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1874)\r\n\tat org.elasticsearch.common.time.JavaDateMathParser.parseDateTime(JavaDateMathParser.java:223)\r\n\t... 23 more\r\n"
      }
    ],

Specifying only

    "format": "date_optional_time",

Results in no error however.

[elasticmachine]

Pinging @elastic/es-analytics-geo

[polyfractal]

This appears to be a general problem with multiple formats, likely introduced/broken in #37222

We're using appendOptional() to append multiple patterns, but I don't think that's the correct method. My understanding is that appendOptional is for truly optional components in the chain. E.g. 2016-06-06 is mandatory, but 12:01:02.123 could be optional. Not for entirely different patterns.

I traced through the code and this does indeed appear to be the case. When DateTimeFormatterBuilder#parse() starts matching the input text, it matches the first pattern (yyyy-MM-dd'T'HH:mm:ss) up to position 19, which is the milliseconds portion. Then it tries the next pattern (date_optional_time) starting at position 19, which does not match so it aborts with "unfinished" characters that were not parsed.

@pgomulka @rjernst any ideas? This is hitting the edge of my java-time knowledge. I skimmed the API and I'm thinking we may need to drop back to an array of parsers, and check them individually with parseUnresolved() similar to this: https://stackoverflow.com/a/44625276?

[polyfractal]

It seems this affects anything using multiple patterns, most notably it breaks indexing:

PUT test
{
  "mappings": {
    "properties": {
      "date": {
        "type":   "date",
        "format": "yyyy-MM-dd'T'HH:mm:ss||date_optional_time"
      }
    }
  }
}

POST /test/_doc/
{
  "date": "2014-06-06T12:01:02.123"
}

{
  "error": {
    "root_cause": [
      {
        "type": "mapper_parsing_exception",
        "reason": "failed to parse field [date] of type [date] in document with id 'wcHYfWkBi3kUMDzPydwQ'"
      }
    ],
    "type": "mapper_parsing_exception",
    "reason": "failed to parse field [date] of type [date] in document with id 'wcHYfWkBi3kUMDzPydwQ'",
    "caused_by": {
      "type": "illegal_argument_exception",
      "reason": "failed to parse date field [2014-06-06T12:01:02.123] with format [yyyy-MM-dd'T'HH:mm:ss||date_optional_time]",
      "caused_by": {
        "type": "date_time_parse_exception",
        "reason": "Text '2014-06-06T12:01:02.123' could not be parsed, unparsed text found at index 19"
      }
    }
  },
  "status": 400
}

("2014-06-06T12:01:02.123" indexes correctly if just date_optional_time is specified)

EDIT:

Doesn't appear as bad as I thought. yyyy-MM-dd'T'HH:mm:ss||epoch_millis and similar patterns seem to work fine. But yyyy-MM-dd'T'HH:mm:ss||yyyy-MM-dd'T'HH:mm:ss.SSS for example fails, so there's something there about partially-matched patterns messing up the next pattern.

[pgomulka]

I think @polyfractal suggestion would work best.
I noticed that these would work (use it in JavaJodaTimeDuellingTests)

assertSameDate("2014-06-06T12:01:02.123", "strictDateTimeNoMillis||yyyy-MM-dd'T'HH:mm:ss.SSS")
 assertSameDate("2014-06-06T12:01:02.123", "yyyy-MM-dd'T'HH:mm:ss||yyyy-MM-dd'T'HH:mm:ss.SSS");

Couldn't find this in javadocs, but looks like DateTimeFormatter's patterns with optional depend on ordering. Someone raised a JDK bug, but it was closed as a non issue with comment:

The optional formats should be supplied longest to shortest, especially if the shorter format would match a prefix of the longer format. 

https://bugs.openjdk.java.net/browse/JDK-8188771

I don't think this is ideal, but we can workaround this with an approach from stackoverflow comment linked by @polyfractal

[elasticmachine]

Pinging @elastic/es-core-infra

[pgomulka]

the alternative to the PR above is just to state in a documentation that patterns have to be in order (longest to shortest / most strict/least strict)

[danielmitterdorfer]

the alternative to the PR above is just to state in a documentation that patterns have to be in order (longest to shortest / most strict/least strict)

You're right but I think that's too trappy.