From 43746996399541ecb5c7b188725a5f097c15ceae Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 29 Jul 2022 16:31:57 -0400 Subject: [PATCH] Fix brown paper bag bug in bbe08b8869bd29d587f24ef18eb45c7d4d14afca. We must issue the TRUNCATE command first and update relfrozenxid and relminmxid afterward; otherwise, TRUNCATE overwrites the previously-set values. Add a test case like I should have done the first time. Per buildfarm report from TestUpgradeXversion.pm, by way of Tom Lane. --- src/bin/pg_dump/pg_dump.c | 18 +++++----- src/bin/pg_upgrade/t/002_pg_upgrade.pl | 46 ++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 320bf51e4d..da6605175a 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -3141,7 +3141,7 @@ dumpDatabase(Archive *fout) PGresult *lo_res; PQExpBuffer loFrozenQry = createPQExpBuffer(); PQExpBuffer loOutQry = createPQExpBuffer(); - PQExpBuffer loVacQry = createPQExpBuffer(); + PQExpBuffer loHorizonQry = createPQExpBuffer(); int i_relfrozenxid, i_relfilenode, i_oid, @@ -3168,14 +3168,14 @@ dumpDatabase(Archive *fout) i_relfilenode = PQfnumber(lo_res, "relfilenode"); i_oid = PQfnumber(lo_res, "oid"); - appendPQExpBufferStr(loOutQry, "\n-- For binary upgrade, set pg_largeobject relfrozenxid and relminmxid\n"); - appendPQExpBufferStr(loVacQry, "\n-- For binary upgrade, preserve pg_largeobject and index relfilenodes\n"); + appendPQExpBufferStr(loHorizonQry, "\n-- For binary upgrade, set pg_largeobject relfrozenxid and relminmxid\n"); + appendPQExpBufferStr(loOutQry, "\n-- For binary upgrade, preserve pg_largeobject and index relfilenodes\n"); for (int i = 0; i < PQntuples(lo_res); ++i) { Oid oid; RelFileNumber relfilenumber; - appendPQExpBuffer(loOutQry, "UPDATE pg_catalog.pg_class\n" + appendPQExpBuffer(loHorizonQry, "UPDATE pg_catalog.pg_class\n" "SET relfrozenxid = '%u', relminmxid = '%u'\n" "WHERE oid = %u;\n", atooid(PQgetvalue(lo_res, i, i_relfrozenxid)), @@ -3186,18 +3186,18 @@ dumpDatabase(Archive *fout) relfilenumber = atooid(PQgetvalue(lo_res, i, i_relfilenode)); if (oid == LargeObjectRelationId) - appendPQExpBuffer(loVacQry, + appendPQExpBuffer(loOutQry, "SELECT pg_catalog.binary_upgrade_set_next_heap_relfilenode('%u'::pg_catalog.oid);\n", relfilenumber); else if (oid == LargeObjectLOidPNIndexId) - appendPQExpBuffer(loVacQry, + appendPQExpBuffer(loOutQry, "SELECT pg_catalog.binary_upgrade_set_next_index_relfilenode('%u'::pg_catalog.oid);\n", relfilenumber); } - appendPQExpBufferStr(loVacQry, + appendPQExpBufferStr(loOutQry, "TRUNCATE pg_catalog.pg_largeobject;\n"); - appendPQExpBufferStr(loOutQry, loVacQry->data); + appendPQExpBufferStr(loOutQry, loHorizonQry->data); ArchiveEntry(fout, nilCatalogId, createDumpId(), ARCHIVE_OPTS(.tag = "pg_largeobject", @@ -3208,8 +3208,8 @@ dumpDatabase(Archive *fout) PQclear(lo_res); destroyPQExpBuffer(loFrozenQry); + destroyPQExpBuffer(loHorizonQry); destroyPQExpBuffer(loOutQry); - destroyPQExpBuffer(loVacQry); } PQclear(res); diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl index 9ed48c4e06..09af8157d0 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -161,6 +161,27 @@ $newnode->command_ok( ], 'dump before running pg_upgrade'); +# Also record the relfrozenxid and relminmxid horizons. +my $horizon_query = <command_ok( + [ + 'psql', '-At', '-d', $oldnode->connstr('postgres'), + '-o', "$tempdir/horizon1.txt", '-c', $horizon_query, + ], + 'horizons before running pg_upgrade'); + # After dumping, update references to the old source tree's regress.so # to point to the new tree. if (defined($ENV{oldinstall})) @@ -294,6 +315,14 @@ $newnode->command_ok( ], 'dump after running pg_upgrade'); +# And second record of horizons as well. +$newnode->command_ok( + [ + 'psql', '-At', '-d', $newnode->connstr('postgres'), + '-o', "$tempdir/horizon2.txt", '-c', $horizon_query, + ], + 'horizons after running pg_upgrade'); + # Compare the two dumps, there should be no differences. my $compare_res = compare("$tempdir/dump1.sql", "$tempdir/dump2.sql"); is($compare_res, 0, 'old and new dumps match after pg_upgrade'); @@ -311,4 +340,21 @@ if ($compare_res != 0) print "=== EOF ===\n"; } +# Compare the horizons, there should be no differences. +$compare_res = compare("$tempdir/horizon1.txt", "$tempdir/horizon2.txt"); +is($compare_res, 0, 'old and new horizons match after pg_upgrade'); + +# Provide more context if the horizons do not match. +if ($compare_res != 0) +{ + my ($stdout, $stderr) = + run_command([ 'diff', "$tempdir/horizon1.txt", "$tempdir/horizon2.txt" ]); + print "=== diff of $tempdir/horizon1.txt and $tempdir/horizon2.txt\n"; + print "=== stdout ===\n"; + print $stdout; + print "=== stderr ===\n"; + print $stderr; + print "=== EOF ===\n"; +} + done_testing(); -- 2.39.5