-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: make extract work with TimeTZ #42618
Conversation
82f9059
to
6a44424
Compare
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 messed around a bit with Postgres and it seems like you should not be forcing all values to be positive:
solon=# select extract('timezone_hour' from '00:00-01:02'::TIMETZ);
date_part
-----------
-1
(1 row)
solon=# select extract('timezone_minute' from '00:00-01:02'::TIMETZ);
date_part
-----------
-2
(1 row)
solon=# select extract('timezone' from '00:00-01:02'::TIMETZ);
date_part
-----------
-3720
(1 row)
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @solongordon)
WEIRD i swear it didn't work... maybe i typed something else by accident.. |
oh.... i tested timestamptz....
|
oh it's even more subtle than that, for timestamp tz it's always where you are when you |
6a44424
to
d477317
Compare
d477317
to
aefb637
Compare
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.
Reviewed 1 of 4 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @otan and @solongordon)
pkg/sql/sem/builtins/builtins.go, line 1835 at r1 (raw file):
}, Info: "Extracts `element` from `input`.\n\n" + "Compatible elements: hour, minute, second, millisecond, microsecond, epoch\n" +
Minor: Missing ,
after epoch
.
aefb637
to
60c22eb
Compare
Add extract for TimeTZ, which has a slight extension to that of Time and a gotcha in epoch. No release note as this will be combined in a later PR. Release note: none
60c22eb
to
13ba75b
Compare
bors r+ |
Build failed (retrying...) |
42618: builtins: make extract work with TimeTZ r=otan a=otan Refs: #26097 Add extract for TimeTZ, which has a slight extension to that of Time and a gotcha in epoch. No release note as this will be combined in a later PR. Release note: none Co-authored-by: Oliver Tan <[email protected]>
Build succeeded |
Refs: #26097
Add extract for TimeTZ, which has a slight extension to that of Time and
a gotcha in epoch.
No release note as this will be combined in a later PR.
Release note: none