Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 7 Aug 2025 09:06:25 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: Joseph Koshakow <koshy44@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: date_trunc invalid units with infinite value
Message-ID: <aJPuAadlimgSac5I@paquier.xyz>
References: 
 <CAAvxfHc4084dGzEJR0_pBZkDuqbPGc5wn7gK_M0XR_kRiCdUJQ@mail.gmail.com>
 <Z1E7cTyVbFZJwUWf@paquier.xyz>
 <Z2pj22b9qZryFIRo@paquier.xyz>
 <Z24wLjuagL5T4hOC@paquier.xyz>
 <4bf60a84-2862-4a53-acd5-8eddf134a60e@eisentraut.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="GilEzjPSs0C2ER9z"
Content-Disposition: inline
In-Reply-To: <4bf60a84-2862-4a53-acd5-8eddf134a60e@eisentraut.org>
Archived-At: 
 <https://www.postgresql.org/message-id/aJPuAadlimgSac5I%40paquier.xyz>
Precedence: bulk


--GilEzjPSs0C2ER9z
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Aug 06, 2025 at 01:07:25PM +0200, Peter Eisentraut wrote:
> 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.
>=20
> Please review the attached patches with additional test cases and the fix.
>=20
> See also [0] for further context:
>=20
> [0]: https://www.postgresql.org/message-id/8246d7ff-f4b7-4363-913e-827dad=
feb145%40eisentraut.org

Yes, confirmed the broken case on 32-bit builds with the incorrect
result returned by timestamptz_trunc_internal():
SELECT date_trunc( 'week', timestamp with time zone 'infinity' );

And confirmed that we don't have any coverage for two code paths as of
HEAD:
- "not supported" in timestamp_trunc() for the entire new section of
where TIMESTAMP_NOT_FINITE() is satisfied.
- "not supported" in timestamptz_trunc_internal() for the entire
section where TIMESTAMP_NOT_FINITE() is satisfied.

0001 adds three new tests for timestamp:
- TIMESTAMP_NOT_FINITE + a valid unit, new path.
- TIMESTAMP_NOT_FINITE + "not supported" unit, new error path
- !TIMESTAMP_NOT_FINITE + "not supported" unit, old error path

0001 four new tests for timestamptz:
1) Three tests for timestamptz_trunc():
- TIMESTAMP_NOT_FINITE + a valid unit, new path.
- TIMESTAMP_NOT_FINITE + "not supported" unit, new path.
- !TIMESTAMP_NOT_FINITE +
2) One test for timestamptz_trunc_zone():
- !TIMESTAMP_NOT_FINITE + "not supported" unit

With what I am reading in your patch, what you are suggesting to add,
and a double-check at the interval tests, that seems complete to me.

This is a v18 open item, for something that I am an owner of as the
committer of d85ce012f99f, so I'll go take care of it.  Thanks!
--
Michael

--GilEzjPSs0C2ER9z
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmiT7gEACgkQnvQgOdby
QH2jhQ/6ArXZuRWlR3KjMMHF3d2WAc8evob7pPGR/j6scQZEPu5DR1QoCZYoZ3+x
tkcvz2LXcz7xmPwnAXLzwFQ5SjES3O4outqaGdQsyEESZ9v5i7NR3oru0rDMyZ04
Ws/OQUWst9w5CHNOX0ER/pqkg/EZ5KMSQtPcoB9w6mPVZ5y3EbJ5cm+PWDMEfcWb
lQqiBT9woNG0b3S9wcqIoALTCjaZctYlSogfg9+o3AIaKrCtMZDGKGjzngcHM/eP
Y1TNSzBBOwfzf5bzLQNQDbr542bNRJUtci75K08wzIFCxUwxHcy8CDgD0lEsUk8W
SJm8tCotcsbNFxLGgSkuAZYUcy2ZjQoS9/HQxEEdgf2C4k1KK42rgTxgZP8DDfm2
P56hpUWbFmieTsOKsms7W4/NTZhoBVsEBh3TRCtXmKTna+mCZvavyjT7CCZmoPe7
31DuPEVRcbN3J8+FPe1cl8/gqLYGL842xmE0HK+RVU4utn0aH4gpbXgL2qOGgt8C
zRWz+V7ChpzBKLUWB/113Fe9QCfcY18hmAUpH/Xw1Pk0ztsPUSvjKj/k6HkVWx4V
BSoOFNR2045F7ailsdpnjRiV6ygFzMPjus43dlQC/kO4ZuDBAJOgYBQnkWtc6NfB
xFNLL01taOKM/d96VpG47fAbY632YPwm1jXJOwcn8d29xZATZWg=
=O5tH
-----END PGP SIGNATURE-----

--GilEzjPSs0C2ER9z--