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

Range filters, date math and time_zone #9814

Closed
mewwts opened this Issue Feb 23, 2015 · 9 comments

Comments

Projects
None yet
3 participants
@mewwts

mewwts commented Feb 23, 2015

Hi guys.

I've stumbled upon an issue. When specifying the time_zone parameters in a range filter where gte and lt are set using datemath, the time zone is not taken into account when rounding!

The following is part of a filters aggregation, and I expect to be given the week 4 weeks ago, starting on a
sunday at 23:00 UTC time, and ending on sunday 23:00 UTC time with a full week in between.

{
  "filters": {
    "4w": {
      "range": {
        "timestamp": {
          "lt": "now-4w+1w/w",
          "gte": "now-4w/w",
          "time_zone": "+01:00"
        }
      }
    }
  }
}

But no, after inspecting the docs that are returned, there are no documents returned between the first sunday 23:00 and monday 00:00 UTC. There are however documents returned between sunday 23:00 and monday 00:00 UTC at the end of the interval.

Is this a bug? Should not timezones be included when doing rounding in date math?

@cbuescher

This comment has been minimized.

Show comment
Hide comment
@cbuescher

cbuescher Feb 25, 2015

Member

Will have a look since this might or might not be related to the rounding issues recently solved in #9790.

Member

cbuescher commented Feb 25, 2015

Will have a look since this might or might not be related to the rounding issues recently solved in #9790.

@cbuescher

This comment has been minimized.

Show comment
Hide comment
@cbuescher

cbuescher Feb 25, 2015

Member

@mewwts Which version are you running?

Member

cbuescher commented Feb 25, 2015

@mewwts Which version are you running?

@cbuescher

This comment has been minimized.

Show comment
Hide comment
@cbuescher

cbuescher Feb 25, 2015

Member

From a first glance: the rounding in DateMathParser (which seems to be used in range filter) is not related to the TimeZoneRounding which was subject of #9790, so I was wrong in assuming the two issues are related. Seems like rounding in DateMathParser does not take into account time zone correctly.

Member

cbuescher commented Feb 25, 2015

From a first glance: the rounding in DateMathParser (which seems to be used in range filter) is not related to the TimeZoneRounding which was subject of #9790, so I was wrong in assuming the two issues are related. Seems like rounding in DateMathParser does not take into account time zone correctly.

cbuescher added a commit to cbuescher/elasticsearch that referenced this issue Feb 25, 2015

DateMath: Fix using time zone when rounding.
Currently rounding in DateMathParser is always done in UTC, even
when another time zone is specified. This fix corrects this by
passing the specified time zone down to the rounding logic.

Closes  #9814
@mewwts

This comment has been minimized.

Show comment
Hide comment
@mewwts

mewwts Feb 25, 2015

Hi @cbuescher, thanks for your reply and quick fix!?
I'm running 1.4.2. I was going to take a look at this myself, but I can see you beat me to it!

mewwts commented Feb 25, 2015

Hi @cbuescher, thanks for your reply and quick fix!?
I'm running 1.4.2. I was going to take a look at this myself, but I can see you beat me to it!

@mewwts

This comment has been minimized.

Show comment
Hide comment
@mewwts

mewwts Feb 25, 2015

Hands down, greatest response ever. Thanks @cbuescher. Now the wait for 1.4.5, I guess?

mewwts commented Feb 25, 2015

Hands down, greatest response ever. Thanks @cbuescher. Now the wait for 1.4.5, I guess?

@cbuescher

This comment has been minimized.

Show comment
Hide comment
@cbuescher

cbuescher Feb 25, 2015

Member

So far only on out main branch, but will merge with the current 1.4 branch, so after that's happened it will go into 1.4.5.

Member

cbuescher commented Feb 25, 2015

So far only on out main branch, but will merge with the current 1.4 branch, so after that's happened it will go into 1.4.5.

@mewwts

This comment has been minimized.

Show comment
Hide comment
@mewwts

mewwts commented Feb 25, 2015

👍 Thanks, @cbuescher.

cbuescher added a commit that referenced this issue Feb 25, 2015

DateMath: Fix using time zone when rounding.
Currently rounding in DateMathParser This always done in UTC, even
when another time zone is specified. This is fixed by passing the time zone
down to the rounding logic when it is specified.

Closes #9814
Closes #9885

cbuescher added a commit that referenced this issue Mar 2, 2015

DateMath: Fix using time zone when rounding.
Currently rounding in DateMathParser This always done in UTC, even
when another time zone is specified. This is fixed by passing the time zone
down to the rounding logic when it is specified.

Closes #9814
Closes #9885
@cbuescher

This comment has been minimized.

Show comment
Hide comment
@cbuescher

cbuescher Mar 2, 2015

Member

On branch 1.4 with dff19cb and on 1.x with 8391b51

Member

cbuescher commented Mar 2, 2015

On branch 1.4 with dff19cb and on 1.x with 8391b51

@mewwts

This comment has been minimized.

Show comment
Hide comment
@mewwts

mewwts commented Mar 3, 2015

Great @cbuescher!

mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015

DateMath: Fix using time zone when rounding.
Currently rounding in DateMathParser This always done in UTC, even
when another time zone is specified. This is fixed by passing the time zone
down to the rounding logic when it is specified.

Closes #9814
Closes #9885
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment