Formats date_optional_time and dateOptionalTime are no longer interchangeable

[kstevens715]

Elasticsearch version (bin/elasticsearch --version): 7.7.1 and 7.6.2. I'm currently using Docker with image: docker.elastic.co/elasticsearch/elasticsearch:7.7.1

Plugins installed: []

JVM version (java -version): Returns nothing in Docker container.

OS version (uname -a if on a Unix-like system): Linux 1a0ec47b8a82 4.19.76-linuxkit #1 SMP Tue May 26 11:42:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Description of the problem including expected versus actual behavior:

You update a date format mapping from "dateOptionalTime" to "date_optional_time".

Expected: As in 7.5.2, the update is successful since both format strings are interchangeable.
Actual: In 7.6.2 and 7.7.1, an error is raised about "conflicts with existing mapping" (see below).

We always update mappings with "dateOptionalTime" but the mappings somehow got changed to "date_optional_time" recently (probably when upgrading from 7.5.2 -> 7.6.2). We ran into this error when updating mappings as we usually do.

Steps to reproduce:

curl -XDELETE localhost:9200/test
curl -XPUT localhost:9200/test
curl -XPUT localhost:9200/test/_mapping -H 'Content-Type: application/json' -d'{  "properties": {    "created_at": {      "type": "date",      "format": "dateOptionalTime"    }  }}'
curl -XPUT localhost:9200/test/_mapping -H 'Content-Type: application/json' -d'{  "properties": {    "created_at": {      "type": "date",      "format": "date_optional_time"    }  }}'

Error on last command:

{
  "error": {
    "root_cause": [
      {
        "type": "illegal_argument_exception",
        "reason": """Mapper for [created_at] conflicts with existing mapping:
[mapper [created_at] has different [format] values]"""
      }
    ],
    "type": "illegal_argument_exception",
    "reason": """Mapper for [created_at] conflicts with existing mapping:
[mapper [created_at] has different [format] values]"""
  },
  "status": 400
}

[elasticmachine]

Pinging @elastic/es-search (:Search/Mapping)

[elasticmachine]

Pinging @elastic/es-core-infra (:Core/Infra/Core)

[pgomulka]

Thank you for raising this @kstevens715

the behaviour for single formats like dateOptionalTime has changed because of #48703 . However it does behave the same way as previously for combined patterns as dateOptionalTime||strictDateOptionalTime which will not accept interchangeability between camel case and snake case.

I don't think interchangeability was intentional (just a side effect of mini optimisation in https://github.com/elastic/elasticsearch/blob/7.5/server/src/main/java/org/elasticsearch/common/time/DateFormatter.java#L146
You were creating pattern with dateOptionalTime but we were always converting this to date_optional_time . Upgrade did not change the mapping to snake case (it was always like this), it made that implementation details visible to you though.

I think we should fix this - allow interchangeability for both single and combined patterns.

I also think that there is little benefit with having patterns in both camel and snake case.
We should also review the patterns we have predefined and remove those which are rarely used. I believe that most of them could be defined as a non named pattern by a user and it would be actually more readable.

the problem with interchangeability for single formats does not exist in master as it was not affected by the fix #48703 (only merged in 7.x as it was aimed at '8' prefix issue). The problem exist for combined patterns

[rjernst]

I think we should fix this - allow interchangeability for both single and combined patterns.

This interchangeability was never documented. While I understand the frustration, I don't think we should add this back, as it was an implementation detail. Instead, we should just remove all the camelCase variants. I had thought we did this already, but realized that was for rest parameters, but we missed named date formats.

[pgomulka]

closed by #57878