Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in date parsing ("2016-11-30T12+01" accepted before ES 7 #49351

Closed
jamshid opened this issue Nov 20, 2019 · 6 comments · Fixed by #50178
Closed

Regression in date parsing ("2016-11-30T12+01" accepted before ES 7 #49351

jamshid opened this issue Nov 20, 2019 · 6 comments · Fixed by #50178
Labels
:Core/Infra/Core Core issues without another label

Comments

@jamshid
Copy link

jamshid commented Nov 20, 2019

Elasticsearch version (bin/elasticsearch --version):

5.6.12 vs 74.2

Plugins installed: []
unrelated

JVM version (java -version):
openjdk version "1.8.0_232"

OS version (uname -a if on a Unix-like system):
centos8 container
Linux 044afd19e126 4.9.184-linuxkit #1 SMP Tue Jul 2 22:58:16 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

Description of the problem including expected versus actual behavior:

Steps to reproduce:
This admittedly weird, incomplete date time string with a timezone offset:
2016-11-30T12+01
is accepted as a date by ES 5.6.12 but fails on ES 7.4.2 (probably any 7.x?) with:

HTTP/1.1 400 Bad Request

      "type" : "illegal_argument_exception",
      "reason" : "failed to parse date field [2016-11-30T12+01] with format [strict_date_optional_time||epoch_millis]",
curl -X PUT "elasticsearch:9200/my_index?pretty" -H 'Content-Type: application/json' --data-binary '{
  "mappings": {
    "doc": {
      "properties": {
        "date": {
          "type":   "date"
        }
      }
    }
  }
}'

curl -i -XPUT -H 'Content-type: application/json' http://elasticsearch:9200/my_index/doc/1 --data-binary  '{"date": "2016-11-30T12+01"}'
HTTP/1.1 201 Created
curl -X PUT "elasticsearch:9200/my_index?include_type_name=true&pretty" -H 'Content-Type: application/json' --data-binary '{            
"mappings": {
   "doc" : {
      "properties": {
        "date": {
          "type":   "date"
        }
      }
    }
  }
}'

curl -i -XPUT -H 'Content-type: application/json' http://elasticsearch:9200/my_index/doc/1?pretty --data-binary  '{"date": "2016-11-30T12+01"}'

HTTP/1.1 400 Bad Request
Warning: 299 Elasticsearch-7.4.2-2f90bbf7b93631e52bafb59b3b049cb44ec25e96 "[types removal] Specifying types in document index requests is deprecated, use the typeless endpoints instead (/{index}/_doc/{id}, /{index}/_doc, or /{index}/_create/{id})."
content-type: application/json; charset=UTF-8
content-length: 767

{
  "error" : {
    "root_cause" : [
      {
        "type" : "mapper_parsing_exception",
        "reason" : "failed to parse field [date] of type [date] in document with id '1'. Preview of field's value: '2016-11-30T12+01'"
      }
    ],
    "type" : "mapper_parsing_exception",
    "reason" : "failed to parse field [date] of type [date] in document with id '1'. Preview of field's value: '2016-11-30T12+01'",
    "caused_by" : {
      "type" : "illegal_argument_exception",
      "reason" : "failed to parse date field [2016-11-30T12+01] with format [strict_date_optional_time||epoch_millis]",
      "caused_by" : {
        "type" : "date_time_parse_exception",
        "reason" : "Failed to parse with all enclosed parsers"
      }
    }
  },
  "status" : 400
}

Btw same behavior if the strict_date_optional_time is used explicitly in the mapping, it's the default in both elasticsearch versions.

          "format": "strict_date_optional_time||epoch_millis"

Provide logs (if relevant):

See output above, it just seems like a change in what the date parser accepts. Can that be fixed or is this by design?

Besides the regression and incompatibility this introduces for users upgrading, can this cause any in-place upgrade problems with ES 5 => 7?

@cbuescher
Copy link
Member

can this cause any in-place upgrade problems with ES 5 => 7

If by "in-place" you mean rolling upgrades, this isn't supported directly from any 5 to 7 version. See https://www.elastic.co/guide/en/elasticsearch/reference/current/setup-upgrade.html for reference.
As you noticed, the date 2016-11-30T12+01 isn't complete and therefore should be rejected when parsed as "strict_date_optional_time". The fact that it was accepted in 5 should be considered a bug and the current behaviour as correct. I hope you don't mind me closing this issue. If you have further questions around upgrades I'd like to point you to the support forums over at https://discuss.elastic.co since we prefer to use Github issues only for bug reports and feature requests.

