Re: pg_upgrade test for binary compatibility of core data types

On Sun, Nov 07, 2021 at 01:22:00PM -0600, Justin Pryzby wrote:
> That may be good enough for test.sh, but if the kludges were moved to a .sql
> script which was also run by the buildfarm (in stead of its hardcoded kludges), then
> it might be necessary to handle the additional stuff my patch did, like:
>
> + DROP TRANSFORM FOR integer LANGUAGE sql CASCADE;"
> + DROP FUNCTION boxarea(box);"
> + DROP FUNCTION funny_dup17();"

These apply for an old version <= v10.

> + DROP TABLE abstime_tbl;"
> + DROP TABLE reltime_tbl;"
> + DROP TABLE tinterval_tbl;"

old version <= 9.3.

> + DROP AGGREGATE first_el_agg_any(anyelement);"

Not sure about this one.

> + DROP AGGREGATE array_cat_accum(anyarray);"
> + DROP OPERATOR @#@(NONE,bigint);"

These are on 9.4. It is worth noting that TestUpgradeXversion.pm
recreates those objects. I'd agree to close the gap completely rather
than just moving what test.sh does to wipe out a maximum client code
for the buildfarm.

> Or, maybe it's guaranteed that the animals all run latest version of old
> branches, in which case I think some of the BF's existing logic could be
> dropped, which would help to reconcile these two scripts:

That seems like a worthy goal to reduce the amount of duplication with
the buildfarm code, while allowing tests from upgrades with older
versions (the WAL segment size and group permission issue in test.sh
had better be addressed in a better way, perhaps once the pg_upgrade
tests are moved to TAP). There are also things specific to contrib/
modules with older versions, but that may be too specific for this
exercise.

+\if :oldpgversion_le84
+DROP FUNCTION public.myfunc(integer);
+\endif

The oldest version tested by the buildfarm is 9.2, so we could ignore
this part I guess?

Andrew, what do you think about this part? Based on my read of this
thread, there is an agreement that this approach makes the buildfarm
code more manageable so as committers would not need to patch the
buildfarm code if their test fail. I agree with this conclusion, but
I wanted to double-check with you first. This would need a backpatch
down to 10 so as we could clean up a maximum of code in
TestUpgradeXversion.pm without waiting for an extra 5 years. Please
note that I am fine to send a patch for the buildfarm client.

> We wouldn't miss new core types, because of the 2nd part of type_sanity which
> tests that each core type was included in the "manytypes" table.

Thanks, I see your point now after a closer read.

There is still a pending question for contrib modules, but I think
that we need to think larger here with a better integration of
contrib/ modules in the upgrade testing process. Making that cheap
would require running the set of regression tests on the instance
to-be-upgraded first. I think that one step in this direction would
be to have unique databases for each contrib/ modules, so as there is
no overlap with objects dropped?

Having some checks with code types looks fine as a first step, so
let's do that. I have reviewed 0001, rewrote a couple of comments.
All the comments from upthread seem to be covered with that. So I'd
like to get that applied on HEAD. We could as well be less
conservative and backpatch that down to 12 to follow on 7c15cef so we
would be more careful with 15~ already (a backpatch down to 14 would
be enough for this purpose, actually thanks to the 14->15 upgrade
path).
--
Michael