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

Negative seconds of epoch are not mapped correctly to dates for values <=-1000, and big negative values throw an unexpected exception. #72123

Closed
alk-gmathews opened this issue Apr 22, 2021 · 5 comments
Labels
>bug :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team

Comments

@alk-gmathews
Copy link

Elasticsearch version (bin/elasticsearch --version):
Version: 7.12.0, Build: default/tar/78722783c38caa25a70982b5b042074cde5d3b3a/2021-03-18T06:17:15.410153305Z, JVM: 15.0.1

Plugins installed: []

JVM version (java -version):
openjdk version "15.0.1" 2020-10-20
OpenJDK Runtime Environment (build 15.0.1+9)
OpenJDK 64-Bit Server VM (build 15.0.1+9, mixed mode, sharing)

OS version (uname -a if on a Unix-like system):
Darwin Georges-MacBook-Pro.local 19.6.0 Darwin Kernel Version 19.6.0: Tue Jan 12 22:13:05 PST 2021; root:xnu-6153.141.16~1/RELEASE_X86_64 x86_64

Description of the problem including expected versus actual behavior:
Dates are not mapped correctly for seconds values having values less than -999.
e.g. for the date in seconds value -1000 the expected mapped date is 1969-12-31T23:43:20.000Z but the result I get is -1000-01-01T00:00:00.000Z

When I try with seconds value of -9999999999, I get this error:

{
  "error": {
    "root_cause": [
      {
        "type": "mapper_parsing_exception",
        "reason": "failed to parse field [date] of type [date] in document with id 'expect_999999999_second_before_1970'. Preview of field's value: '-9999999999'"
      }
    ],
    "type": "mapper_parsing_exception",
    "reason": "failed to parse field [date] of type [date] in document with id 'expect_999999999_second_before_1970'. Preview of field's value: '-9999999999'",
    "caused_by": {
      "type": "date_time_exception",
      "reason": "Invalid value for Year (valid values -999999999 - 999999999): -9999999999"
    }
  },
  "status": 400
}

Steps to reproduce:

  1. Create mappings
curl --request PUT \
  --url 'http://localhost:9200/my-datetime-index?pretty=' \
  --header 'Content-Type: application/json' \
  --data '{
  "mappings": {
    "properties": {
      "date": {
        "type":   "date",
        "format": "strict_date_optional_time||epoch_second"
      }
    }
  }
}'
  1. Bulk Add Dates
curl --request POST \
 --url 'http://localhost:9200/_bulk?refresh=&pretty=' \
 --header 'Content-Type: application/json' \
 --data '{ "create" : { "_index" : "my-datetime-index", "_id" : "expect_one_second_after_1970"}}
{"date": 1}
{ "create" : { "_index" : "my-datetime-index", "_id" : "expect_2021/04/01"}}
{ "date": 1617235200}
{ "create" : { "_index" : "my-datetime-index", "_id" : "expect_1_second_before_1970"}}
{ "date": -1}
{ "create" : { "_index" : "my-datetime-index", "_id" : "expect_10_second_before_1970"}}
{ "date": -10}
{ "create" : { "_index" : "my-datetime-index", "_id" : "expect_100_second_before_1970"}}
{ "date": -100}
{ "create" : { "_index" : "my-datetime-index", "_id" : "expect_999_second_before_1970"}}
{ "date": -999}
{ "create" : { "_index" : "my-datetime-index", "_id" : "expect_1000_second_before_1970"}}
{ "date": -1000}

'
  1. Fetch Results of Bulk Additions in Step-2. Note that -1000 is incorrectly mapped as "-1000-01-01T00:00:00.000Z"
curl --request POST \
 --url 'http://localhost:9200/my-datetime-index/_search?pretty=' \
 --header 'Content-Type: application/json' \
 --data '{
 "fields": [ {"field": "date"}],
 "_source": false
}'

Result Seen:

{
  "took": 2,
  "timed_out": false,
  "_shards": {
    "total": 1,
    "successful": 1,
    "skipped": 0,
    "failed": 0
  },
  "hits": {
    "total": {
      "value": 7,
      "relation": "eq"
    },
    "max_score": 1.0,
    "hits": [
      {
        "_index": "my-datetime-index",
        "_type": "_doc",
        "_id": "expect_one_second_after_1970",
        "_score": 1.0,
        "fields": {
          "date": [
            "1970-01-01T00:00:01.000Z"
          ]
        }
      },
      {
        "_index": "my-datetime-index",
        "_type": "_doc",
        "_id": "expect_2021/04/01",
        "_score": 1.0,
        "fields": {
          "date": [
            "2021-04-01T00:00:00.000Z"
          ]
        }
      },
      {
        "_index": "my-datetime-index",
        "_type": "_doc",
        "_id": "expect_1_second_before_1970",
        "_score": 1.0,
        "fields": {
          "date": [
            "1969-12-31T23:59:59.000Z"
          ]
        }
      },
      {
        "_index": "my-datetime-index",
        "_type": "_doc",
        "_id": "expect_10_second_before_1970",
        "_score": 1.0,
        "fields": {
          "date": [
            "1969-12-31T23:59:50.000Z"
          ]
        }
      },
      {
        "_index": "my-datetime-index",
        "_type": "_doc",
        "_id": "expect_100_second_before_1970",
        "_score": 1.0,
        "fields": {
          "date": [
            "1969-12-31T23:58:20.000Z"
          ]
        }
      },
      {
        "_index": "my-datetime-index",
        "_type": "_doc",
        "_id": "expect_999_second_before_1970",
        "_score": 1.0,
        "fields": {
          "date": [
            "1969-12-31T23:43:21.000Z"
          ]
        }
      },
      {
        "_index": "my-datetime-index",
        "_type": "_doc",
        "_id": "expect_1000_second_before_1970",
        "_score": 1.0,
        "fields": {
          "date": [
            "-1000-01-01T00:00:00.000Z"
          ]
        }
      }
    ]
  }
}
  1. Add Big Negative Seconds -9999999999 expecting Monday, February 10, 1653 6:13:21 AM
