From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Noah Misch <noah@leadboat.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: Vik Fearing <vik@postgresfriends.org>
Cc: cary huang <hcary328@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Add support for AT LOCAL
Date: Tue, 17 Oct 2023 12:45:28 -0400
Message-ID: <2820407.1697561128@sss.pgh.pa.us> (raw)
In-Reply-To: <2726901.1697556666@sss.pgh.pa.us>
References: <ZSyYqySa74x_RAH_@paquier.xyz>
<CA+hUKGLYaVJ7df+Uu=ctUunRcqUsGY55feVQa-1pzz1=PT=z3w@mail.gmail.com>
<2188264.1697425365@sss.pgh.pa.us>
<CA+hUKGJSEOVonGsYCiuhZXsBFCxXtjNyHGYvn-w9jykGo2PgqQ@mail.gmail.com>
<2191749.1697427017@sss.pgh.pa.us>
<20231016045804.c0@rfd.leadboat.com>
<20231016055051.3a@rfd.leadboat.com>
<2208907.1697435663@sss.pgh.pa.us>
<20231016060510.72@rfd.leadboat.com>
<2659915.1697521218@sss.pgh.pa.us>
<ZS4oykeg1ydoTM0W@paquier.xyz>
<2726901.1697556666@sss.pgh.pa.us>
I wrote:
> Yeah, the same thing occurred to me in the shower this morning, and it
> does seem to work! We can replace both loops with a %= operator, at
> least if we're willing to assume C99 division semantics, which seems
> pretty safe in 2023.
Whoops, no: for negative starting values we'd need truncate-towards-
minus-infinity division whereas C99 specifies truncate-towards-zero.
However, the attached does pass for me on cfarm111 as well as my
usual dev machine.
Presumably this is a pre-existing bug that also appears in back
branches. But in the interests of science I propose that we
back-patch only the test case and see which machine(s) fail it
before back-patching the code change.
regards, tom lane
Attachments:v1-fix-timetz-modulo-ops.patch (text/x-diff, 2.8K) downloaddiff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.cindex c4da10d47a..56c7746c11 100644--- a/src/backend/utils/adt/date.c+++ b/src/backend/utils/adt/date.c@@ -3083,10 +3083,11 @@ timetz_zone(PG_FUNCTION_ARGS)
result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
result->time = t->time + (t->zone - tz) * USECS_PER_SEC;
+ /* C99 modulo has the wrong sign convention for negative input */
while (result->time < INT64CONST(0))
result->time += USECS_PER_DAY;
- while (result->time >= USECS_PER_DAY)- result->time -= USECS_PER_DAY;+ if (result->time >= USECS_PER_DAY)+ result->time %= USECS_PER_DAY;
result->zone = tz;
@@ -3116,10 +3117,11 @@ timetz_izone(PG_FUNCTION_ARGS)
result = (TimeTzADT *) palloc(sizeof(TimeTzADT));
result->time = time->time + (time->zone - tz) * USECS_PER_SEC;
+ /* C99 modulo has the wrong sign convention for negative input */
while (result->time < INT64CONST(0))
result->time += USECS_PER_DAY;
- while (result->time >= USECS_PER_DAY)- result->time -= USECS_PER_DAY;+ if (result->time >= USECS_PER_DAY)+ result->time %= USECS_PER_DAY;
result->zone = tz;
diff --git a/src/test/regress/expected/timetz.out b/src/test/regress/expected/timetz.outindex 3f8e005ce7..cbab6cfe5d 100644--- a/src/test/regress/expected/timetz.out+++ b/src/test/regress/expected/timetz.out@@ -304,4 +304,25 @@ TABLE timetz_local_view;
23:59:59.99-07 | 06:59:59.99+00 | 06:59:59.99+00 | 06:59:59.99+00 | 06:59:59.99+00
(12 rows)
+SELECT f1 AS dat,+ f1 AT TIME ZONE 'UTC+10' AS dat_at_tz,+ f1 AT TIME ZONE INTERVAL '-10:00' AS dat_at_int+ FROM TIMETZ_TBL+ ORDER BY f1;+ dat | dat_at_tz | dat_at_int +----------------+----------------+----------------+ 00:01:00-07 | 21:01:00-10 | 21:01:00-10+ 01:00:00-07 | 22:00:00-10 | 22:00:00-10+ 02:03:00-07 | 23:03:00-10 | 23:03:00-10+ 08:08:00-04 | 02:08:00-10 | 02:08:00-10+ 07:07:00-08 | 05:07:00-10 | 05:07:00-10+ 11:59:00-07 | 08:59:00-10 | 08:59:00-10+ 12:00:00-07 | 09:00:00-10 | 09:00:00-10+ 12:01:00-07 | 09:01:00-10 | 09:01:00-10+ 15:36:39-04 | 09:36:39-10 | 09:36:39-10+ 15:36:39-05 | 10:36:39-10 | 10:36:39-10+ 23:59:00-07 | 20:59:00-10 | 20:59:00-10+ 23:59:59.99-07 | 20:59:59.99-10 | 20:59:59.99-10+(12 rows)+
ROLLBACK;
diff --git a/src/test/regress/sql/timetz.sql b/src/test/regress/sql/timetz.sqlindex 33f7f8aafb..d797f478f0 100644--- a/src/test/regress/sql/timetz.sql+++ b/src/test/regress/sql/timetz.sql@@ -100,4 +100,9 @@ CREATE VIEW timetz_local_view AS
ORDER BY f1;
SELECT pg_get_viewdef('timetz_local_view', true);
TABLE timetz_local_view;
+SELECT f1 AS dat,+ f1 AT TIME ZONE 'UTC+10' AS dat_at_tz,+ f1 AT TIME ZONE INTERVAL '-10:00' AS dat_at_int+ FROM TIMETZ_TBL+ ORDER BY f1;
ROLLBACK;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, noah@leadboat.com, thomas.munro@gmail.com, vik@postgresfriends.org, hcary328@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Add support for AT LOCAL
In-Reply-To: <2820407.1697561128@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox