builtins: fix incorrect timezone conversions for extract and date_trunc

[otan]

Resolves #42244, fixes more of #42006.

Turns out date_trunc is more broken than I thought, and MakeDTimestampTZFromDate actually not ignoring the argument has revealed a lot of weird usages of it. I also missed a test case with a negative timezone in my earlier PR, which also resolves another bug here. Wonder why govet didn't catch these unused arguments.

---

extract: Due to actually taking loc into account for
MakeDTimestampTZFromDate (as opposed to ignoring the argument before) in
827c97e04041e280a4413984d80cee99520c1d27, this exposed other bugs.

date_trunc: timestamp was incorrect because it tried to adapt to the
timezone, even though it is returning a timestamp without time zone
type. Furthermore, the date needed an additional offset for a given
timezone as the given date is in UTC, but we need to truncate it in
the correct timezone. This meant that my fix in the previous PR only
fixed positive UTC offsets, but not negative UTC offsets.

This PR fixes the immediate issue of extract and date_trunc,
and I will make an audit of all other incorrect usages in an upcoming PR.

There will be an upcoming PR to fix more other bugs found in the Plus
and Minus expressions with the MakeDTimestampTZFromDate change.

Release note (bug fix): Previously, <date>:date when context
local timestamp is set would result in the previous day
(<date-1>::date>) if the timezone is less than UTC+00:00 due to a
recently introduced bug. This change fixes this.

Release note (bug fix): Previously, date_trunc for timestamp were
incorrect if in a local timezone was set. This change fixes this.

Release note (bug fix): Previously, date_trunc for date types were
incorrect with a negative timezone offset in a local timezone. This
change fixes this.

[cockroach-teamcity]

This change is 

[rafiss]

thanks -- lgtm! just a few small questions but feel free to merge after that.

for the commit message, I think it would be a bit better to just have one release note, that synthesizes all the fixes. I don't think you need the full level of detail in the release note area; that might be fine for the commit message body, but that is more up to you.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @otan and @rafiss)

---

pkg/sql/logictest/testdata/logic_test/datetime, line 1390 at r1 (raw file):

select date_trunc('day', '2011-01-02 01:30:00'::timestamp)
----
2011-01-02 00:00:00 +0000 +0000

nice tests! I didn't check myself, but let's just make sure Postgres does the same (apart from the quick where we show the tz offset for non-tz types)

---

pkg/sql/logictest/testdata/logic_test/datetime, line 1512 at r1 (raw file):

UTC

subtest regression_42244

I actually haven't seen this syntax before! could you educate me about what it means/does?

[otan]

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

---

pkg/sql/logictest/testdata/logic_test/datetime, line 1390 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

nice tests! I didn't check myself, but let's just make sure Postgres does the same (apart from the quick where we show the tz offset for non-tz types)

yep, that's where i got the inspiration from :)

---

pkg/sql/logictest/testdata/logic_test/datetime, line 1512 at r1 (raw file):

Previously, rafiss (Rafi Shamim) wrote…

I actually haven't seen this syntax before! could you educate me about what it means/does?

afaik it's just labelling when it fails, it'll give you the specific failure.

[otan]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)