pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Further further fix pg_upgrade crossversion test for adminpack. 4+ messages / 2 participants [nested] [flat]
Commits from this thread: 2024-03-09 519443162d Simplify and merge unwanted-module drop logic in AdjustUpgrade.pm. 2024-03-09 5ba8b70deb Fix cross-version pg_upgrade test.
* pgsql: Further further fix pg_upgrade crossversion test for adminpack. @ 2024-03-04 18:10 Tom Lane <tgl@sss.pgh.pa.us> 2024-03-09 07:48 ` Re: pgsql: Further further fix pg_upgrade crossversion test for adminpack. Jeff Davis <pgsql@j-davis.com> 0 siblings, 1 reply; 4+ messages in thread From: Tom Lane @ 2024-03-04 18:10 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Further further fix pg_upgrade crossversion test for adminpack. Apparently, buildfarm animal crake has the adminpack regression DB named as "regression_adminpack" in some branches. Not clear why I didn't see that when testing here. In any case, drop that too. Discussion: https://postgr.es/m/0CFB76D0-0510-48B2-9916-1199F93BC28C@yesql.se Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/cb6945dc8061d03e6ec670a6856228f12e94264c Modified Files -------------- src/test/perl/PostgreSQL/Test/AdjustUpgrade.pm | 3 +++ 1 file changed, 3 insertions(+) ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pgsql: Further further fix pg_upgrade crossversion test for adminpack. 2024-03-04 18:10 pgsql: Further further fix pg_upgrade crossversion test for adminpack. Tom Lane <tgl@sss.pgh.pa.us> @ 2024-03-09 07:48 ` Jeff Davis <pgsql@j-davis.com> 2024-03-09 17:49 ` Re: pgsql: Further further fix pg_upgrade crossversion test for adminpack. Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 1 reply; 4+ messages in thread From: Jeff Davis @ 2024-03-09 07:48 UTC (permalink / raw) To: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org 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 Attachments:^ permalink raw reply [nested|flat] 4+ messages in thread0001-Fix-cross-version-pg_upgrade-test.patch (text/x-patch, 1.1K) download
From 54e13ddb01b859606ebe938a98587aef666df668 Mon Sep 17 00:00:00 2001 From: Jeff Davis <jeff@j-davis.com> Date: Fri, 8 Mar 2024 23:37:16 -0800 Subject: [PATCH] Fix cross-version pg_upgrade test. --- src/bin/pg_upgrade/t/002_pg_upgrade.pl | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl index d951ed3af0..16b42d475f 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -246,15 +246,19 @@ if (defined($ENV{oldinstall})) foreach my $updb (keys %$adjust_cmds) { - my $upcmds = join(";\n", @{ $adjust_cmds->{$updb} }); + my @command_args = (); + for my $upcmd (@{ $adjust_cmds->{$updb} }) + { + push @command_args, '-c', $upcmd; + } # For simplicity, use the newer version's psql to issue the commands. $newnode->command_ok( [ 'psql', '-X', '-v', 'ON_ERROR_STOP=1', - '-c', $upcmds, '-d', $oldnode->connstr($updb), + @command_args, ], "ran version adaptation commands for database $updb"); } -- 2.34.1
* Re: pgsql: Further further fix pg_upgrade crossversion test for adminpack. 2024-03-04 18:10 pgsql: Further further fix pg_upgrade crossversion test for adminpack. Tom Lane <tgl@sss.pgh.pa.us> 2024-03-09 07:48 ` Re: pgsql: Further further fix pg_upgrade crossversion test for adminpack. Jeff Davis <pgsql@j-davis.com> @ 2024-03-09 17:49 ` Tom Lane <tgl@sss.pgh.pa.us> 2024-03-09 19:55 ` Re: pgsql: Further further fix pg_upgrade crossversion test for adminpack. Jeff Davis <pgsql@j-davis.com> 0 siblings, 1 reply; 4+ messages in thread From: Tom Lane @ 2024-03-09 17:49 UTC (permalink / raw) To: Jeff Davis <pgsql@j-davis.com>; +Cc: pgsql-committers@lists.postgresql.org 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 ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pgsql: Further further fix pg_upgrade crossversion test for adminpack. 2024-03-04 18:10 pgsql: Further further fix pg_upgrade crossversion test for adminpack. Tom Lane <tgl@sss.pgh.pa.us> 2024-03-09 07:48 ` Re: pgsql: Further further fix pg_upgrade crossversion test for adminpack. Jeff Davis <pgsql@j-davis.com> 2024-03-09 17:49 ` Re: pgsql: Further further fix pg_upgrade crossversion test for adminpack. Tom Lane <tgl@sss.pgh.pa.us> @ 2024-03-09 19:55 ` Jeff Davis <pgsql@j-davis.com> 0 siblings, 0 replies; 4+ messages in thread From: Jeff Davis @ 2024-03-09 19:55 UTC (permalink / raw) To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org 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 ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2024-03-09 19:55 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-03-04 18:10 pgsql: Further further fix pg_upgrade crossversion test for adminpack. Tom Lane <tgl@sss.pgh.pa.us> 2024-03-09 07:48 ` Jeff Davis <pgsql@j-davis.com> 2024-03-09 17:49 ` Tom Lane <tgl@sss.pgh.pa.us> 2024-03-09 19:55 ` Jeff Davis <pgsql@j-davis.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox