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.

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e00ee5b288f878c06ed1dc54401741dc3fbcd75f..0410211243a0210f0f0121c455eb7962f2594df1 100644 (file)
--- 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;
            Oid     relfilenode;
 
-           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)
            relfilenode = 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",
                                  relfilenode);
            else if (oid == LargeObjectLOidPNIndexId)
-               appendPQExpBuffer(loVacQry,
+               appendPQExpBuffer(loOutQry,
                                  "SELECT pg_catalog.binary_upgrade_set_next_index_relfilenode('%u'::pg_catalog.oid);\n",
                                  relfilenode);
        }
 
-       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 9ed48c4e06aa0a75c836253ff2735a960647af99..09af8157d0dcf010e166bafc3fb57e43a20f03a6 100644 (file)
--- 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 = <
+SELECT
+  c.oid::regclass, c.relfrozenxid, c.relminmxid
+FROM
+  pg_class c, pg_namespace n
+WHERE
+  c.relnamespace = n.oid AND
+  ((n.nspname !~ '^pg_temp_' AND n.nspname !~ '^pg_toast_temp_' AND
+    n.nspname NOT IN ('pg_catalog', 'information_schema', 'binary_upgrade',
+                      'pg_toast'))
+  OR (n.nspname = 'pg_catalog' AND relname IN ('pg_largeobject')))
+EOM
+$horizon_query =~ s/\s+/ /g; # run it together on one line
+$newnode->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();