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

builtins: add CURRENT_TIME functionality #42928

Merged
merged 1 commit into from
Dec 6, 2019

Conversation

otan
Copy link
Contributor

@otan otan commented Dec 3, 2019

Resolves #31708
Refs #26097

This PR adds CURRENT_TIME as a builtin.

Release note (sql change): This PR adds the CURRENT_TIME builtin, which
can be used with precision, e.g. SELECT CURRENT_TIME, CURRENT_TIME(3).

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@otan otan requested review from solongordon and a team December 3, 2019 22:00
@otan otan force-pushed the current_time branch 4 times, most recently from ee1df74 to 29a5eaf Compare December 4, 2019 00:08
Copy link
Contributor

@solongordon solongordon left a comment

Choose a reason for hiding this comment

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

It seems like the TIMETZ version differs from Postgres in that it does not reflect the session time zone. Is that right? For example set time zone -5; select current_time::text; returns different results between CRDB and Postgres.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @solongordon)

Copy link
Contributor Author

@otan otan left a comment

Choose a reason for hiding this comment

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

image.png

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

Copy link
Contributor Author

@otan otan left a comment

Choose a reason for hiding this comment

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

trying to figure out why current_timestamp is okay though...

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

Copy link
Contributor Author

@otan otan left a comment

Choose a reason for hiding this comment

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

hint: it is not -- #43012

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

This PR adds CURRENT_TIME as a builtin.

Release note (sql change): This PR adds the CURRENT_TIME builtin, which
can be used with precision, e.g. `SELECT CURRENT_TIME, CURRENT_TIME(3)`.
Copy link
Contributor

@solongordon solongordon left a comment

Choose a reason for hiding this comment

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

Reviewed 9 of 9 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@otan
Copy link
Contributor Author

otan commented Dec 6, 2019

bors r+

craig bot pushed a commit that referenced this pull request Dec 6, 2019
42928: builtins: add CURRENT_TIME functionality r=otan a=otan

Resolves #31708
Refs #26097

This PR adds CURRENT_TIME as a builtin.

Release note (sql change): This PR adds the CURRENT_TIME builtin, which
can be used with precision, e.g. `SELECT CURRENT_TIME, CURRENT_TIME(3)`.

43012: sql: fix current_timestamp behaviour with time zone set r=otan a=otan

With time zones set, current_timestamp needs to localise to the timezone
set in the context for time options, e.g. for `TIME` with `UTC+3` at
`UTC midnight`, `current_timestamp()` should return `3am`. This was
previously not handled correctly by Timestamp, and is rectified in this
PR.

Release note (bug fix): Previously, current_timestamp would not
correctly account for `SET TIME ZONE` in the background when storing
results, storing the timestamp as `UTC` instead. This is fixed in this
PR.

Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
@craig
Copy link
Contributor

craig bot commented Dec 6, 2019

Build succeeded

@craig craig bot merged commit 7d30f95 into cockroachdb:master Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sql: support current_time
3 participants