-
Notifications
You must be signed in to change notification settings - Fork 265
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 timezone support to v3 SQL docs #5581
base: master
Are you sure you want to change the base?
Conversation
sanderson
commented
Aug 28, 2024
- Updates information about time zones in SQL across all v3 products
- Updates SQL data types documentation
- Adds missing time and date functions
- Adds information about how timezones change the behavior of some functions.
- Rebased/mergeable
Thanks so much for the pull request! |
| INTERVAL | INTERVAL | Interval(IntervalUnit::YearMonth) or Interval(IntervalUnit::DayTime) | | ||
| SQL data type | Arrow data type | Description | | ||
| :------------ | :----------------------------- | :------------------------------------------- | | ||
| TIMESTAMP | Timestamp(Nanosecond, None) | Nanosecond timestamp with a time zone offset | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a bit pedantic, but technically this is a timestamp without a time zone offset. This is what Arrow calls a "point in time".
I think the intention of what you have here is good though, so maybe change the Arrow data type to:
Timestamp(Nanosecond, Some("UTC"))
which will also help solidify the change to UTC.
'2024-01-01 00:00:00'::TIMESTAMP AT TIME ZONE 'America/Los_Angeles' AS 'Local time with TZ offset' | ||
``` | ||
|
||
| UTC with TZ offset | | ||
| :----------------------- | | ||
| 2024-01-01T08:00:00.000Z | | ||
| Local time with TZ offset | | ||
| :------------------------ | | ||
| 2024-01-01T00:00:00-08:00 | | ||
|
||
{{% /expand %}} | ||
{{< /expand-wrapper >}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm currently suggesting this is a bug. Assuming others agree, this will get fixed in DF so that this behaves the same as the code above it.
Or, more accurately, this would be considered a bug in versions of 3.0 that have timezone support turned on.
| INTERVAL | INTERVAL | Interval(IntervalUnit::YearMonth) or Interval(IntervalUnit::DayTime) | | ||
| SQL data type | Arrow data type | Description | | ||
| :------------ | :----------------------------- | :------------------------------------------- | | ||
| TIMESTAMP | Timestamp(Nanosecond, None) | Nanosecond timestamp with a time zone offset | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above
'2024-01-01 00:00:00'::TIMESTAMP AT TIME ZONE 'America/Los_Angeles' AS 'Local time with TZ offset' | ||
``` | ||
|
||
| UTC with TZ offset | | ||
| :----------------------- | | ||
| 2024-01-01T08:00:00.000Z | | ||
| Local time with TZ offset | | ||
| :------------------------ | | ||
| 2024-01-01T00:00:00-08:00 | | ||
|
||
{{% /expand %}} | ||
{{< /expand-wrapper >}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as below
| INTERVAL | INTERVAL | Interval(IntervalUnit::YearMonth) or Interval(IntervalUnit::DayTime) | | ||
| SQL data type | Arrow data type | Description | | ||
| :------------ | :----------------------------- | :------------------------------------------- | | ||
| TIMESTAMP | Timestamp(Nanosecond, None) | Nanosecond timestamp with a time zone offset | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
'2024-01-01 00:00:00'::TIMESTAMP AT TIME ZONE 'America/Los_Angeles' AS 'Local time with TZ offset' | ||
``` | ||
|
||
| UTC with TZ offset | | ||
| :----------------------- | | ||
| 2024-01-01T08:00:00.000Z | | ||
| Local time with TZ offset | | ||
| :------------------------ | | ||
| 2024-01-01T00:00:00-08:00 | | ||
|
||
{{% /expand %}} | ||
{{< /expand-wrapper >}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as below
content/influxdb/cloud-dedicated/reference/sql/functions/time-and-date.md
Outdated
Show resolved
Hide resolved
content/influxdb/cloud-dedicated/reference/sql/functions/time-and-date.md
Outdated
Show resolved
Hide resolved
|
||
Converts a value to a date (`YYYY-MM-DD`). | ||
Supports strings and numeric types as input. | ||
Strings are parsed as `YYYY-MM-DD` unless another format is specified. if no Chrono formats are provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete sentence.
content/influxdb/cloud-dedicated/reference/sql/functions/time-and-date.md
Outdated
Show resolved
Hide resolved
content/influxdb/cloud-serverless/reference/sql/functions/time-and-date.md
Outdated
Show resolved
Hide resolved
content/influxdb/clustered/reference/sql/functions/time-and-date.md
Outdated
Show resolved
Hide resolved
content/influxdb/clustered/reference/sql/functions/time-and-date.md
Outdated
Show resolved
Hide resolved
|
||
Converts a value to a date (`YYYY-MM-DD`). | ||
Supports strings and numeric types as input. | ||
Strings are parsed as `YYYY-MM-DD` unless another format is specified. if no Chrono formats are provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete sentence
Co-authored-by: Jason Stirnaman <[email protected]>
Thanks so much for the pull request! |
Thanks so much for the pull request! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Maybe we need a note about the potential bug Jeff pointed out?
@jeffreyssmith2nd I know this is an upstream bug. Are we going to hold off pushing the TZ support out until this bug is fixed? When should these doc changes go live? |
I don't know yet and am headed out for a long weekend. I'll check in with the team Tuesday on if we should delay or not. Until then, I think we should hold the docs changes. |
|
||
Converts a value to a date (`YYYY-MM-DD`). | ||
Supports strings and numeric types as input. | ||
Strings are parsed as `YYYY-MM-DD` unless another format is specified. if no Chrono formats are provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete sentence
Strings are parsed as
YYYY-MM-DD
unless another format is specified. if no Chrono formats are provided.
?
@jstirnaman I think you may have been viewing the diff of an old commit. I went through and fixed all of those. |
@sanderson Still on hold for the docs changes, waiting on some upstream discussion on if the In the meantime, I'm removing all references to it in the docs where it isn't necessary to prevent any confusion for people down the line. I will update the blog post and make a corresponding PR here as well once we know for sure what behavior we want out of |