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

Support timezone offsets for queries #7762

Merged
merged 1 commit into from
Mar 28, 2017
Merged

Conversation

jsternberg
Copy link
Contributor

@jsternberg jsternberg commented Dec 25, 2016

The timezone for a query can now be added to the end with something like
TZ('America/Los_Angeles') and it will localize the results of the
query to be in that timezone. The offset will automatically be set to
the offset for that timezone and offsets will automatically adjust for
daylight savings time so grouping by a day will result in a 25 hour day
once a year and a 23 hour day another day of the year.

The automatic adjustment of intervals for timezone offsets changing will
only happen if the group by period is greater than the timezone offset
would be. That means grouping by an hour or less will not be affected by
daylight savings time, but a 2 hour or 1 day interval will be.

The default timezone is UTC and existing queries are unaffected by this
change.

When times are returned as strings (when epoch=1 is not used), the
results will be returned using the requested timezone format in RFC3339
format.

Fixes #6541.

@jsternberg
Copy link
Contributor Author

This still requires tests. I've only verified a single query with this and haven't tried the edge cases or tested the efficiency. I also think the changes to the fill iterator are going to need to be different so it's faster, but that's only a hunch at the moment.

For those following this issue and seeing this PR, it will not end up in 1.2 and is potentially targeted at 1.3 at the moment. I just wanted to get it up now because I finally thought of a way to do this and wanted to write the code down before I forgot.

@jsternberg jsternberg force-pushed the js-6541-timezone-support branch 3 times, most recently from a3991df to a5a5bfa Compare December 27, 2016 15:58
@jsternberg
Copy link
Contributor Author

This now has tests and fixed some bugs when traveling between time offsets. This is likely fine to be reviewed and tested more thoroughly now.

@jsternberg jsternberg force-pushed the js-6541-timezone-support branch 2 times, most recently from d2078aa to f18d491 Compare December 27, 2016 20:22
@jwilder jwilder added this to the 1.3.0 milestone Jan 4, 2017
@jsternberg jsternberg force-pushed the js-6541-timezone-support branch 2 times, most recently from d907d70 to 04d1cc3 Compare January 24, 2017 14:22
@jgeiger
Copy link

jgeiger commented Feb 9, 2017

👍 This would be useful for pulling CSV data that no longer needs to be modified by the pulling application.

@jwilder jwilder mentioned this pull request Feb 21, 2017
@jwilder jwilder added the area/influxql Issues related to InfluxQL query language label Feb 21, 2017
@jsternberg jsternberg force-pushed the js-6541-timezone-support branch from 04d1cc3 to 67f214e Compare March 15, 2017 17:50
@jsternberg jsternberg requested a review from dgnorton March 16, 2017 17:25
@jsternberg jsternberg force-pushed the js-6541-timezone-support branch 2 times, most recently from 4f3d241 to bdd003f Compare March 21, 2017 19:35
@dgnorton
Copy link
Contributor

@jsternberg I think the syntax should be single quotes.

TZ('America/Los_Angeles')

Thoughts?

@jsternberg
Copy link
Contributor Author

I can do that. I wasn't sure which one to use anyway since we muddle the water between identifiers and strings in a few too many places.

@jsternberg jsternberg force-pushed the js-6541-timezone-support branch 2 times, most recently from 2085425 to b9c9f01 Compare March 22, 2017 20:05
The timezone for a query can now be added to the end with something like
`TZ("America/Los_Angeles")` and it will localize the results of the
query to be in that timezone. The offset will automatically be set to
the offset for that timezone and offsets will automatically adjust for
daylight savings time so grouping by a day will result in a 25 hour day
once a year and a 23 hour day another day of the year.

The automatic adjustment of intervals for timezone offsets changing will
only happen if the group by period is greater than the timezone offset
would be. That means grouping by an hour or less will not be affected by
daylight savings time, but a 2 hour or 1 day interval will be.

The default timezone is UTC and existing queries are unaffected by this
change.

When times are returned as strings (when `epoch=1` is not used), the
results will be returned using the requested timezone format in RFC3339
format.
@jsternberg jsternberg force-pushed the js-6541-timezone-support branch from b9c9f01 to 347b018 Compare March 22, 2017 20:09
@jsternberg jsternberg merged commit 3e52ec7 into master Mar 28, 2017
@jsternberg jsternberg deleted the js-6541-timezone-support branch March 28, 2017 15:39
@openbufo
Copy link

openbufo commented May 5, 2017

Updated to 1.2.3 using deb package on ubuntu. While trying queries with TZ('America/Los_Angeles') at the end, I m getting the following error:
ERR: error parsing query: found TZ, expected ; at line 1
Exact query statement: SELECT mean(value) FROM cpu WHERE time >= '2017-05-04T00:00:00Z' GROUP BY time(1d) TZ("America/Los_Angeles").
Where am I going wrong? Isnt this feature available on 1.2.3 deb pkg yet? Or do i need to build from source?

@jsternberg
Copy link
Contributor Author

Time zones are slated for 1.3. They should be in master right now.

@openbufo
Copy link

openbufo commented May 6, 2017

Okay. So, I build influxdb from the git master branch on a docker image. TZ function is now recognizable but cant find the timezone. Using tz('America/Los_Angeles') throws an error "unable to find time zone America/Los_Angeles". Error goes with other timezones as well.

@jsternberg
Copy link
Contributor Author

Please direct usage questions to our community website at https://community.influxdata.com.

@jsternberg
Copy link
Contributor Author

Updated the PR comment to use single quotes instead of double quotes. That got changed during review but never got changed in the PR comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/influxql Issues related to InfluxQL query language
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants