public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 1/5] bootstrap: convert Typ to a List*
6+ messages / 3 participants
[nested] [flat]

* [PATCH 1/5] bootstrap: convert Typ to a List*
@ 2020-11-20 02:48 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Justin Pryzby @ 2020-11-20 02:48 UTC (permalink / raw)

---
 src/backend/bootstrap/bootstrap.c | 69 ++++++++++++++-----------------
 1 file changed, 31 insertions(+), 38 deletions(-)

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 6f615e6622..18eb62ca47 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -159,7 +159,7 @@ struct typmap
 	FormData_pg_type am_typ;
 };
 
-static struct typmap **Typ = NULL;
+static List *Typ = NIL; /* List of struct typmap* */
 static struct typmap *Ap = NULL;
 
 static Datum values[MAXATTR];	/* current row's attribute values */
@@ -597,7 +597,7 @@ boot_openrel(char *relname)
 	 * pg_type must be filled before any OPEN command is executed, hence we
 	 * can now populate the Typ array if we haven't yet.
 	 */
-	if (Typ == NULL)
+	if (Typ == NIL)
 		populate_typ_array();
 
 	if (boot_reldesc != NULL)
@@ -688,7 +688,7 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
 
 	typeoid = gettype(type);
 
-	if (Typ != NULL)
+	if (Typ != NIL)
 	{
 		attrtypes[attnum]->atttypid = Ap->am_oid;
 		attrtypes[attnum]->attlen = Ap->am_typ.typlen;
@@ -877,36 +877,25 @@ populate_typ_array(void)
 	Relation	rel;
 	TableScanDesc scan;
 	HeapTuple	tup;
-	int			nalloc;
-	int			i;
-
-	Assert(Typ == NULL);
 
-	nalloc = 512;
-	Typ = (struct typmap **)
-		MemoryContextAlloc(TopMemoryContext, nalloc * sizeof(struct typmap *));
+	Assert(Typ == NIL);
 
 	rel = table_open(TypeRelationId, NoLock);
 	scan = table_beginscan_catalog(rel, 0, NULL);
-	i = 0;
 	while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
 	{
 		Form_pg_type typForm = (Form_pg_type) GETSTRUCT(tup);
+		struct typmap *newtyp;
+		MemoryContext old;
 
-		/* make sure there will be room for a trailing NULL pointer */
-		if (i >= nalloc - 1)
-		{
-			nalloc *= 2;
-			Typ = (struct typmap **)
-				repalloc(Typ, nalloc * sizeof(struct typmap *));
-		}
-		Typ[i] = (struct typmap *)
-			MemoryContextAlloc(TopMemoryContext, sizeof(struct typmap));
-		Typ[i]->am_oid = typForm->oid;
-		memcpy(&(Typ[i]->am_typ), typForm, sizeof(Typ[i]->am_typ));
-		i++;
+		old = MemoryContextSwitchTo(TopMemoryContext);
+		newtyp = (struct typmap *) palloc(sizeof(struct typmap));
+		Typ = lappend(Typ, newtyp);
+		MemoryContextSwitchTo(old);
+
+		newtyp->am_oid = typForm->oid;
+		memcpy(&newtyp->am_typ, typForm, sizeof(newtyp->am_typ));
 	}
-	Typ[i] = NULL;				/* Fill trailing NULL pointer */
 	table_endscan(scan);
 	table_close(rel, NoLock);
 }
