-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 timestamp and timezone management documentation #10036
Conversation
✅ Deploy Preview for meta-velox canceled.
|
Cc: @PHILO-HE |
@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
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.
Thank you for putting this together. Super useful. Some nits.
velox/docs/develop/timestamp.rst
Outdated
reading of a wall clock and a calendar, e.g, ``2024-04-09 18:25:00``. Note that | ||
a TIMESTAMP does not represent an absolute point in time, as the exact same | ||
wall clock time may be read in different instants in time depending on where | ||
one is situated on earth. For example, ``2024-04-09 18:25:00`` in California |
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.
nit: wondering if earth should be Earth
velox/docs/develop/timestamp.rst
Outdated
a TIMESTAMP does not represent an absolute point in time, as the exact same | ||
wall clock time may be read in different instants in time depending on where | ||
one is situated on earth. For example, ``2024-04-09 18:25:00`` in California | ||
and in China were perceived at different absolute points in time, about 15h |
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.
nit: 15h -> 15 hours
velox/docs/develop/timestamp.rst
Outdated
* While “seconds” can be negative to represent time before ``1970-01-01 | ||
00:00:00``, “nanoseconds” are always positive. | ||
|
||
* Engines like Presto and Spark only support milliseconds precision, so |
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 believe Spark supports at least microseconds precision.
velox/docs/develop/timestamp.rst
Outdated
00:00:00``, “nanoseconds” are always positive. | ||
|
||
* Engines like Presto and Spark only support milliseconds precision, so | ||
naturally “nanoseconds” are zeroed. |
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.
Nanoseconds are used even with millisecond precision.
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.
Rephrased it
velox/docs/develop/timestamp.rst
Outdated
TimestampWithTimezones are serialized, for example. | ||
|
||
**TimestampWithTimezone:** To represent an absolute point in time, Velox provides | ||
the `TimestampWithTimezone class <https://github.com/facebookincubator/velox/blob/main/velox/functions/prestosql/types/TimestampWithTimeZoneType.h>`_. |
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 don't think there is a TimestampWithTimezone class.
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.
well, it's an alias:
I'll rephrase.
velox/docs/develop/timestamp.rst
Outdated
|
||
**TimestampWithTimezone:** To represent an absolute point in time, Velox provides | ||
the `TimestampWithTimezone class <https://github.com/facebookincubator/velox/blob/main/velox/functions/prestosql/types/TimestampWithTimeZoneType.h>`_. | ||
This class implements the TIMESTAMP WITH TIMEZONE sql semantic discussed above, |
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.
sql -> SQL
velox/docs/develop/timestamp.rst
Outdated
**TimestampWithTimezone:** To represent an absolute point in time, Velox provides | ||
the `TimestampWithTimezone class <https://github.com/facebookincubator/velox/blob/main/velox/functions/prestosql/types/TimestampWithTimeZoneType.h>`_. | ||
This class implements the TIMESTAMP WITH TIMEZONE sql semantic discussed above, | ||
and was based on Presto’s implementation - therefore only supporting |
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.
was based -> is based
velox/docs/develop/timestamp.rst
Outdated
millisecond-precision. | ||
|
||
TimestampWithTimezone physically packs two integers in a single 64 word, using | ||
12 bits for timezone ID, and 52 for a millisecond-precision timestamp. |
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.
52 -> 52 bits
|
||
.. code-block:: bash | ||
|
||
$ rpm -qa | grep tzdata |
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 tried this command on my Mac, but it didn't print anything.
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.
this is for linux; added to the text to clarify
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.
Thanks for composing this doc. This is super helpful!
velox/docs/develop/timestamp.rst
Outdated
|
||
To represent absolute points in time, SQL defines a TIMESTAMP WITH TIMEZONE | ||
type, which conceptually represents a pair of a wall time and calendar read | ||
(say, ``2024-04-09 18:25:00``), and a timezone (``PST``, or |
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.
April should be in PDT
instead of PST
at America/Los_Angeles. Or we can just put PT
here.
velox/docs/develop/timestamp.rst
Outdated
|
||
.. code-block:: sql | ||
|
||
presto:de> select typeof(TIMESTAMP '1970-01-01 00:00:00'); |
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.
Let's omit the presto:de
part? Should we keep the format of the examples consistent as https://facebookincubator.github.io/velox/functions/presto/conversion.html#cast-to-timestamp
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.
Nice! That's better
Adding a new documentation page to describe details of management of timestamp, timezone and conversions in Velox.
8f8c11e
to
96daf21
Compare
@pedroerp has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
…r#10036) Summary: Adding a new documentation page to describe details of management of timestamps, timezone and conversions in Velox. Pull Request resolved: facebookincubator#10036 Reviewed By: mbasmanova, zacw7 Differential Revision: D58172617 Pulled By: pedroerp fbshipit-source-id: a8fc2e5a78bd65bf587b65e3296ef3c27f23da28
…r#10036) Summary: Adding a new documentation page to describe details of management of timestamps, timezone and conversions in Velox. Pull Request resolved: facebookincubator#10036 Reviewed By: mbasmanova, zacw7 Differential Revision: D58172617 Pulled By: pedroerp fbshipit-source-id: a8fc2e5a78bd65bf587b65e3296ef3c27f23da28
…r#10036) Summary: Adding a new documentation page to describe details of management of timestamps, timezone and conversions in Velox. Pull Request resolved: facebookincubator#10036 Reviewed By: mbasmanova, zacw7 Differential Revision: D58172617 Pulled By: pedroerp fbshipit-source-id: a8fc2e5a78bd65bf587b65e3296ef3c27f23da28
Adding a new documentation page to describe details of management of timestamps, timezone and conversions in Velox.