Re: Missing update of all_hasnulls in BRIN opclasses

On 4/24/23 17:46, Tom Lane wrote:
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
>> On 2023-Apr-23, Tomas Vondra wrote:
>>> Both cases have a patch extending pageinspect to show the new flag, but
>>> obviously we should commit that only in master. In backbranches it's
>>> meant only to make testing easier.
>
>> In backbranches, I think it should be reasonably easy to add a
>> --1.7--1.7.1.sql file and set the default version to 1.7.1; that then
>> enables us to have the functionality (and the tests) in older branches
>> too. If you just add a --1.X.1--1.12.sql version to each branch that's
>> identical to that branch's current pageinspect version upgrade script,
>> that would let us have working upgrade paths for all branches. This is
>> a bit laborious but straightforward enough.
>
> "A bit laborious"? That seems enormously out of proportion to the
> benefit of putting this test case into back branches. It will have
> costs for end users too, not only us. As an example, it would
> unecessarily block some upgrade paths, if the upgraded-to installation
> is slightly older and lacks the necessary --1.X.1--1.12 script.
>

Why would that block the upgrade? Presumably we'd add two upgrade
scripts in the master, to allow upgrade both from 1.X and 1.X.1.

>> If you don't feel like adding that, I volunteer to add the necessary
>> scripts to all branches after you commit the bugfix, and ensure that all
>> upgrade paths work correctly.
>
> I do not think this should happen at all, whether you're willing to
> do the work or not.

FWIW I'm fine with not doing that. As mentioned, I only included this
patch to make testing the patch easier (otherwise the flag is impossible
to inspect directly).

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company