From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jeff Davis <pgsql@j-davis.com>
cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Further further fix pg_upgrade crossversion test for
 adminpack.
In-reply-to: <bca97aecb50b2026b7dbc26604bf31861c819a64.camel@j-davis.com>
References: <E1rhClo-0024Gh-8k@gemulon.postgresql.org>
 <bca97aecb50b2026b7dbc26604bf31861c819a64.camel@j-davis.com>
Comments: In-reply-to Jeff Davis <pgsql@j-davis.com>
	message dated "Fri, 08 Mar 2024 23:48:08 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1066871.1710006597.1@sss.pgh.pa.us>
Date: Sat, 09 Mar 2024 12:49:57 -0500
Message-ID: <1066872.1710006597@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1066872.1710006597%40sss.pgh.pa.us>
Precedence: bulk

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