@@ -925,16 +914,17 @@ populate_typ_array(void)
 static Oid
 gettype(char *type)
 {
-	if (Typ != NULL)
+	if (Typ != NIL)
 	{
-		struct typmap **app;
+		ListCell *lc;
 
-		for (app = Typ; *app != NULL; app++)
+		foreach (lc, Typ)
 		{
-			if (strncmp(NameStr((*app)->am_typ.typname), type, NAMEDATALEN) == 0)
+			struct typmap *app = lfirst(lc);
+			if (strncmp(NameStr(app->am_typ.typname), type, NAMEDATALEN) == 0)
 			{
-				Ap = *app;
-				return (*app)->am_oid;
+				Ap = app;
+				return app->am_oid;
 			}
 		}
 	}
@@ -980,14 +970,17 @@ boot_get_type_io_data(Oid typid,
 	if (Typ != NULL)
 	{
 		/* We have the boot-time contents of pg_type, so use it */
-		struct typmap **app;
-		struct typmap *ap;
-
-		app = Typ;
-		while (*app && (*app)->am_oid != typid)
-			++app;
-		ap = *app;
-		if (ap == NULL)
+		struct typmap *ap = NULL;
+		ListCell *lc;
+
+		foreach (lc, Typ)
+		{
+			ap = lfirst(lc);
+			if (ap->am_oid == typid)
+				break;
+		}
+
+		if (!ap || ap->am_oid != typid)
 			elog(ERROR, "type OID %u not found in Typ list", typid);
 
 		*typlen = ap->am_typ.typlen;
-- 
2.26.2


--------------614DDB87AFFED893713AC0E9
Content-Type: text/x-patch; charset=UTF-8;
 name="0002-Allow-composite-types-in-bootstrap-20210304.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="0002-Allow-composite-types-in-bootstrap-20210304.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible
@ 2026-02-04 16:06 Nathan Bossart <[email protected]>
  2026-02-04 20:08 ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Nathan Bossart @ 2026-02-04 16:06 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Nitin Motiani <[email protected]>; Hannu Krosing <[email protected]>; PostgreSQL Hackers <[email protected]>

On Tue, Feb 03, 2026 at 06:46:25PM -0500, Andres Freund wrote:
>> The reason, I think, is that the COPY is happening into a relfilenode that
>> will be overwritten later, it doesn't yet contain the contents of the old
>> cluster.
>> 
>> Presumably we do this because we need the temporary pg_largeobject_metadata to
>> make COMMENT ON and security label commands not fail.
>> 
>> If this is the reasoning / how it works, shouldn't there be a comment in the
>> code or the commit message explaining that? Because it sure seems non-obvious
>> to me.

Right, the COPY for LOs with comments and security labels is solely meant
to avoid failure when restoring the comments and security labels, since we
won't have transferred the relation files yet.  This was the case before
commit 12a53c732c, where we had this comment in getBlobs():

        * We *do* dump out the definition of the blob because we need that to
        * make the restoration of the comments, and anything else, work since
        * pg_upgrade copies the files behind pg_largeobject and
        * pg_largeobject_metadata after the dump is restored.

Commit 3bcfcd815e (mine) added this one to pg_dump.c:

        * If upgrading from v16 or newer, only dump large objects with
        * comments/seclabels.  For these upgrades, pg_upgrade can copy/link
        * pg_largeobject_metadata's files (which is usually faster) but we
        * still need to dump LOs with comments/seclabels here so that the
        * subsequent COMMENT and SECURITY LABEL commands work.  pg_upgrade
        * can't copy/link the files from older versions because aclitem
        * (needed by pg_largeobject_metadata.lomacl) changed its storage
        * format in v16.

IIUC your critique is that this doesn't explain the overwriting behavior
like the older comment does.  I'll work on adding that.

>> It's also not entirely obvious to me that this is safe - after all
>> (bbe08b8869bd, revised in 0e758ae89) appeared to have taken some pains to
>> ensure that the file gets unlinked immediately during the "binary upgrade
>> mode" TRUNCATE. But now we are actually filling that file again, after the
>> relation had been truncated?
> 
> An example of what could go wrong:
>
> [... examples of what could go wrong ...] 

I'm considering a couple of options here, but it seems like the easiest
thing to do is to move the TRUNCATE commands to the end of the dump file.
At least, that seems to be sufficient for our existing tests.  If that
seems okay to you, I can work on putting together a patch.

-- 
nathan






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible
  2026-02-04 16:06 Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
@ 2026-02-04 20:08 ` Nathan Bossart <[email protected]>
  2026-02-05 16:19   ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Andres Freund <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Nathan Bossart @ 2026-02-04 20:08 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Nitin Motiani <[email protected]>; Hannu Krosing <[email protected]>; PostgreSQL Hackers <[email protected]>

On Wed, Feb 04, 2026 at 10:06:29AM -0600, Nathan Bossart wrote:
> IIUC your critique is that this doesn't explain the overwriting behavior
> like the older comment does.  I'll work on adding that.
> 
> [...]
> 
> I'm considering a couple of options here, but it seems like the easiest
> thing to do is to move the TRUNCATE commands to the end of the dump file.
> At least, that seems to be sufficient for our existing tests.  If that
> seems okay to you, I can work on putting together a patch.

Here is a rough first draft of a patch that does this.

-- 
nathan

From c911f5b16132514a461f72b550ee8cd86dfadc1d Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 4 Feb 2026 14:04:18 -0600
Subject: [PATCH v1 1/1] fix pg_largeobject_metadata file transfer

---
 src/bin/pg_dump/pg_dump.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2bebefd0ba2..3ba57492fa6 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -379,6 +379,7 @@ static const char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
 static void getLOs(Archive *fout);
 static void dumpLO(Archive *fout, const LoInfo *loinfo);
 static int	dumpLOs(Archive *fout, const void *arg);
+static void dumpLOTruncation(Archive *fout);
 static void dumpPolicy(Archive *fout, const PolicyInfo *polinfo);
 static void dumpPublication(Archive *fout, const PublicationInfo *pubinfo);
 static void dumpPublicationTable(Archive *fout, const PublicationRelInfo *pubrinfo);
@@ -1157,7 +1158,10 @@ main(int argc, char **argv)
 		 * subsequent COMMENT and SECURITY LABEL commands work.  pg_upgrade
 		 * can't copy/link the files from older versions because aclitem
 		 * (needed by pg_largeobject_metadata.lomacl) changed its storage
-		 * format in v16.
+		 * format in v16.  At the end of the dump, we'll generate a TRUNCATE
+		 * command for pg_largeobject_metadata so that it's contents are
+		 * cleared in preparation for the subsequent file transfer by
+		 * pg_upgrade.
 		 */
 		if (fout->remoteVersion >= 160000)
 			lo_metadata->dataObj->filtercond = "WHERE oid IN "
@@ -1243,6 +1247,13 @@ main(int argc, char **argv)
 	for (i = 0; i < numObjs; i++)
 		dumpDumpableObject(fout, dobjs[i]);
 
+	/*
+	 * For binary upgrades, set relfrozenxids, relminmxids, and relfilenodes
+	 * of pg_largeobject and maybe pg_largeobject_metadata, and remove all
+	 * their files.  We will transfer them from the old cluster as needed.
+	 */
+	dumpLOTruncation(fout);
+
 	/*
 	 * Set up options info to ensure we dump what we want.
 	 */
@@ -3671,6 +3682,20 @@ dumpDatabase(Archive *fout)
 								  .dropStmt = delQry->data,
 								  .deps = &dbDumpId));
 
+	PQclear(res);
+
+	free(qdatname);
+	destroyPQExpBuffer(dbQry);
+	destroyPQExpBuffer(delQry);
+	destroyPQExpBuffer(creaQry);
+	destroyPQExpBuffer(labelq);
+}
+
+static void
+dumpLOTruncation(Archive *fout)
+{
+	DumpOptions *dopt = fout->dopt;
+
 	/*
 	 * pg_largeobject comes from the old system intact, so set its
 	 * relfrozenxids, relminmxids and relfilenode.
@@ -3769,14 +3794,14 @@ dumpDatabase(Archive *fout)
 		ArchiveEntry(fout, nilCatalogId, createDumpId(),
 					 ARCHIVE_OPTS(.tag = "pg_largeobject",
 								  .description = "pg_largeobject",
-								  .section = SECTION_PRE_DATA,
+								  .section = SECTION_POST_DATA,
 								  .createStmt = loOutQry->data));
 
 		if (fout->remoteVersion >= 160000)
 			ArchiveEntry(fout, nilCatalogId, createDumpId(),
 						 ARCHIVE_OPTS(.tag = "pg_largeobject_metadata",
 									  .description = "pg_largeobject_metadata",
-									  .section = SECTION_PRE_DATA,
+									  .section = SECTION_POST_DATA,
 									  .createStmt = lomOutQry->data));
 
 		PQclear(lo_res);
@@ -3787,14 +3812,6 @@ dumpDatabase(Archive *fout)
 		destroyPQExpBuffer(loOutQry);
 		destroyPQExpBuffer(lomOutQry);
 	}
-
-	PQclear(res);
-
-	free(qdatname);
-	destroyPQExpBuffer(dbQry);
-	destroyPQExpBuffer(delQry);
-	destroyPQExpBuffer(creaQry);
-	destroyPQExpBuffer(labelq);
 }
 
 /*
-- 
2.50.1 (Apple Git-155)



Attachments:

  [text/plain] v1-0001-fix-pg_largeobject_metadata-file-transfer.patch (3.5K, ../../aYOnT5EqAqI3ooBO@nathan/2-v1-0001-fix-pg_largeobject_metadata-file-transfer.patch)
  download | inline diff:
From c911f5b16132514a461f72b550ee8cd86dfadc1d Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 4 Feb 2026 14:04:18 -0600
Subject: [PATCH v1 1/1] fix pg_largeobject_metadata file transfer

---
 src/bin/pg_dump/pg_dump.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2bebefd0ba2..3ba57492fa6 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -379,6 +379,7 @@ static const char *getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
 static void getLOs(Archive *fout);
 static void dumpLO(Archive *fout, const LoInfo *loinfo);
 static int	dumpLOs(Archive *fout, const void *arg);
+static void dumpLOTruncation(Archive *fout);
 static void dumpPolicy(Archive *fout, const PolicyInfo *polinfo);
 static void dumpPublication(Archive *fout, const PublicationInfo *pubinfo);
 static void dumpPublicationTable(Archive *fout, const PublicationRelInfo *pubrinfo);
@@ -1157,7 +1158,10 @@ main(int argc, char **argv)
 		 * subsequent COMMENT and SECURITY LABEL commands work.  pg_upgrade
 		 * can't copy/link the files from older versions because aclitem
 		 * (needed by pg_largeobject_metadata.lomacl) changed its storage
-		 * format in v16.
+		 * format in v16.  At the end of the dump, we'll generate a TRUNCATE
+		 * command for pg_largeobject_metadata so that it's contents are
+		 * cleared in preparation for the subsequent file transfer by
+		 * pg_upgrade.
 		 */
 		if (fout->remoteVersion >= 160000)
 			lo_metadata->dataObj->filtercond = "WHERE oid IN "
@@ -1243,6 +1247,13 @@ main(int argc, char **argv)
 	for (i = 0; i < numObjs; i++)
 		dumpDumpableObject(fout, dobjs[i]);
 
+	/*
+	 * For binary upgrades, set relfrozenxids, relminmxids, and relfilenodes
+	 * of pg_largeobject and maybe pg_largeobject_metadata, and remove all
+	 * their files.  We will transfer them from the old cluster as needed.
+	 */
+	dumpLOTruncation(fout);
+
 	/*
 	 * Set up options info to ensure we dump what we want.
 	 */
@@ -3671,6 +3682,20 @@ dumpDatabase(Archive *fout)
 								  .dropStmt = delQry->data,
 								  .deps = &dbDumpId));
 
+	PQclear(res);
+
+	free(qdatname);
+	destroyPQExpBuffer(dbQry);
+	destroyPQExpBuffer(delQry);
+	destroyPQExpBuffer(creaQry);
+	destroyPQExpBuffer(labelq);
+}
+
+static void
+dumpLOTruncation(Archive *fout)
+{
+	DumpOptions *dopt = fout->dopt;
+
 	/*
 	 * pg_largeobject comes from the old system intact, so set its
 	 * relfrozenxids, relminmxids and relfilenode.
@@ -3769,14 +3794,14 @@ dumpDatabase(Archive *fout)
 		ArchiveEntry(fout, nilCatalogId, createDumpId(),
 					 ARCHIVE_OPTS(.tag = "pg_largeobject",
 								  .description = "pg_largeobject",
-								  .section = SECTION_PRE_DATA,
+								  .section = SECTION_POST_DATA,
 								  .createStmt = loOutQry->data));
 
 		if (fout->remoteVersion >= 160000)
 			ArchiveEntry(fout, nilCatalogId, createDumpId(),
 						 ARCHIVE_OPTS(.tag = "pg_largeobject_metadata",
 									  .description = "pg_largeobject_metadata",
-									  .section = SECTION_PRE_DATA,
+									  .section = SECTION_POST_DATA,
 									  .createStmt = lomOutQry->data));
 
 		PQclear(lo_res);
@@ -3787,14 +3812,6 @@ dumpDatabase(Archive *fout)
 		destroyPQExpBuffer(loOutQry);
 		destroyPQExpBuffer(lomOutQry);
 	}
-
-	PQclear(res);
-
-	free(qdatname);
-	destroyPQExpBuffer(dbQry);
-	destroyPQExpBuffer(delQry);
-	destroyPQExpBuffer(creaQry);
-	destroyPQExpBuffer(labelq);
 }
 
 /*
-- 
2.50.1 (Apple Git-155)



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible
  2026-02-04 16:06 Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
  2026-02-04 20:08 ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
@ 2026-02-05 16:19   ` Andres Freund <[email protected]>
  2026-02-05 17:36     ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Andres Freund @ 2026-02-05 16:19 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Nitin Motiani <[email protected]>; Hannu Krosing <[email protected]>; PostgreSQL Hackers <[email protected]>

Hi,

On 2026-02-04 10:06:29 -0600, Nathan Bossart wrote:
> IIUC your critique is that this doesn't explain the overwriting behavior
> like the older comment does.  I'll work on adding that.

I think even the old comment was woefully under-documenting that the commands
are all just make work that's going to be thrown out almost immediately after.

Thanks for addressing that!


On 2026-02-04 14:08:47 -0600, Nathan Bossart wrote:
> On Wed, Feb 04, 2026 at 10:06:29AM -0600, Nathan Bossart wrote:
> > IIUC your critique is that this doesn't explain the overwriting behavior
> > like the older comment does.  I'll work on adding that.
> > 
> > [...]
> > 
> > I'm considering a couple of options here, but it seems like the easiest
> > thing to do is to move the TRUNCATE commands to the end of the dump file.
> > At least, that seems to be sufficient for our existing tests.  If that
> > seems okay to you, I can work on putting together a patch.
> 
> Here is a rough first draft of a patch that does this.

It certainly seems better than what we do now.  Still feels pretty grotty and
error prone to me that we fill the catalog table and then throw the contents
out.


> @@ -1157,7 +1158,10 @@ main(int argc, char **argv)
>  		 * subsequent COMMENT and SECURITY LABEL commands work.  pg_upgrade
>  		 * can't copy/link the files from older versions because aclitem
>  		 * (needed by pg_largeobject_metadata.lomacl) changed its storage
> -		 * format in v16.
> +		 * format in v16.  At the end of the dump, we'll generate a TRUNCATE
> +		 * command for pg_largeobject_metadata so that it's contents are
> +		 * cleared in preparation for the subsequent file transfer by
> +		 * pg_upgrade.
>  		 */

I'd move that comment to earlier in the paragraph, it sounds a bit like it
applies to the v16 specific bits.


>  		if (fout->remoteVersion >= 160000)
>  			lo_metadata->dataObj->filtercond = "WHERE oid IN "
> @@ -1243,6 +1247,13 @@ main(int argc, char **argv)
>  	for (i = 0; i < numObjs; i++)
>  		dumpDumpableObject(fout, dobjs[i]);
>  
> +	/*
> +	 * For binary upgrades, set relfrozenxids, relminmxids, and relfilenodes
> +	 * of pg_largeobject and maybe pg_largeobject_metadata, and remove all
> +	 * their files.  We will transfer them from the old cluster as needed.
> +	 */
> +	dumpLOTruncation(fout);
> +
>  	/*
>  	 * Set up options info to ensure we dump what we want.
>  	 */

Seems good to move this to a dedicated function, regardless of anything else.


Do I see correctly that we just rely on the ordering in the file, rather than
dependencies? That's not a complaint, I just don't know that code very well.

Greetings,

Andres Freund






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible
  2026-02-04 16:06 Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
  2026-02-04 20:08 ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
  2026-02-05 16:19   ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Andres Freund <[email protected]>
@ 2026-02-05 17:36     ` Nathan Bossart <[email protected]>
  2026-02-05 17:40       ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Nathan Bossart @ 2026-02-05 17:36 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Nitin Motiani <[email protected]>; Hannu Krosing <[email protected]>; PostgreSQL Hackers <[email protected]>

On Thu, Feb 05, 2026 at 11:19:46AM -0500, Andres Freund wrote:
> It certainly seems better than what we do now.  Still feels pretty grotty and
> error prone to me that we fill the catalog table and then throw the contents
> out.

Before I go any further with this approach, I thought of something else we
could do that I believe is worth considering...

As of commit 3bcfcd815e, the only reason we are dumping any of
pg_largeobject_metadata at all is to avoid an ERROR during COMMENT ON or
SECURITY LABEL ON because the call to LargeObjectExists() in
get_object_address() returns false.  If we bypass that check in
binary-upgrade mode, we can skip dumping pg_largeobject_metadata entirely.

The attached patch passes our existing tests, and it seems to create the
expected binary-upgrade-mode dump files, too.  I haven't updated any of the
comments yet.

-- 
nathan

From 1fa1890f6bf98b718118f070226d8b102073e165 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 5 Feb 2026 11:26:39 -0600
Subject: [PATCH v2 1/1] fix pg_largeobject_metadata file transfer

---
 src/backend/catalog/objectaddress.c |  2 +-
 src/bin/pg_dump/pg_dump.c           | 53 +++++++++++++----------------
 2 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 02af64b82c6..1762146c09a 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -1046,7 +1046,7 @@ get_object_address(ObjectType objtype, Node *object,
 				address.classId = LargeObjectRelationId;
 				address.objectId = oidparse(object);
 				address.objectSubId = 0;
-				if (!LargeObjectExists(address.objectId))
+				if (!LargeObjectExists(address.objectId) && !IsBinaryUpgrade)
 				{
 					if (!missing_ok)
 						ereport(ERROR,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2bebefd0ba2..6bcb2f61fe8 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1129,19 +1129,11 @@ main(int argc, char **argv)
 	 */
 	if (dopt.binary_upgrade && fout->remoteVersion >= 120000)
 	{
-		TableInfo  *lo_metadata = findTableByOid(LargeObjectMetadataRelationId);
-		TableInfo  *shdepend = findTableByOid(SharedDependRelationId);
+		TableInfo  *shdepend;
 
-		makeTableDataInfo(&dopt, lo_metadata);
+		shdepend = findTableByOid(SharedDependRelationId);
 		makeTableDataInfo(&dopt, shdepend);
 
-		/*
-		 * Save pg_largeobject_metadata's dump ID for use as a dependency for
-		 * pg_shdepend and any large object comments/seclabels.
-		 */
-		lo_metadata_dumpId = lo_metadata->dataObj->dobj.dumpId;
-		addObjectDependency(&shdepend->dataObj->dobj, lo_metadata_dumpId);
-
 		/*
 		 * Only dump large object shdepend rows for this database.
 		 */
@@ -1149,22 +1141,20 @@ main(int argc, char **argv)
 			"AND dbid = (SELECT oid FROM pg_database "
 			"            WHERE datname = current_database())";
 
-		/*
-		 * If upgrading from v16 or newer, only dump large objects with
-		 * comments/seclabels.  For these upgrades, pg_upgrade can copy/link
-		 * pg_largeobject_metadata's files (which is usually faster) but we
-		 * still need to dump LOs with comments/seclabels here so that the
-		 * subsequent COMMENT and SECURITY LABEL commands work.  pg_upgrade
-		 * can't copy/link the files from older versions because aclitem
-		 * (needed by pg_largeobject_metadata.lomacl) changed its storage
-		 * format in v16.
-		 */
-		if (fout->remoteVersion >= 160000)
-			lo_metadata->dataObj->filtercond = "WHERE oid IN "
-				"(SELECT objoid FROM pg_description "
-				"WHERE classoid = " CppAsString2(LargeObjectRelationId) " "
-				"UNION SELECT objoid FROM pg_seclabel "
-				"WHERE classoid = " CppAsString2(LargeObjectRelationId) ")";
+		if (fout->remoteVersion < 160000)
+		{
+			TableInfo  *lo_metadata;
+
+			lo_metadata = findTableByOid(LargeObjectMetadataRelationId);
+			makeTableDataInfo(&dopt, lo_metadata);
+
+			/*
+			 * Save pg_largeobject_metadata's dump ID for use as a dependency
+			 * for pg_shdepend and any large object comments/seclabels.
+			 */
+			lo_metadata_dumpId = lo_metadata->dataObj->dobj.dumpId;
+			addObjectDependency(&shdepend->dataObj->dobj, lo_metadata_dumpId);
+		}
 	}
 
 	/*
@@ -4079,7 +4069,7 @@ getLOs(Archive *fout)
 				 * We should've saved pg_largeobject_metadata's dump ID before
 				 * this point.
 				 */
-				Assert(lo_metadata_dumpId);
+				Assert(lo_metadata_dumpId || fout->remoteVersion >= 160000);
 
 				loinfo->dobj.dump &= ~(DUMP_COMPONENT_DATA | DUMP_COMPONENT_ACL | DUMP_COMPONENT_DEFINITION);
 
@@ -4088,9 +4078,12 @@ getLOs(Archive *fout)
 				 * pg_largeobject_metadata so that any large object
 				 * comments/seclables are dumped after it.
 				 */
-				loinfo->dobj.dependencies = (DumpId *) pg_malloc(sizeof(DumpId));
-				loinfo->dobj.dependencies[0] = lo_metadata_dumpId;
-				loinfo->dobj.nDeps = loinfo->dobj.allocDeps = 1;
+				if (fout->remoteVersion < 160000)
+				{
+					loinfo->dobj.dependencies = (DumpId *) pg_malloc(sizeof(DumpId));
+					loinfo->dobj.dependencies[0] = lo_metadata_dumpId;
+					loinfo->dobj.nDeps = loinfo->dobj.allocDeps = 1;
+				}
 			}
 			else
 				loinfo->dobj.dump &= ~DUMP_COMPONENT_DATA;
-- 
2.50.1 (Apple Git-155)



Attachments:

  [text/plain] v2-0001-fix-pg_largeobject_metadata-file-transfer.patch (4.3K, ../../aYTVAFTPelebVl6W@nathan/2-v2-0001-fix-pg_largeobject_metadata-file-transfer.patch)
  download | inline diff:
From 1fa1890f6bf98b718118f070226d8b102073e165 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 5 Feb 2026 11:26:39 -0600
Subject: [PATCH v2 1/1] fix pg_largeobject_metadata file transfer

---
 src/backend/catalog/objectaddress.c |  2 +-
 src/bin/pg_dump/pg_dump.c           | 53 +++++++++++++----------------
 2 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 02af64b82c6..1762146c09a 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -1046,7 +1046,7 @@ get_object_address(ObjectType objtype, Node *object,
 				address.classId = LargeObjectRelationId;
 				address.objectId = oidparse(object);
 				address.objectSubId = 0;
-				if (!LargeObjectExists(address.objectId))
+				if (!LargeObjectExists(address.objectId) && !IsBinaryUpgrade)
 				{
 					if (!missing_ok)
 						ereport(ERROR,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 2bebefd0ba2..6bcb2f61fe8 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1129,19 +1129,11 @@ main(int argc, char **argv)
 	 */
 	if (dopt.binary_upgrade && fout->remoteVersion >= 120000)
 	{
-		TableInfo  *lo_metadata = findTableByOid(LargeObjectMetadataRelationId);
-		TableInfo  *shdepend = findTableByOid(SharedDependRelationId);
+		TableInfo  *shdepend;
 
-		makeTableDataInfo(&dopt, lo_metadata);
+		shdepend = findTableByOid(SharedDependRelationId);
 		makeTableDataInfo(&dopt, shdepend);
 
-		/*
-		 * Save pg_largeobject_metadata's dump ID for use as a dependency for
-		 * pg_shdepend and any large object comments/seclabels.
-		 */
-		lo_metadata_dumpId = lo_metadata->dataObj->dobj.dumpId;
-		addObjectDependency(&shdepend->dataObj->dobj, lo_metadata_dumpId);
-
 		/*
 		 * Only dump large object shdepend rows for this database.
 		 */
@@ -1149,22 +1141,20 @@ main(int argc, char **argv)
 			"AND dbid = (SELECT oid FROM pg_database "
 			"            WHERE datname = current_database())";
 
-		/*
-		 * If upgrading from v16 or newer, only dump large objects with
-		 * comments/seclabels.  For these upgrades, pg_upgrade can copy/link
-		 * pg_largeobject_metadata's files (which is usually faster) but we
-		 * still need to dump LOs with comments/seclabels here so that the
-		 * subsequent COMMENT and SECURITY LABEL commands work.  pg_upgrade
-		 * can't copy/link the files from older versions because aclitem
-		 * (needed by pg_largeobject_metadata.lomacl) changed its storage
-		 * format in v16.
-		 */
-		if (fout->remoteVersion >= 160000)
-			lo_metadata->dataObj->filtercond = "WHERE oid IN "
-				"(SELECT objoid FROM pg_description "
-				"WHERE classoid = " CppAsString2(LargeObjectRelationId) " "
-				"UNION SELECT objoid FROM pg_seclabel "
-				"WHERE classoid = " CppAsString2(LargeObjectRelationId) ")";
+		if (fout->remoteVersion < 160000)
+		{
+			TableInfo  *lo_metadata;
+
+			lo_metadata = findTableByOid(LargeObjectMetadataRelationId);
+			makeTableDataInfo(&dopt, lo_metadata);
+
+			/*
+			 * Save pg_largeobject_metadata's dump ID for use as a dependency
+			 * for pg_shdepend and any large object comments/seclabels.
+			 */
+			lo_metadata_dumpId = lo_metadata->dataObj->dobj.dumpId;
+			addObjectDependency(&shdepend->dataObj->dobj, lo_metadata_dumpId);
+		}
 	}
 
 	/*
@@ -4079,7 +4069,7 @@ getLOs(Archive *fout)
 				 * We should've saved pg_largeobject_metadata's dump ID before
 				 * this point.
 				 */
-				Assert(lo_metadata_dumpId);
+				Assert(lo_metadata_dumpId || fout->remoteVersion >= 160000);
 
 				loinfo->dobj.dump &= ~(DUMP_COMPONENT_DATA | DUMP_COMPONENT_ACL | DUMP_COMPONENT_DEFINITION);
 
@@ -4088,9 +4078,12 @@ getLOs(Archive *fout)
 				 * pg_largeobject_metadata so that any large object
 				 * comments/seclables are dumped after it.
 				 */
-				loinfo->dobj.dependencies = (DumpId *) pg_malloc(sizeof(DumpId));
-				loinfo->dobj.dependencies[0] = lo_metadata_dumpId;
-				loinfo->dobj.nDeps = loinfo->dobj.allocDeps = 1;
+				if (fout->remoteVersion < 160000)
+				{
+					loinfo->dobj.dependencies = (DumpId *) pg_malloc(sizeof(DumpId));
+					loinfo->dobj.dependencies[0] = lo_metadata_dumpId;
+					loinfo->dobj.nDeps = loinfo->dobj.allocDeps = 1;
+				}
 			}
 			else
 				loinfo->dobj.dump &= ~DUMP_COMPONENT_DATA;
-- 
2.50.1 (Apple Git-155)



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible
  2026-02-04 16:06 Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
  2026-02-04 20:08 ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
  2026-02-05 16:19   ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Andres Freund <[email protected]>
  2026-02-05 17:36     ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
@ 2026-02-05 17:40       ` Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Nathan Bossart @ 2026-02-05 17:40 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Nitin Motiani <[email protected]>; Hannu Krosing <[email protected]>; PostgreSQL Hackers <[email protected]>

On Thu, Feb 05, 2026 at 11:36:00AM -0600, Nathan Bossart wrote:
> @@ -1046,7 +1046,7 @@ get_object_address(ObjectType objtype, Node *object,
>  				address.classId = LargeObjectRelationId;
>  				address.objectId = oidparse(object);
>  				address.objectSubId = 0;
> -				if (!LargeObjectExists(address.objectId))
> +				if (!LargeObjectExists(address.objectId) && !IsBinaryUpgrade)
>  				{
>  					if (!missing_ok)
>  						ereport(ERROR,

(Probably better to set missing_ok for only the specific commands we want
to bypass this check, but you get the idea...)

-- 
nathan






^ permalink  raw  reply  [nested|flat] 6+ messages in thread


end of thread, other threads:[~2026-02-05 17:40 UTC | newest]

Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 02:48 [PATCH 1/5] bootstrap: convert Typ to a List* Justin Pryzby <[email protected]>
2026-02-04 16:06 Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
2026-02-04 20:08 ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
2026-02-05 16:19   ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Andres Freund <[email protected]>
2026-02-05 17:36     ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>
2026-02-05 17:40       ` Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible Nathan Bossart <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox