Re: pg_upgrade test for binary compatibility of core data types

On Fri, Oct 01, 2021 at 04:58:41PM +0900, Michael Paquier wrote:
> I was looking at this CF entry, and what you are doing in 0004 to move
> the tweaks from pg_upgrade's test.sh to a separate SQL script that
> uses psql's meta-commands like \if to check which version we are on is
> really interesting. The patch does not apply anymore, so this needs a
> rebase. The entry has been switched as waiting on author by Tom, but
> you did not update it after sending the new versions in [1]. I am
> wondering if we could have something cleaner than just a set booleans
> as you do here for each check, as that does not help with the
> readability of the tests.

And so, I am back at this thread, looking at the set of patches
proposed from 0001 to 0004. The patches are rather messy and mix many
things and concepts, but there are basically four things that stand
out:
- test.sh is completely broken when using PG >= 14 as new version
because of the removal of the test tablespace. Older versions of
pg_regress don't support --make-tablespacedir so I am fine to stick a
couple of extra mkdirs for testtablespace/, expected/ and sql/ to
allow the script to work properly for major upgrades as a workaround,
but only if we use an old version. We need to do something here for
HEAD and REL_14_STABLE.
- The script would fail when using PG <= 11 as old version because of
WITH OIDS relations. We need to do something down to REL_12_STABLE.
I did not like much the approach taken to stick 4 ALTER TABLE queries
though (the patch was actually failing here for me), so instead I have
borrowed what the buildfarm has been doing with a DO block. That
works fine, and that's more portable.
- Not using --extra-float-digits with PG <= 11 as older version causes
a bunch of diffs in the dumps, making the whole unreadable. The patch
was doing that unconditionally for *all version*, which is not good.
We should only do that on the versions that need it, and we know the
old version number before taking any dumps so that's easy to check.
- The addition of --wal-segsize and --allow-group-access breaks the
script when using PG < 10 at initdb time as these got added in 11.
With 10 getting EOL'd next year and per the lack of complaints, I am
not excited to do anything here and I'd rather leave this out so as we
keep coverage for those options across *all* major versions upgraded
from 11~. The buildfarm has tests down to 9.2, but for devs my take
is that this is enough for now.

This is for the basics in terms of fixing test.sh and what should be
backpatched. In this aspect patches 0001 and 0002 were a bit
incorrect. I am not sure that 0003 is that interesting as designed as
we would miss any new core types introduced.

0004 is something I'd like to get done on HEAD to ease the move of the
pg_upgrade tests to TAP, but it could be made a bit easier to read by
not having all those oldpgversion_XX_YY flags grouped together for
one. So I am going to rewrite portions of it once done with the
above.

For now, attached is a patch to address the issues with test.sh that I
am planning to backpatch. This fixes the facility on HEAD, while
minimizing the diffs between the dumps. We could do more, like a
s/PROCEDURE/FUNCTION/ but that does not make the diffs really
unreadable either. I have only tested that on HEAD as new version
down to 11 as the oldest version per the business with --wal-segsize.
This still needs tests with 12~ as new version though, which is boring
but not complicated at all :)
--
Michael