curl --request PUT \
  --url 'http://localhost:9200/my-datetime-index/_doc/expect_9999999999_second_before_1970?refresh=&pretty=' \
  --header 'Content-Type: application/json' \
  --data '{"date": -9999999999}
'

Result Seen:

{
  "error": {
    "root_cause": [
      {
        "type": "mapper_parsing_exception",
        "reason": "failed to parse field [date] of type [date] in document with id 'expect_9999999999_second_before_1970'. Preview of field's value: '-9999999999'"
      }
    ],
    "type": "mapper_parsing_exception",
    "reason": "failed to parse field [date] of type [date] in document with id 'expect_9999999999_second_before_1970'. Preview of field's value: '-9999999999'",
    "caused_by": {
      "type": "date_time_exception",
      "reason": "Invalid value for Year (valid values -999999999 - 999999999): -9999999999"
    }
  },
  "status": 400
}

Provide logs (if relevant):

@alk-gmathews alk-gmathews added >bug needs:triage Requires assignment of a team area label labels Apr 22, 2021
@alk-gmathews alk-gmathews changed the title Negative seconds are not mapped correctly to dates for values <=-1000, and big negative values throw an unexpected exception. Negative seconds of epoch are not mapped correctly to dates for values <=-1000, and big negative values throw an unexpected exception. Apr 22, 2021
@jtibshirani jtibshirani added :Core/Infra/Core Core issues without another label and removed needs:triage Requires assignment of a team area label labels Apr 22, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Apr 22, 2021
@elasticmachine
Copy link
Collaborator

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

@rjernst
Copy link
Member

rjernst commented Apr 23, 2021

Negative epoch times were deprecated in 6.7 (#36793) and removed in 7.0. If you are curious why, I recommend reading #40983 which had a long discussion as to the underlying reasons.

So this is working as intended. The number you are passing in is considered part of the year of the date, since no other parts exist. Please convert your date from epoch time to another date format for years prior to 1970.

@rjernst rjernst closed this as completed Apr 23, 2021
@alk-gmathews
Copy link
Author

alk-gmathews commented Apr 23, 2021

@rjernst I read your final comments on the issue regarding java instants. As per the explanation, the argument makes sense for milliseconds but not for seconds. And the other argument, "we also think negative epoch values in general are uncommon" , it seems people before me have raised concerns about this as well. Either way, considering the detailed discussion on the thread I imagine that further discussion on this is unnecessary. If anything, the precision issue could have been handled better by letting the user make the decision on it instead of deprecating the possibility altogether.

The document needs to be updated for the date type and the format parameter since it doesn't indicate that negative seconds values are invalid:
https://www.elastic.co/guide/en/elasticsearch/reference/current/date.html
https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-date-format.html

On the other hand, could you give me an idea on the repercussions of using negative integer values in epoch seconds with date? In my current tests I seem to have no issue if I use epoch_second, or epoch_second||strict_date_optional_time.

Additionally, in a future release will you reject all negative epoch values with a proper exception versus the current situation where it works for some cases?

@rjernst
Copy link
Member

rjernst commented May 5, 2021

Apologies @alk-gmathews , I was incorrect in my assertion before. We did deprecate and remove negative value support for epoch_millis, but apparent we did not for epoch_second. We may want to get those formats back in sync in the future, so I would still recommend using a structured date format like ISO8601 for dates before epoch, but negative values for epoch_second are not deprecated and we don't have any plans to deprecate it at this time.

Looking back, the problem you had is a common one. Since the strict_date_optional_time appears first in the formats list, the negative value is parsed there as a year. As you said in your last reply above, swapping the order of the formats to use epoch_second first works. This isn't a bug in the epoch time parser: it never sees the value. It's simply how the compound format specifications work in Elasticsearch: the first one that parses correctly wins. The naming of strict_date_optional_time may be confusing (it was picked up from Joda terminology long ago). "strict" here has nothing to do with requiring a full date. Instead, only a year is required to match, so putting the epoch parser first is advised in this case.

@alk-gmathews
Copy link
Author

@rjernst thanks for the update. This is actually really good news since we can proceed with updating our mappings, which is a medium-sized endeavor but still simpler to execute, instead of the other options we were considering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

No branches or pull requests

4 participants