From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
cc: Noah Misch <noah@leadboat.com>, Thomas Munro <thomas.munro@gmail.com>,
        Vik Fearing <vik@postgresfriends.org>,
 cary huang <hcary328@gmail.com>,
        pgsql-hackers@lists.postgresql.org
Subject: Re: Add support for AT LOCAL
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>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Tue, 17 Oct 2023 11:31:06 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <2820184.1697560951.0@sss.pgh.pa.us>
Date: Tue, 17 Oct 2023 12:45:28 -0400
Message-ID: <2820407.1697561128@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2820407.1697561128%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2820184.1697560951.1@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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="v1-fix-timetz-modulo-ops.patch";
	charset="us-ascii"
Content-ID: <2820184.1697560951.2@sss.pgh.pa.us>
Content-Description: v1-fix-timetz-modulo-ops.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 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 =3D (TimeTzADT *) palloc(sizeof(TimeTzADT));
 =

 	result->time =3D 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 +=3D USECS_PER_DAY;
-	while (result->time >=3D USECS_PER_DAY)
-		result->time -=3D USECS_PER_DAY;
+	if (result->time >=3D USECS_PER_DAY)
+		result->time %=3D USECS_PER_DAY;
 =

 	result->zone =3D tz;
 =

@@ -3116,10 +3117,11 @@ timetz_izone(PG_FUNCTION_ARGS)
 	result =3D (TimeTzADT *) palloc(sizeof(TimeTzADT));
 =

 	result->time =3D 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 +=3D USECS_PER_DAY;
-	while (result->time >=3D USECS_PER_DAY)
-		result->time -=3D USECS_PER_DAY;
+	if (result->time >=3D USECS_PER_DAY)
+		result->time %=3D USECS_PER_DAY;
 =

 	result->zone =3D tz;
 =

diff --git a/src/test/regress/expected/timetz.out b/src/test/regress/expec=
ted/timetz.out
index 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:5=
9: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=
.sql
index 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;

------- =_aaaaaaaaaa0--