On 27.12.24 05:42, Michael Paquier wrote:
On Tue, Dec 24, 2024 at 04:33:47PM +0900, Michael Paquier wrote:
I am planning to get this one applied around the end of this week on
Friday for HEAD, that should be enough if there are comments and/or
objections.
And done for now. If there are any remarks and/or objections, of
course feel free.
It turned out this had a bug, and also the newly added test cases didn't
actually cover the new code, otherwise this would have shown up.
Please review the attached patches with additional test cases and the fix.
See also [0] for further context:
[0]:
https://www.postgresql.org/message-id/8246d7ff-f4b7-4363-913e-827dadfeb145%40eisentraut.orgFrom 11eb6931c0903ebac40efd95cda201f341bec59b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 6 Aug 2025 12:20:44 +0200
Subject: [PATCH 1/2] Improve test coverage of date_trunc() on infinity
Commit d85ce012f99 added new error handling code to date_trunc() of
timestamp, timestamptz, and interval with infinite values. But the
new test cases added by that commit did not actually test the new
code, since they used the "not recognized" unit 'ago' but not a "not
supported" unit, which is what the new code was dealing with. This
patch adds some test cases to cover the new code. I'm using
'timezone' as the not supported unit.
---
src/test/regress/expected/timestamp.out | 10 ++++++++++
src/test/regress/expected/timestamptz.out | 12 ++++++++++++
src/test/regress/sql/timestamp.sql | 4 +++-
src/test/regress/sql/timestamptz.sql | 6 ++++--
4 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/src/test/regress/expected/timestamp.out
b/src/test/regress/expected/timestamp.out
index 6aaa19c8f4e..14a9f5b56a6 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -591,6 +591,16 @@ SELECT date_trunc( 'week', timestamp '2004-02-29
15:44:17.71393' ) AS week_trunc
Mon Feb 23 00:00:00 2004
(1 row)
+SELECT date_trunc( 'week', timestamp 'infinity' ) AS inf_trunc;
+ inf_trunc
+-----------
+ infinity
+(1 row)
+
+SELECT date_trunc( 'timezone', timestamp '2004-02-29 15:44:17.71393' ) AS
notsupp_trunc;
+ERROR: unit "timezone" not supported for type timestamp without time zone
+SELECT date_trunc( 'timezone', timestamp 'infinity' ) AS notsupp_inf_trunc;
+ERROR: unit "timezone" not supported for type timestamp without time zone
SELECT date_trunc( 'ago', timestamp 'infinity' ) AS invalid_trunc;
ERROR: unit "ago" not recognized for type timestamp without time zone
-- verify date_bin behaves the same as date_trunc for relevant intervals
diff --git a/src/test/regress/expected/timestamptz.out
b/src/test/regress/expected/timestamptz.out
index 2a69953ff25..953656affa4 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -760,6 +760,16 @@ SELECT date_trunc( 'week', timestamp with time zone
'2004-02-29 15:44:17.71393'
Mon Feb 23 00:00:00 2004 PST
(1 row)
+SELECT date_trunc( 'week', timestamp with time zone 'infinity' ) AS inf_trunc;
+ inf_trunc
+-----------
+ infinity
+(1 row)
+
+SELECT date_trunc( 'timezone', timestamp with time zone '2004-02-29
15:44:17.71393' ) AS notsupp_trunc;
+ERROR: unit "timezone" not supported for type timestamp with time zone
+SELECT date_trunc( 'timezone', timestamp with time zone 'infinity' ) AS
notsupp_inf_trunc;
+ERROR: unit "timezone" not supported for type timestamp with time zone
SELECT date_trunc( 'ago', timestamp with time zone 'infinity' ) AS
invalid_trunc;
ERROR: unit "ago" not recognized for type timestamp with time zone
SELECT date_trunc('day', timestamp with time zone '2001-02-16 20:38:40+00',
'Australia/Sydney') as sydney_trunc; -- zone name
@@ -780,6 +790,8 @@ SELECT date_trunc('day', timestamp with time zone
'2001-02-16 20:38:40+00', 'VET
Thu Feb 15 20:00:00 2001 PST
(1 row)
+SELECT date_trunc('timezone', timestamp with time zone 'infinity', 'GMT') AS
notsupp_zone_trunc;
+ERROR: unit "timezone" not supported for type timestamp with time zone
SELECT date_trunc('ago', timestamp with time zone 'infinity', 'GMT') AS
invalid_zone_trunc;
ERROR: unit "ago" not recognized for type timestamp with time zone
-- verify date_bin behaves the same as date_trunc for relevant intervals
diff --git a/src/test/regress/sql/timestamp.sql
b/src/test/regress/sql/timestamp.sql
index 55f80530ea0..313757ed041 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -175,7 +175,9 @@ CREATE TABLE TIMESTAMP_TBL (d1 timestamp(2) without time
zone);
FROM TIMESTAMP_TBL WHERE d1 BETWEEN '1902-01-01' AND '2038-01-01';
SELECT date_trunc( 'week', timestamp '2004-02-29 15:44:17.71393' ) AS
week_trunc;
-
+SELECT date_trunc( 'week', timestamp 'infinity' ) AS inf_trunc;
+SELECT date_trunc( 'timezone', timestamp '2004-02-29 15:44:17.71393' ) AS
notsupp_trunc;
+SELECT date_trunc( 'timezone', timestamp 'infinity' ) AS notsupp_inf_trunc;
SELECT date_trunc( 'ago', timestamp 'infinity' ) AS invalid_trunc;
-- verify date_bin behaves the same as date_trunc for relevant intervals
diff --git a/src/test/regress/sql/timestamptz.sql
b/src/test/regress/sql/timestamptz.sql
index caca3123f13..e01618907e0 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -217,15 +217,17 @@ CREATE TABLE TIMESTAMPTZ_TBL (d1 timestamp(2) with time
zone);
FROM TIMESTAMPTZ_TBL WHERE d1 BETWEEN '1902-01-01' AND '2038-01-01';
SELECT date_trunc( 'week', timestamp with time zone '2004-02-29
15:44:17.71393' ) AS week_trunc;
+SELECT date_trunc( 'week', timestamp with time zone 'infinity' ) AS inf_trunc;
+SELECT date_trunc( 'timezone', timestamp with time zone '2004-02-29
15:44:17.71393' ) AS notsupp_trunc;
+SELECT date_trunc( 'timezone', timestamp with time zone 'infinity' ) AS
notsupp_inf_trunc;
SELECT date_trunc( 'ago', timestamp with time zone 'infinity' ) AS
invalid_trunc;
SELECT date_trunc('day', timestamp with time zone '2001-02-16 20:38:40+00',
'Australia/Sydney') as sydney_trunc; -- zone name
SELECT date_trunc('day', timestamp with time zone '2001-02-16 20:38:40+00',
'GMT') as gmt_trunc; -- fixed-offset abbreviation
SELECT date_trunc('day', timestamp with time zone '2001-02-16 20:38:40+00',
'VET') as vet_trunc; -- variable-offset abbreviation
+SELECT date_trunc('timezone', timestamp with time zone 'infinity', 'GMT') AS
notsupp_zone_trunc;
SELECT date_trunc('ago', timestamp with time zone 'infinity', 'GMT') AS
invalid_zone_trunc;
-
-
-- verify date_bin behaves the same as date_trunc for relevant intervals
SELECT
str,
--
2.50.1
From 4f9e5dde59efc00c682f14a3047fd432c87f4209 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 6 Aug 2025 12:37:41 +0200
Subject: [PATCH 2/2] Fix incorrect Datum conversion in
timestamptz_trunc_internal()
It used PG_RETURN_TIMESTAMPTZ(), but the return type is TimestampTz,
not Datum. On 64-bit systems, there is no effect, since this just
ends up casting 64-bit integers back and forth. But on 32-bit
systems, timestamptz is pass-by-reference, and so
PG_RETURN_TIMESTAMPTZ() allocates new memory and returns the address,
but the caller will try to interpret this as a timestamp value. The
effect is that date_trunc(..., 'infinity'::timestamptz) will return
random values (instead of the correct return value 'infinity').
The fix is to use a straight "return" call.
Commit d85ce012f99 added this code but provided tests that did not
cover this correctly. This was fixed in the previous patch.
Reviewed-by: Tom Lane <t...@sss.pgh.pa.us>
Discussion:
https://www.postgresql.org/message-id/flat/8246d7ff-f4b7-4363-913e-827dadfeb145%40eisentraut.org
---
src/backend/utils/adt/timestamp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/timestamp.c
b/src/backend/utils/adt/timestamp.c
index 25cff56c3d0..e640b48205b 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -4954,7 +4954,7 @@ timestamptz_trunc_internal(text *units, TimestampTz
timestamp, pg_tz *tzp)
case DTK_SECOND:
case DTK_MILLISEC:
case DTK_MICROSEC:
- PG_RETURN_TIMESTAMPTZ(timestamp);
+ return timestamp;
break;
default:
--
2.50.1
