pgsql: Further further fix pg_upgrade crossversion test for adminpack.

On Mon, 2024-03-04 at 18:10 +0000, Tom Lane wrote:

Further further fix pg_upgrade crossversion test for adminpack.

Ever since this commit the cross-version upgrade test is failing (for
me, at least) with:

# Running: psql -X -v ON_ERROR_STOP=1 -c drop database if exists
contrib_regression_adminpack;
drop database if exists regression_adminpack -d port=53977
host=/tmp/EK6UT_TufI dbname='postgres'
ERROR: DROP DATABASE cannot run inside a transaction block

It looks like when you added another command, the two were joined with
";\n", which ends up running the commands in a transaction block, which
doesn't work for DROP DATABASE.

I'm not sure how this test is succeeding for others, so perhaps I'm
doing something wrong?

Patch attached, though I'm not particularly great with perl and the
array flattening in my implementation might be too magical.

Regards,
Jeff Davis

Jeff Davis <pgsql@j-davis.com> writes:

Ever since this commit the cross-version upgrade test is failing (for
me, at least) with:

# Running: psql -X -v ON_ERROR_STOP=1 -c drop database if exists
contrib_regression_adminpack;
drop database if exists regression_adminpack -d port=53977
host=/tmp/EK6UT_TufI dbname='postgres'
ERROR: DROP DATABASE cannot run inside a transaction block

Ugh.

I'm not sure how this test is succeeding for others, so perhaps I'm
doing something wrong?

I've only tested it in the context of the buildfarm's script, which
implements this by dumping the given commands into a temp file
and invoking psql with -f. So that's fine with multiple DROP
DATABASE commands, but what 002_pg_upgrade.pl is doing isn't.
I guess you are running that in some way that the rest of us aren't.

Patch attached, though I'm not particularly great with perl and the
array flattening in my implementation might be too magical.

I'm no perl maven either, but this looks fine to me.

Now that I look at it, I see that I missed a bet in the additions
to AdjustUpgrade.pm: rather than using IF EXISTS it should be
doing something more like the pre-existing logic

if ($dbnames{"contrib_regression_$bad_module"})
{
_add_st($result, 'postgres',
"drop database contrib_regression_$bad_module");
delete($dbnames{"contrib_regression_$bad_module"});
}

However, we'd inevitably hit the more-than-one-DB-to-drop problem
eventually, so better to fix that now. Please push your fix and
then I'll clean up the unnecessary DROPs.

regards, tom lane

On Sat, 2024-03-09 at 12:49 -0500, Tom Lane wrote:

I guess you are running that in some way that the rest of us aren't.

FWIW, I am following the directions in src/bin/pg_upgrade/TESTING.

However, we'd inevitably hit the more-than-one-DB-to-drop problem
eventually, so better to fix that now.  Please push your fix and
then I'll clean up the unnecessary DROPs.

Thank you for looking. Pushed, and separately ran perltidy.

Regards,
Jeff Davis