Date: Sat, 2 Jul 2022 10:49:17 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org, Bruce Momjian <bruce@momjian.us>,
	Shruthi Gowda <gowdashru@gmail.com>
Subject: Re: pg15b2: large objects lost on upgrade
Message-ID: <20220702154917.GM13040@telsasoft.com>
References: <20220701231413.GI13040@telsasoft.com>
 <CA+TgmobwWGO98mWB1X2xpo25gatKsjpENtnYaUFcvVs7raRQPg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="ieNMXl1Fr3cevapt"
Content-Disposition: inline
In-Reply-To: 
 <CA+TgmobwWGO98mWB1X2xpo25gatKsjpENtnYaUFcvVs7raRQPg@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220702154917.GM13040%40telsasoft.com>
Precedence: bulk


--ieNMXl1Fr3cevapt
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Sat, Jul 02, 2022 at 08:34:04AM -0400, Robert Haas wrote:
> On Fri, Jul 1, 2022 at 7:14 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > I noticed this during beta1, but dismissed the issue when it wasn't easily
> > reproducible.  Now, I saw the same problem while upgrading from beta1 to beta2,
> > so couldn't dismiss it.  It turns out that LOs are lost if VACUUM FULL was run.
> 
> Yikes. That's really bad, and I have no idea what might be causing it,
> either. I'll plan to investigate this on Tuesday unless someone gets
> to it before then.

I suppose it's like Bruce said, here.

https://www.postgresql.org/message-id/20210601140949.GC22012%40momjian.us

|One tricky case is pg_largeobject, which is copied from the old to new
|cluster since it has user data.  To preserve that relfilenode, you would
|need to have pg_upgrade perform cluster surgery in each database to
|renumber its relfilenode to match since it is created by initdb.  I
|can't think of a case where pg_upgrade already does something like that.

Rather than setting the filenode of the next object as for user tables,
pg-upgrade needs to UPDATE the relfilenode.

This patch "works for me" but feel free to improve on it.

--ieNMXl1Fr3cevapt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-wip-preserve-relfilenode-of-pg_largeobject-and-its-i.patch"

From 88cbe118d4eb4dcf9b6d2831d81f723587d80942 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 2 Jul 2022 00:48:47 -0500
Subject: [PATCH] wip: preserve relfilenode of pg_largeobject and its indexes

An empty table is created by initdb, but its filenode was not preserved, so
pg_largeobject was empty after upgrades.

See also:
9a974cbcba005256a19991203583a94b4f9a21a9
https://www.postgresql.org/message-id/20210601140949.GC22012%40momjian.us
https://www.postgresql.org/message-id/20220701231413.GI13040%40telsasoft.com
---
 src/bin/pg_dump/pg_dump.c              | 37 +++++++++++++++-----------
 src/bin/pg_upgrade/t/002_pg_upgrade.pl |  2 ++
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d9c5bcafd20..c629f4154fc 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3135,7 +3135,7 @@ dumpDatabase(Archive *fout)
 
 	/*
 	 * pg_largeobject comes from the old system intact, so set its
-	 * relfrozenxids and relminmxids.
+	 * relfrozenxids, relminmxids and relfilenode.
 	 */
 	if (dopt->binary_upgrade)
 	{
@@ -3143,34 +3143,41 @@ dumpDatabase(Archive *fout)
 		PQExpBuffer loFrozenQry = createPQExpBuffer();
 		PQExpBuffer loOutQry = createPQExpBuffer();
 		int			i_relfrozenxid,
+					i_relfilenode,
+					i_oid,
 					i_relminmxid;
 
 		/*
 		 * pg_largeobject
 		 */
 		if (fout->remoteVersion >= 90300)
-			appendPQExpBuffer(loFrozenQry, "SELECT relfrozenxid, relminmxid\n"
+			appendPQExpBuffer(loFrozenQry, "SELECT relfrozenxid, relminmxid, relfilenode, oid\n"
 							  "FROM pg_catalog.pg_class\n"
-							  "WHERE oid = %u;\n",
-							  LargeObjectRelationId);
+							  "WHERE oid IN (%u, %u);\n",
+							  LargeObjectRelationId, LargeObjectLOidPNIndexId);
 		else
-			appendPQExpBuffer(loFrozenQry, "SELECT relfrozenxid, 0 AS relminmxid\n"
+			appendPQExpBuffer(loFrozenQry, "SELECT relfrozenxid, 0 AS relminmxid, relfilenode, oid\n"
 							  "FROM pg_catalog.pg_class\n"
-							  "WHERE oid = %u;\n",
-							  LargeObjectRelationId);
+							  "WHERE oid IN (%u, %u);\n",
+							  LargeObjectRelationId, LargeObjectLOidPNIndexId);
 
-		lo_res = ExecuteSqlQueryForSingleRow(fout, loFrozenQry->data);
+		lo_res = ExecuteSqlQuery(fout, loFrozenQry->data, PGRES_TUPLES_OK);
 
 		i_relfrozenxid = PQfnumber(lo_res, "relfrozenxid");
 		i_relminmxid = PQfnumber(lo_res, "relminmxid");
+		i_relfilenode = PQfnumber(lo_res, "relfilenode");
+		i_oid = PQfnumber(lo_res, "oid");
+
+		appendPQExpBufferStr(loOutQry, "\n-- For binary upgrade, set pg_largeobject relfrozenxid, relminmxid and relfilenode\n");
+		for (int i = 0; i < PQntuples(lo_res); ++i)
+			appendPQExpBuffer(loOutQry, "UPDATE pg_catalog.pg_class\n"
+							  "SET relfrozenxid = '%u', relminmxid = '%u', relfilenode = '%u'\n"
+							  "WHERE oid = %u;\n",
+							  atooid(PQgetvalue(lo_res, i, i_relfrozenxid)),
+							  atooid(PQgetvalue(lo_res, i, i_relminmxid)),
+							  atooid(PQgetvalue(lo_res, i, i_relfilenode)),
+							  atooid(PQgetvalue(lo_res, i, i_oid)));
 
-		appendPQExpBufferStr(loOutQry, "\n-- For binary upgrade, set pg_largeobject relfrozenxid and relminmxid\n");
-		appendPQExpBuffer(loOutQry, "UPDATE pg_catalog.pg_class\n"
-						  "SET relfrozenxid = '%u', relminmxid = '%u'\n"
-						  "WHERE oid = %u;\n",
-						  atooid(PQgetvalue(lo_res, 0, i_relfrozenxid)),
-						  atooid(PQgetvalue(lo_res, 0, i_relminmxid)),
-						  LargeObjectRelationId);
 		ArchiveEntry(fout, nilCatalogId, createDumpId(),
 					 ARCHIVE_OPTS(.tag = "pg_largeobject",
 								  .description = "pg_largeobject",
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 67e0be68568..a22f0f8c885 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -208,6 +208,8 @@ if (defined($ENV{oldinstall}))
 	}
 }
 
+$oldnode->safe_psql("regression", "VACUUM FULL pg_largeobject;");
+
 # In a VPATH build, we'll be started in the source directory, but we want
 # to run pg_upgrade in the build directory so that any files generated finish
 # in it, like delete_old_cluster.{sh,bat}.
-- 
2.17.1


--ieNMXl1Fr3cevapt--