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

date_nano field min/max aggregation gives the wrong date string #52568

Closed
jackielii opened this issue Feb 20, 2020 · 7 comments
Closed

date_nano field min/max aggregation gives the wrong date string #52568

jackielii opened this issue Feb 20, 2020 · 7 comments
Assignees
Labels
:Analytics/Aggregations Aggregations Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)

Comments

@jackielii
Copy link

Elasticsearch version (bin/elasticsearch --version):
OpenJDK 64-Bit Server VM warning: Option UseConcMarkSweepGC was deprecated in version 9.0 and will likely be removed in a future release.
Version: 7.5.2, Build: default/docker/8bec50e1e0ad29dad5653712cf3bb580cd1afcdf/2020-01-15T12:11:52.313576Z, JVM: 13.0.1

Plugins installed: []

JVM version (java -version):
openjdk 13.0.1 2019-10-15
OpenJDK Runtime Environment AdoptOpenJDK (build 13.0.1+9)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 13.0.1+9, mixed mode, sharing)

OS version (uname -a if on a Unix-like system):
Linux e175e3875c6b 4.15.0-76-generic #86-Ubuntu SMP Fri Jan 17 17:24:28 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Description of the problem including expected versus actual behavior:

date_nano field using min or max aggregation gives wrong string value, e.g:

    "max7" : {
      "value" : 1.66790755312345011E18,
      "value_as_string" : "+52855856-04-21T13:24:10"
    },

If we use the value as nano seconds, it is the expected string: 2022-11-08T11:39:13.12345011

Steps to reproduce:

Please include a minimal but complete recreation of the problem, including
(e.g.) index creation, mappings, settings, query etc. The easier you make for
us to reproduce it, the more likely that somebody will take the time to look at it.

  1. create index:
curl -X PUT "localhost:9200/datenano?pretty" -H 'Content-Type: application/json' -d'
{
  "mappings": {
    "dynamic" : "strict",
    "properties" : {
      "datefield" : {
        "type": "date_nanos",
        "format": "strict_date_hour_minute_second_millis"
      }
    }
  }
}'
  1. index a doc:
curl -X PUT "localhost:9200/datenano/_doc/1?pretty" -H 'Content-Type: application/json' -d'
{
  "datefield": "2022-11-08T11:39:13.12345011"
}'
  1. get the doc: this yields the correct behaviour:
curl -X GET "localhost:9200/datenano/_doc/1"

response:

{
  "_index" : "datenano",
  "_type" : "_doc",
  "_id" : "1",
  "_version" : 1,
  "_seq_no" : 1,
  "_primary_term" : 1,
  "found" : true,
  "_source" : {
    "datefield" : "2022-11-08T11:39:13.12345011"
  }
}
  1. min aggregation:
curl -X POST "localhost:9200/datenano/_search?pretty" -H 'Content-Type: application/json' -d'
{
  "size":0,
  "aggregations": {
    "mindate": {
      "min":{
        "field":"datefield"
      }
    }
  }
}'

response:

{
  "took" : 0,
  "timed_out" : false,
  "_shards" : {
    "total" : 1,
    "successful" : 1,
    "skipped" : 0,
    "failed" : 0
  },
  "hits" : {
    "total" : {
      "value" : 2,
      "relation" : "eq"
    },
    "max_score" : null,
    "hits" : [ ]
  },
  "aggregations" : {
    "mindate" : {
      "value" : 1.66790755312345011E18,
      "value_as_string" : "+52855856-04-21T13:24:10.112" // <= this is incorrect behavior
    }
  }
}

Work around:

Get the value of the aggregation, and create using Instant.ofEpochSecond(0, min.toLong()) in Java or t := time.Unix(0, int64(mindate.value)) in Go

Provide logs (if relevant):

@iverase iverase added the :Analytics/Aggregations Aggregations label Feb 21, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (:Analytics/Aggregations)

@nik9000 nik9000 self-assigned this Feb 21, 2020
@nik9000
Copy link
Member

nik9000 commented Feb 21, 2020

I think I may have fixed this in #52336. I'll check later today.

@matriv
Copy link
Contributor

matriv commented Feb 21, 2020

@nik9000
It's not 100% fixed. I followed the reproduction steps with master and the result is:

...
"aggregations": {
        "mindate": {
            "value": 1.667907553123E12,
            "value_as_string": "2022-11-08T11:39:13.123"
        }
    }

So the nanos are missing, we have only millis.

@nik9000
Copy link
Member

nik9000 commented Feb 21, 2020

So the nanos are missing, we have only millis.

Fair enough. The bug in #52336 was that the an optimization gave a totally wrong date when you didn't have a query. I fixed that so it gave you the same date as without the optimization. Neither code path gives nanos. That is how all aggregations work. I'm not really sure why.

@polyfractal
Copy link
Contributor

None of the aggregations currently work with nanoseconds, so this is "working as intended": https://www.elastic.co/guide/en/elasticsearch/reference/master/date_nanos.html#date-nanos-limitations

I don't recall if there was a technical reason nanoseconds aren't supported, or if it was just deferred until later since the java-time/nanosecond change was already huge and complicated without adjusting aggs

@nik9000
Copy link
Member

nik9000 commented Feb 21, 2020

I think it'd be nice to have an issue to track making aggs work properly with nanoseconds.

@rjernst rjernst added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 4, 2020
@wchaparro
Copy link
Member

Bug fixed, closing this one in favor of #60149

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo)
Projects
None yet
Development

No branches or pull requests

8 participants