pgsql: Set the volatility of the timestamptz version of date_bin() back

Started by John Naylorover 4 years ago7 messageshackers
Jump to latest
#1John Naylor
john.naylor@enterprisedb.com

Set the volatility of the timestamptz version of date_bin() back to immutable

543f36b43d was too hasty in thinking that the volatility of date_bin()
had to match date_trunc(), since only the latter references
session_timezone.

Bump catversion

Per feedback from Aleksander Alekseev
Backpatch to v14, as the former commit was

Branch
------
REL_14_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/67c33a114f38edbd66f68d7b2c0cb7b03611ec48

Modified Files
--------------
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_proc.dat | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: John Naylor (#1)
Re: pgsql: Set the volatility of the timestamptz version of date_bin() back

On 2021-Sep-03, John Naylor wrote:

Set the volatility of the timestamptz version of date_bin() back to immutable

543f36b43d was too hasty in thinking that the volatility of date_bin()
had to match date_trunc(), since only the latter references
session_timezone.

Bump catversion

These catversion bumps in branch 14 this late in the cycle seem suspect.
Didn't we have some hesitation to push multirange unnest around beta2
precisely because of a desire to avoid catversion bumps?

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/

#3John Naylor
john.naylor@enterprisedb.com
In reply to: Alvaro Herrera (#2)
Re: pgsql: Set the volatility of the timestamptz version of date_bin() back

On Fri, Sep 3, 2021 at 1:46 PM Alvaro Herrera <alvherre@alvh.no-ip.org>
wrote:

On 2021-Sep-03, John Naylor wrote:
These catversion bumps in branch 14 this late in the cycle seem suspect.
Didn't we have some hesitation to push multirange unnest around beta2
precisely because of a desire to avoid catversion bumps?

This was for correcting a mistake (although the first commit turned out to
be a mistake itself), so I understood it to be necessary.

--
John Naylor
EDB: http://www.enterprisedb.com

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: John Naylor (#3)
Re: pgsql: Set the volatility of the timestamptz version of date_bin() back

On 2021-Sep-03, John Naylor wrote:

On Fri, Sep 3, 2021 at 1:46 PM Alvaro Herrera <alvherre@alvh.no-ip.org>
wrote:

On 2021-Sep-03, John Naylor wrote:
These catversion bumps in branch 14 this late in the cycle seem suspect.
Didn't we have some hesitation to push multirange unnest around beta2
precisely because of a desire to avoid catversion bumps?

This was for correcting a mistake (although the first commit turned out to
be a mistake itself), so I understood it to be necessary.

A crazy idea might have been to return to the original value.

--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/

#5Justin Pryzby
pryzby@telsasoft.com
In reply to: Alvaro Herrera (#4)
Re: pgsql: Set the volatility of the timestamptz version of date_bin() back

On Fri, Sep 03, 2021 at 01:56:50PM -0400, Alvaro Herrera wrote:

On 2021-Sep-03, John Naylor wrote:

On Fri, Sep 3, 2021 at 1:46 PM Alvaro Herrera <alvherre@alvh.no-ip.org>
wrote:

On 2021-Sep-03, John Naylor wrote:
These catversion bumps in branch 14 this late in the cycle seem suspect.
Didn't we have some hesitation to push multirange unnest around beta2
precisely because of a desire to avoid catversion bumps?

This was for correcting a mistake (although the first commit turned out to
be a mistake itself), so I understood it to be necessary.

A crazy idea might have been to return to the original value.

+1. I think the catversion usually is always increased even in a "revert", but
in this exceptional case [0]two commits close to each other, with no other catalog changes, and with the specific goal of allowing trivial upgrade from b3. it would be nice if beta4/rc1 had the same number
as b3.

[0]: two commits close to each other, with no other catalog changes, and with the specific goal of allowing trivial upgrade from b3.
the specific goal of allowing trivial upgrade from b3.

--
Justin

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: John Naylor (#1)
Re: pgsql: Set the volatility of the timestamptz version of date_bin() back

John Naylor <john.naylor@postgresql.org> writes:

Set the volatility of the timestamptz version of date_bin() back to immutable
543f36b43d was too hasty in thinking that the volatility of date_bin()
had to match date_trunc(), since only the latter references
session_timezone.

Bump catversion

What you should have done here, at least in the back branch, was *revert*
catversion to what it had been. As things stand, it would force users of
14beta3 to initdb or pg_upgrade to move to 14.0, for no reason whatsoever.

regards, tom lane

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#4)
Re: pgsql: Set the volatility of the timestamptz version of date_bin() back

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

On 2021-Sep-03, John Naylor wrote:

On Fri, Sep 3, 2021 at 1:46 PM Alvaro Herrera <alvherre@alvh.no-ip.org>
wrote:

These catversion bumps in branch 14 this late in the cycle seem suspect.
Didn't we have some hesitation to push multirange unnest around beta2
precisely because of a desire to avoid catversion bumps?

This was for correcting a mistake (although the first commit turned out to
be a mistake itself), so I understood it to be necessary.

A crazy idea might have been to return to the original value.

Yes, that is what should have been done, as I complained over
in pgsql-committers before seeing this exchange. As things
stand, a pg_upgrade is going to be forced on beta3 users
without even the excuse of fixing a bug.

regards, tom lane