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

Add time zone setting for relative date math in range filter/query #7113

Closed

Conversation

dadoonet
Copy link
Member

Filters and Queries now supports time_zone parameter which defines which time zone should be applied to the query or filter to convert it to UTC time based value.

When applied on date fields the range filter and queries accept also a time_zone parameter.

The time_zone parameter will be applied to your input lower and upper bounds and will move them to UTC time based date:

{
    "constant_score": {
        "filter": {
            "range" : {
                "born" : {
                    "gte": "2012-01-01",
                    "lte": "now",
                    "time_zone": "+1:00"
                }
            }
        }
    }
}

{
    "range" : {
        "born" : {
            "gte": "2012-01-01",
            "lte": "now",
            "time_zone": "+1:00"
        }
    }
}

In the above examples, gte will be actually moved to 2011-12-31T23:00:00 UTC date.

NOTE: if you give a date with a timezone explicitly defined and use the time_zone parameter, time_zone will be
ignored. For example, setting from to 2012-01-01T00:00:00+01:00 with "time_zone":"+10:00" will still use +01:00 time zone.

Closes #3729.

…query

Filters and Queries now supports `time_zone` parameter which defines which time zone should be applied to the query or filter to convert it to UTC time based value.

Closes elastic#3729.
@dadoonet dadoonet self-assigned this Jul 31, 2014
}

public long adjustToUTCTime(long value, DateTimeZone zone) {
// Small optimization: we don't do math when not needed :)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it is needed? I would expect DateTimeZone.UTC.getOffset to already have this optimization and constantly return 0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Removed.

@jpountz
Copy link
Contributor

jpountz commented Aug 1, 2014

Instead of always moving the time after the date is parser, maybe we should rather configure the parser to use the provided time zone as a default, ie. when a timezone is not already provided in the date? This way, dates that are explicit about the timezone, eg.

  • "2012-04-21T18:25:43-05:00"
  • "2012-04-23T18:25:43.511Z"

would not be moved, while dates that don't specify a timezone, eg.

  • "2012-01-01"
    would be moved?

This seems to be possible by replacing parser.parseMillis(date); with parser.withZone(timeZone).parseMillis(date); (didn't try).

I think we should not try to move numbers of milliseconds since Epoch either, since Epoch is the same instant on all timezones.

@jpountz jpountz removed the review label Aug 1, 2014
@dadoonet
Copy link
Member Author

dadoonet commented Aug 1, 2014

@jpountz PR updated

@dadoonet dadoonet added the review label Aug 1, 2014
@@ -30,6 +30,29 @@ The `range` filter accepts the following parameters:
`lte`:: Less-than or equal to
`lt`:: Less-than

When applied on `date` fields the `range` filter accepts also a `time_zone` parameter.
The `time_zone` parameter will be applied to your input lower and upper bounds and will
move them to UTC time based date:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to document that it is only applied for dates that don't make the timezone explicit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They'll get a clean exception in that case :) But Yes I can add a note saying that they can't mix things.

@dadoonet
Copy link
Member Author

dadoonet commented Aug 1, 2014

@jpountz Updated :)

In the above example, `gte` will be actually moved to `2011-12-31T23:00:00` UTC date.

NOTE: you can not enter a date with a timezone explicitly defined with the `time_zone` parameter.
For example, setting `from` to `2012-01-01T00:00:00+01:00` will fail if you set any `time_zone`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried it and didn't get any error, the time zone was just ignored (which is fine I think)

@jpountz
Copy link
Contributor

jpountz commented Aug 4, 2014

@dadoonet Just played with the pull request, seems to work fine. I left a couple of minor comments but other than that it looks good to me!

@jpountz jpountz removed the review label Aug 4, 2014
@dadoonet
Copy link
Member Author

dadoonet commented Aug 4, 2014

@jpountz thanks for the comments. Just updated my branch.

I'll rebase on master and run the full tests again.

Do you think it should go in other branch than master and 1.4?

@jpountz
Copy link
Contributor

jpountz commented Aug 4, 2014

+1 to push, this looks great!

Do you think it should go in other branch than master and 1.4?

master and 1.4 sound good to me. This is a new feature, so it doesn't really make sense to push it to older branches.

@dadoonet
Copy link
Member Author

dadoonet commented Aug 4, 2014

I guess I need to update doc with a "coming in 1.4" :)

@dadoonet
Copy link
Member Author

dadoonet commented Aug 4, 2014

pushed in master (873a45e) and 1.x (2c0db1f)

@dadoonet dadoonet closed this Aug 4, 2014
@dadoonet dadoonet deleted the pr/3729-timezone-rangefilter branch August 4, 2014 13:44
@clintongormley clintongormley changed the title Search: add time zone setting for relative date math in range filter/query Search: Add time zone setting for relative date math in range filter/query Sep 8, 2014
@clintongormley clintongormley changed the title Search: Add time zone setting for relative date math in range filter/query Query DSL: Add time zone setting for relative date math in range filter/query Sep 8, 2014
@mewwts
Copy link

mewwts commented Feb 23, 2015

Hi, I've opened #9814 because I can't seem to get this to work with rounding in date math. Is this expected behaviour?

@clintongormley clintongormley changed the title Query DSL: Add time zone setting for relative date math in range filter/query Add time zone setting for relative date math in range filter/query Jun 7, 2015
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Query DSL labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories v1.4.0.Beta1 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add time zone setting for relative date math in range filter/query
4 participants