@jamshid
Copy link
Author

jamshid commented Nov 21, 2019

Okay thanks, I guess closing is fine but fwiw {"date": "2016-11-30T12"} is accepted.
ES 7 changed to no longer allow this partial time iff a time zone is specified.

That seems inconsistent. Minutes and seconds are optional in ISO 8601, and I believe a time zone is allowed.

@pgomulka
Copy link
Contributor

@jamshid @cbismuth this is a problem that was raised recently by other users too. We accepted this as a regression bug and fixed in ES v7.5 (which is not released as of yet)
#46814

@jamshid
Copy link
Author

jamshid commented Dec 11, 2019

@pgomulka this regression does not appear fixed in v7.5.0. Any chance it will be fixed?

docker run -d -p 9200:9200 -p 9300:9300 -e "ES_JAVA_OPTS=-Xms512m -Xmx512m" -e "discovery.type=single-node" docker.elastic.co/elasticsearch/elasticsearch:7.5.0

curl -X PUT "ELASTICSEARCH:9200/my_index?pretty" -H 'Content-Type: application/json' --data-binary '{
  "mappings": {
      "properties": {
        "date": {
          "type":   "date"
        }
      }
  }
}'

curl -i -XPUT -H 'Content-type: application/json' http://ELASTICSEARCH:9200/my_index/_doc/1 --data-binary  '{"date": "2016-11-30T12+01"}'

HTTP/1.1 400 Bad Request
content-type: application/json; charset=UTF-8
content-length: 767

{
  "error" : {
    "root_cause" : [
      {
        "type" : "mapper_parsing_exception",
        "reason" : "failed to parse field [date] of type [date] in document with id '1'. Preview of field's value: '2016-11-30T12+01'"
      }
    ],
    "type" : "mapper_parsing_exception",
    "reason" : "failed to parse field [date] of type [date] in document with id '1'. Preview of field's value: '2016-11-30T12+01'",
    "caused_by" : {
      "type" : "illegal_argument_exception",
      "reason" : "failed to parse date field [2016-11-30T12+01] with format [strict_date_optional_time||epoch_millis]",
      "caused_by" : {
        "type" : "date_time_parse_exception",
        "reason" : "Failed to parse with all enclosed parsers"
      }
    }
  },
  "status" : 400
}

@pgomulka
Copy link
Contributor

@jamshid that looks to be broken, I will have a look

@elasticmachine
Copy link
Collaborator

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

pgomulka added a commit that referenced this issue Jan 8, 2020
strict_date_optional_time changes to have optional minute part. It already allowed optional second and fraction of second part. This allows parsing 2018-01-01T00+01 , 2018-01-01T00:00+01 , 2018-01-01T00:00:00+01 , 2018-01-01T00:00:00.000+01
It won't allow parsing a timezone without an hour part as this is not allowed by iso8601 spec

closes #49351
pgomulka added a commit to pgomulka/elasticsearch that referenced this issue Jan 8, 2020
strict_date_optional_time changes to have optional minute part.
It already allowed optional second and fraction of second part.
This allows parsing 2018-01-01T00+01 , 2018-01-01T00:00+01 , 2018-01-01T00:00:00+01 , 2018-01-01T00:00:00.000+01
It won't allow parsing a timezone without an hour part as this is not allowed by iso8601 spec

closes elastic#49351
pgomulka added a commit that referenced this issue Jan 8, 2020
…50740)

strict_date_optional_time changes to have optional minute part.
It already allowed optional second and fraction of second part.
This allows parsing 2018-01-01T00+01 , 2018-01-01T00:00+01 , 2018-01-01T00:00:00+01 , 2018-01-01T00:00:00.000+01
It won't allow parsing a timezone without an hour part as this is not allowed by iso8601 spec

closes #49351
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this issue Jan 23, 2020
strict_date_optional_time changes to have optional minute part. It already allowed optional second and fraction of second part. This allows parsing 2018-01-01T00+01 , 2018-01-01T00:00+01 , 2018-01-01T00:00:00+01 , 2018-01-01T00:00:00.000+01
It won't allow parsing a timezone without an hour part as this is not allowed by iso8601 spec

closes elastic#49351
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants