elastic / elasticsearch-net

This strongly-typed, client library enables working with Elasticsearch. It is the official client maintained and supported by Elastic.
https://www.elastic.co/guide/en/elasticsearch/client/net-api/current/index.html
Apache License 2.0
17 stars 1.15k forks source link

Time parser acuracy #2569

Closed niemyjski closed 7 years ago

niemyjski commented 7 years ago

I noticed that we have it set to 30 days in a month. This isn't that accurate (https://www.quora.com/What-is-the-average-number-of-days-in-a-month) Should we be updating this?

russcam commented 7 years ago

Are you referring to

Time time = "1M";

evaluating to milliseconds of 30 days? What would be the benefit of choosing another value e.g. 30.4375?

niemyjski commented 7 years ago

Yes, We'll it could change the range or accuracy of returned data over time? I'm trying to remember what all takes a time unit and what accepts a double..... But I could see it if you were expecting better accuracy like maybe parsing a based in time unit and applying it to the returned date time aggregation (which we do in the foundatio project). Food for thought, I just logged this as I know in our code base we are using specific fractions of time to represent days in month and this wasn't...

gmarz commented 7 years ago

We purposely do not calculate milliseconds when months or years are passed because accuracy is impossible.

An approximate fraction like 30.4375 is just as wrong (or approximate) as 30, so I don't think we should do anything here.