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

Elasticsearch 7 reports a different key_as_string for dayOfWeek for date_histogram aggs than Elasticsearch 6 did #43275

Closed
centic9 opened this issue Jun 17, 2019 · 6 comments
Labels
>bug :Core/Infra/Core Core issues without another label

Comments

@centic9
Copy link
Contributor

centic9 commented Jun 17, 2019

Elasticsearch version (bin/elasticsearch --version): 6.7.2 and 7.1.1

Plugins installed: []

JVM version (java -version): Java HotSpot(TM) 64-Bit Server VM/11.0.1/11.0.1+13-LTS

OS version (uname -a if on a Unix-like system): Windows 10

Description of the problem including expected versus actual behavior:

We are looking at upgrading from Elasticsearch 6.x to 7.x and one of our tests related to day-of-week in date-histogramm aggregations is failing. When investigating a bit more we found that there is actually a different response for the "key_as_string" for buckets of a date_histogramm aggregation which aggregates on "day of week" in Elasticsearch 7 compared to Elasticsearch 6.

I could not find any "breaking change" mentioned for this, so I believe this may be an unintended side-effect of switching from Joda Time to Java DateTime API (https://www.elastic.co/guide/en/elasticsearch/reference/7.0/breaking-changes-7.0.html#breaking_70_java_time_changes)?

Steps to reproduce:

curl -X PUT "localhost:9200/twitter/_doc/1" -H 'Content-Type: application/json' -d'
{
    "user" : "kimchy",
    "post_date" : "2009-11-15T14:12:12",
    "message" : "trying out Elasticsearch"
}
'
curl -XPOST 'http://localhost:9200/twitter/_search' -d '{
 "size": 0,
 "aggregations": {
  "q2": {
   "date_histogram": {
    "field": "post_date",
    "format": "e",
    "interval": "day",
    "offset": 0
   }
  }
 }
}'

On Elasticsearch 6.7.2 this returns:

{
  "took": 2,
  "timed_out": false,
  "_shards": {
    "total": 5,
    "successful": 5,
    "skipped": 0,
    "failed": 0
  },
  "hits": {
    "total": 1,
    "max_score": 0,
    "hits": [

    ]
  },
  "aggregations": {
    "q2": {
      "buckets": [
        {
          "key_as_string": "7",
          "key": 1258243200000,
          "doc_count": 1
        }
      ]
    }
  }
}

however on Elasticsearch 7.1.1 is returns (note the difference in "key_as_string":

{
  "took": 1,
  "timed_out": false,
  "_shards": {
    "total": 1,
    "successful": 1,
    "skipped": 0,
    "failed": 0
  },
  "hits": {
    "total": {
      "value": 1,
      "relation": "eq"
    },
    "max_score": null,
    "hits": [

    ]
  },
  "aggregations": {
    "q2": {
      "buckets": [
        {
          "key_as_string": "1",
          "key": 1258243200000,
          "doc_count": 1
        }
      ]
    }
  }
}

Provide logs (if relevant):

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@DaveCTurner
Copy link
Contributor

This reproduces for me. It looks like Joda (and hence Elasticsearch ≤6) maps Monday to 1 whereas Java (and hence Elasticsearch ≥7) maps Sunday to 1.

@DaveCTurner DaveCTurner added :Core/Infra/Core Core issues without another label and removed :Analytics/Aggregations Aggregations labels Jun 17, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@rjernst
Copy link
Member

rjernst commented Jun 20, 2019

While this is definitely fixable, I think we might be past the point of reversing a breaking, and introducing a breaking change in and of itself. I'll mark this for discussion.

@centic9
Copy link
Contributor Author

centic9 commented Jun 21, 2019

Seems not many people are using it in the first place if it went unnoticed for such a long time.

Also I would expect that most large deployments did not start upgrades to 7 yet, so the majority of affected users might not even have started to look at 7.x.

We are running a number of clusters and are upgrading gradually across them, so having to distinguish between v6 and v7 for this would be fairly painful on the client side.

@pgomulka
Copy link
Contributor

relates #42588

@pgomulka pgomulka self-assigned this Jun 26, 2019
pgomulka added a commit to pgomulka/elasticsearch that referenced this issue Aug 9, 2019
Introducing a IsoLocal.ROOT constant which should be used instead of java.util.Locale.ROOT in ES when dealing with dates. IsoLocal.ROOT  customises start of the week to be Monday instead of Sunday.

closes elastic#42588 an issue with investigation details
relates elastic#41670 bug raised (this won't fix it on its own. joda.parseInto has to be reimplemented
closes elastic#43275 an issue raised by community member
pgomulka added a commit to pgomulka/elasticsearch that referenced this issue Oct 17, 2019
Introducing a IsoLocal.ROOT constant which should be used instead of java.util.Locale.ROOT in ES when dealing with dates. IsoLocal.ROOT  customises start of the week to be Monday instead of Sunday.

closes elastic#42588 an issue with investigation details
relates elastic#41670 bug raised (this won't fix it on its own. joda.parseInto has to be reimplemented
closes elastic#43275 an issue raised by community member

change skip reason

compile error

not orking spi

working unit test

cleanup

 change providers for 9+

revert changes IsoLocale

cleanup

move spi files to server

make unit test pass from gradle

expermienting with gradle tasks

uncomment jar hell check

only add settings in buildplugin

allign options for locale providers
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
Projects
None yet
Development

No branches or pull requests

5 participants