pg_dump, ATTACH, and independently restorable child partitions

Started by Justin Pryzbyabout 5 years ago11 messages
#1Justin Pryzby
pryzby@telsasoft.com

Since this commit, pg_dump CREATEs tables and then ATTACHes them:

|commit 33a53130a89447e171a8268ae0b221bb48af6468
|Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
|Date: Mon Jun 10 18:56:23 2019 -0400
|
| Make pg_dump emit ATTACH PARTITION instead of PARTITION OF (reprise)
|...
| This change also has the advantage that the partition is restorable from
| the dump (as a standalone table) even if its parent table isn't
| restored.

I like the idea of child tables being independently restorable, but it doesn't
seem to work.

|psql postgres -c 'DROP TABLE IF EXISTS t' -c 'CREATE TABLE t(i int) PARTITION BY RANGE(i)' -c 'CREATE TABLE t1 PARTITION OF t FOR VALUES FROM (1)TO(2)'
|pg_dump postgres -Fc -t t1 >dump.t1
|psql postgres -c 'DROP TABLE t'
|pg_restore -d postgres ./dump.t1
|pg_restore: while PROCESSING TOC:
|pg_restore: from TOC entry 457; 1259 405311409 TABLE t1 pryzbyj
|pg_restore: error: could not execute query: ERROR: relation "public.t" does not exist
|Command was: CREATE TABLE public.t1 (
| i integer
|);
|ALTER TABLE ONLY public.t ATTACH PARTITION public.t1 FOR VALUES FROM (1) TO (2);
|
|pg_restore: error: could not execute query: ERROR: relation "public.t1" does not exist
|Command was: ALTER TABLE public.t1 OWNER TO pryzbyj;
|
|pg_restore: from TOC entry 4728; 0 405311409 TABLE DATA t1 pryzbyj
|pg_restore: error: could not execute query: ERROR: relation "public.t1" does not exist
|Command was: COPY public.t1 (i) FROM stdin;
|pg_restore: warning: errors ignored on restore: 3

Now that I look, it seems like this is calling PQexec(), which sends a single,
"simple" libpq message with:
|CREATE TABLE ..; ALTER TABLE .. ATTACH PARTITION;
..which is transactional, so when the 2nd command fails, the CREATE is rolled back.
https://www.postgresql.org/docs/9.5/libpq-exec.html#LIBPQ-EXEC-MAIN

Telsasoft does a lot of dynamic DDL, so this happens sometimes due to columns
added or promoted. Up to now, when this has come up, I've run:
pg_restore |grep -v 'ATTACH PARTITION' |psql. Am I missing something ?

The idea of being independently restorable maybe originated with Tom's comment
here: /messages/by-id/30049.1555537881@sss.pgh.pa.us

--
Justin

#2Justin Pryzby
pryzby@telsasoft.com
In reply to: Justin Pryzby (#1)
1 attachment(s)
Re: pg_dump, ATTACH, and independently restorable child partitions

On Fri, Oct 23, 2020 at 12:29:40AM -0500, Justin Pryzby wrote:

Since this commit, pg_dump CREATEs tables and then ATTACHes them:

|commit 33a53130a89447e171a8268ae0b221bb48af6468
|Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
|Date: Mon Jun 10 18:56:23 2019 -0400
|
| Make pg_dump emit ATTACH PARTITION instead of PARTITION OF (reprise)
|...
| This change also has the advantage that the partition is restorable from
| the dump (as a standalone table) even if its parent table isn't
| restored.

I like the idea of child tables being independently restorable, but it doesn't
seem to work.

...

Now that I look, it seems like this is calling PQexec(), which sends a single,
"simple" libpq message with:
|CREATE TABLE ..; ALTER TABLE .. ATTACH PARTITION;
..which is transactional, so when the 2nd command fails, the CREATE is rolled back.
https://www.postgresql.org/docs/9.5/libpq-exec.html#LIBPQ-EXEC-MAIN

The easy fix is to add an explicit begin/commit.

--
Justin

Attachments:

0001-pg_dump-Allow-child-partitions-to-be-independently-r.patchtext/plain; charset=us-asciiDownload
From 4ca8cf7de0e575b0175fbc3a5797b75857bd2fab Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 24 Oct 2020 14:51:18 -0500
Subject: [PATCH] pg_dump: Allow child partitions to be independently restored

..even if the parent doesn't exist, or has missing/incompatible columns

This seems to have been intended by commit 33a53130a
---
 src/bin/pg_dump/pg_dump.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 40844fa1e7..8a783f1d3e 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -15676,6 +15676,13 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 			binary_upgrade_set_pg_class_oids(fout, q,
 											 tbinfo->dobj.catId.oid, false);
 
+		/*
+		 * Use explicit transaction commands so that failure in a
+		 * following command in the same txn doesn't cause ROLLBACK of
+		 * the "CREATE TABLE".
+		 */
+		appendPQExpBufferStr(q, "begin;\n");
+
 		appendPQExpBuffer(q, "CREATE %s%s %s",
 						  tbinfo->relpersistence == RELPERSISTENCE_UNLOGGED ?
 						  "UNLOGGED " : "",
@@ -15896,6 +15903,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 		}
 		else
 			appendPQExpBufferStr(q, ";\n");
+		appendPQExpBufferStr(q, "commit;\n");
 
 		/* Materialized views can depend on extensions */
 		if (tbinfo->relkind == RELKIND_MATVIEW)
-- 
2.17.0

#3Justin Pryzby
pryzby@telsasoft.com
In reply to: Justin Pryzby (#2)
1 attachment(s)
Re: pg_dump, ATTACH, and independently restorable child partitions

On Sat, Oct 24, 2020 at 02:59:49PM -0500, Justin Pryzby wrote:

On Fri, Oct 23, 2020 at 12:29:40AM -0500, Justin Pryzby wrote:

Since this commit, pg_dump CREATEs tables and then ATTACHes them:

|commit 33a53130a89447e171a8268ae0b221bb48af6468
|Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
|Date: Mon Jun 10 18:56:23 2019 -0400
|
| Make pg_dump emit ATTACH PARTITION instead of PARTITION OF (reprise)
|...
| This change also has the advantage that the partition is restorable from
| the dump (as a standalone table) even if its parent table isn't
| restored.

I like the idea of child tables being independently restorable, but it doesn't
seem to work.

...

Now that I look, it seems like this is calling PQexec(), which sends a single,
"simple" libpq message with:
|CREATE TABLE ..; ALTER TABLE .. ATTACH PARTITION;
..which is transactional, so when the 2nd command fails, the CREATE is rolled back.
https://www.postgresql.org/docs/9.5/libpq-exec.html#LIBPQ-EXEC-MAIN

The easy fix is to add an explicit begin/commit.

Now with updated test script.

--
Justin

Attachments:

v2-0001-pg_dump-Allow-child-partitions-to-be-independentl.patchtext/x-diff; charset=us-asciiDownload
From 8e34426bf7d863e3e8878a2e31c64ca999ec4464 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 24 Oct 2020 14:51:18 -0500
Subject: [PATCH v2] pg_dump: Allow child partitions to be independently
 restored

..even if the parent doesn't exist, or has missing/incompatible columns

This seems to have been intended by commit 33a53130a
---
 src/bin/pg_dump/pg_dump.c        | 8 ++++++++
 src/bin/pg_dump/t/002_pg_dump.pl | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 03f9d4d9e8..4767418849 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -15651,6 +15651,13 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 			binary_upgrade_set_pg_class_oids(fout, q,
 											 tbinfo->dobj.catId.oid, false);
 
+		/*
+		 * Use explicit transaction commands so that failure in a
+		 * following command in the same txn doesn't cause ROLLBACK of
+		 * the "CREATE TABLE".
+		 */
+		appendPQExpBufferStr(q, "begin;\n");
+
 		appendPQExpBuffer(q, "CREATE %s%s %s",
 						  tbinfo->relpersistence == RELPERSISTENCE_UNLOGGED ?
 						  "UNLOGGED " : "",
@@ -15871,6 +15878,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 		}
 		else
 			appendPQExpBufferStr(q, ";\n");
+		appendPQExpBufferStr(q, "commit;\n");
 
 		/* Materialized views can depend on extensions */
 		if (tbinfo->relkind == RELKIND_MATVIEW)
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index ec63662060..90dc2c5ae9 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2343,6 +2343,7 @@ my %tests = (
 		regexp => qr/^
 			\Q-- Name: measurement;\E.*\n
 			\Q--\E\n\n
+			\Qbegin;\E\n
 			\QCREATE TABLE dump_test.measurement (\E\n
 			\s+\Qcity_id integer NOT NULL,\E\n
 			\s+\Qlogdate date NOT NULL,\E\n
@@ -2351,6 +2352,7 @@ my %tests = (
 			\s+\QCONSTRAINT measurement_peaktemp_check CHECK ((peaktemp >= '-460'::integer))\E\n
 			\)\n
 			\QPARTITION BY RANGE (logdate);\E\n
+			\Qcommit;\E\n
 			/xm,
 		like =>
 		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
-- 
2.17.0

#4Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Justin Pryzby (#2)
Re: pg_dump, ATTACH, and independently restorable child partitions

On 2020-Oct-24, Justin Pryzby wrote:

On Fri, Oct 23, 2020 at 12:29:40AM -0500, Justin Pryzby wrote:

Now that I look, it seems like this is calling PQexec(), which sends a single,
"simple" libpq message with:
|CREATE TABLE ..; ALTER TABLE .. ATTACH PARTITION;
..which is transactional, so when the 2nd command fails, the CREATE is rolled back.
https://www.postgresql.org/docs/9.5/libpq-exec.html#LIBPQ-EXEC-MAIN

The easy fix is to add an explicit begin/commit.

Hmm, I think this throws a warning when used with "pg_restore -1",
right? I don't think that's sufficient reason to discard the idea, but
it be better to find some other way.

I have no ideas ATM :-(

#5Justin Pryzby
pryzby@telsasoft.com
In reply to: Alvaro Herrera (#4)
2 attachment(s)
Re: pg_dump, ATTACH, and independently restorable child partitions

On Fri, Nov 06, 2020 at 11:18:35PM -0300, Alvaro Herrera wrote:

On 2020-Oct-24, Justin Pryzby wrote:

On Fri, Oct 23, 2020 at 12:29:40AM -0500, Justin Pryzby wrote:

Now that I look, it seems like this is calling PQexec(), which sends a single,
"simple" libpq message with:
|CREATE TABLE ..; ALTER TABLE .. ATTACH PARTITION;
..which is transactional, so when the 2nd command fails, the CREATE is rolled back.
https://www.postgresql.org/docs/9.5/libpq-exec.html#LIBPQ-EXEC-MAIN

The easy fix is to add an explicit begin/commit.

Hmm, I think this throws a warning when used with "pg_restore -1",
right? I don't think that's sufficient reason to discard the idea, but
it be better to find some other way.

Worse, right ? It'd commit in the middle and then continue outside of a txn.
I guess there's no test case for this :(

I have no ideas ATM :-(

1. Maybe pg_restore ExecuteSqlCommandBuf() should (always?) call
ExecuteSimpleCommands() instead of ExecuteSqlCommand(). It doesn't seem to
break anything (although that surprised me).

2. Otherwise, the createStmt would need to be split into a createStmt2 or a
char *createStmt[], which I think would then require changing the output
format. It seems clearly better to keep the sql commands split up initially
than to reverse engineer them during restore.

I tried using \x01 to separate commands, and strtok to split them to run them
individually. But that breaks the pg_dumpall tests. As an experiment, I used
\x00, which is somewhat invasive but actually works.

Obviously patching pg_dump will affect only future backups, and the pg_restore
patch allows independently restoring parent tables in existing dumps.

--
Justin

Attachments:

v2-0001-pg_restore-parse-and-run-separately-SQL-commands.patchtext/x-diff; charset=us-asciiDownload
From f1ce24cb79dd4cdcd93d602ddede711efff8227e Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 19 Nov 2020 19:10:02 -0600
Subject: [PATCH v2 1/2] pg_restore: parse and run separately SQL commands

If pg_dump emits multiple DDL commands, run them in separate transactions, to
avoid failures in the 2ndary (ALTER) commands from rolling back the first,
primary (CREATE) command.

XXX: is there any performance benefit possible if can we call
ExecuteSqlCommand() in some cases?  I don't think it matters much for DDL.
---
 src/bin/pg_dump/pg_backup_db.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 5ba43441f5..1565155e20 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -477,21 +477,20 @@ ExecuteSqlCommandBuf(Archive *AHX, const char *buf, size_t bufLen)
 	else
 	{
 		/*
-		 * General SQL commands; we assume that commands will not be split
-		 * across calls.
+		 * General SQL commands
 		 *
 		 * In most cases the data passed to us will be a null-terminated
 		 * string, but if it's not, we have to add a trailing null.
 		 */
 		if (buf[bufLen] == '\0')
-			ExecuteSqlCommand(AH, buf, "could not execute query");
+			ExecuteSimpleCommands(AH, buf, bufLen);
 		else
 		{
 			char	   *str = (char *) pg_malloc(bufLen + 1);
 
 			memcpy(str, buf, bufLen);
 			str[bufLen] = '\0';
-			ExecuteSqlCommand(AH, str, "could not execute query");
+			ExecuteSimpleCommands(AH, buf, bufLen);
 			free(str);
 		}
 	}
-- 
2.17.0

v2-0002-pg_dump-Allow-child-partitions-to-be-independentl.patchtext/x-diff; charset=us-asciiDownload
From c6ec806b245a08c47f6e1d7c4909a532f49e69e1 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 24 Oct 2020 14:51:18 -0500
Subject: [PATCH v2 2/2] pg_dump: Allow child partitions to be independently
 restored

..even if the parent doesn't exist, or has missing/incompatible columns

This seems to have been intended by commit 33a53130a
---
 src/bin/pg_dump/pg_backup_archiver.c | 50 +++++++++++++++++--
 src/bin/pg_dump/pg_dump.c            | 72 ++++++++++++++++++++++++++--
 src/bin/pg_dump/t/002_pg_dump.pl     |  2 +-
 3 files changed, 117 insertions(+), 7 deletions(-)

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index b961a24b36..becf9ec34d 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -143,6 +143,8 @@ static void inhibit_data_for_failed_table(ArchiveHandle *AH, TocEntry *te);
 
 static void StrictNamesCheck(RestoreOptions *ropt);
 
+static size_t WriteStrs(ArchiveHandle *AH, const char *c);
+
 
 /*
  * Allocate a new DumpOptions block containing all default values.
@@ -1081,9 +1083,20 @@ ArchiveEntry(Archive *AHX, CatalogId catalogId, DumpId dumpId,
 	newToc->tableam = opts->tableam ? pg_strdup(opts->tableam) : NULL;
 	newToc->owner = opts->owner ? pg_strdup(opts->owner) : NULL;
 	newToc->desc = pg_strdup(opts->description);
-	newToc->defn = opts->createStmt ? pg_strdup(opts->createStmt) : NULL;
 	newToc->dropStmt = opts->dropStmt ? pg_strdup(opts->dropStmt) : NULL;
 	newToc->copyStmt = opts->copyStmt ? pg_strdup(opts->copyStmt) : NULL;
+	if (opts->createStmt == NULL)
+		newToc->defn = NULL;
+	else
+	{
+		const char *p = opts->createStmt;
+		size_t	len;
+		for (; *p != 0;)
+			p += 1 + strlen(p);
+		len = p - opts->createStmt + 1;
+		newToc->defn = palloc(len);
+		memcpy(newToc->defn, opts->createStmt, len);
+	}
 
 	if (opts->nDeps > 0)
 	{
@@ -2037,6 +2050,29 @@ WriteStr(ArchiveHandle *AH, const char *c)
 	return res;
 }
 
+size_t
+WriteStrs(ArchiveHandle *AH, const char *c)
+{
+	size_t		res;
+
+	if (c)
+	{
+		int			len;
+		const char *s = c;
+		for ( ; *s; )
+			s += 1 + strlen(s);
+		len = s - c + 1;
+
+		res = WriteInt(AH, len);
+		AH->WriteBufPtr(AH, c, len);
+		res += len;
+	}
+	else
+		res = WriteInt(AH, -1);
+
+	return res;
+}
+
 char *
 ReadStr(ArchiveHandle *AH)
 {
@@ -2522,7 +2558,7 @@ WriteToc(ArchiveHandle *AH)
 		WriteStr(AH, te->tag);
 		WriteStr(AH, te->desc);
 		WriteInt(AH, te->section);
-		WriteStr(AH, te->defn);
+		WriteStrs(AH, te->defn);
 		WriteStr(AH, te->dropStmt);
 		WriteStr(AH, te->copyStmt);
 		WriteStr(AH, te->namespace);
@@ -3607,7 +3643,15 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
 	else
 	{
 		if (te->defn && strlen(te->defn) > 0)
-			ahprintf(AH, "%s\n\n", te->defn);
+		{
+			char *p = te->defn;
+			for (; *p;)
+			{
+				ahprintf(AH, "%s\n", p);
+				p += 1 + strlen(p);
+			}
+			ahprintf(AH, "\n");
+		}
 	}
 
 	/*
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index dc1d41dd8d..7c8af9b19d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2362,6 +2362,7 @@ refreshMatViewData(Archive *fout, TableDataInfo *tdinfo)
 
 	appendPQExpBuffer(q, "REFRESH MATERIALIZED VIEW %s;\n",
 					  fmtQualifiedDumpable(tbinfo));
+	appendPQExpBufferChar(q, '\x00');
 
 	if (tdinfo->dobj.dump & DUMP_COMPONENT_DATA)
 		ArchiveEntry(fout,
@@ -2925,6 +2926,7 @@ dumpDatabase(Archive *fout)
 		appendPQExpBuffer(creaQry, " TABLESPACE = %s",
 						  fmtId(tablespace));
 	appendPQExpBufferStr(creaQry, ";\n");
+	appendPQExpBufferChar(creaQry, '\x00');
 
 	appendPQExpBuffer(delQry, "DROP DATABASE %s;\n",
 					  qdatname);
@@ -2965,6 +2967,7 @@ dumpDatabase(Archive *fout)
 			appendPQExpBuffer(dbQry, "COMMENT ON DATABASE %s IS ", qdatname);
 			appendStringLiteralAH(dbQry, comment, fout);
 			appendPQExpBufferStr(dbQry, ";\n");
+			appendPQExpBufferChar(dbQry, '\x00');
 
 			ArchiveEntry(fout, nilCatalogId, createDumpId(),
 						 ARCHIVE_OPTS(.tag = labelq->data,
@@ -2994,6 +2997,7 @@ dumpDatabase(Archive *fout)
 		shres = ExecuteSqlQuery(fout, seclabelQry->data, PGRES_TUPLES_OK);
 		resetPQExpBuffer(seclabelQry);
 		emitShSecLabels(conn, shres, seclabelQry, "DATABASE", datname);
+		appendPQExpBufferChar(seclabelQry, '\x00');
 		if (seclabelQry->len > 0)
 			ArchiveEntry(fout, nilCatalogId, createDumpId(),
 						 ARCHIVE_OPTS(.tag = labelq->data,
@@ -3064,6 +3068,7 @@ dumpDatabase(Archive *fout)
 						  frozenxid, minmxid);
 		appendStringLiteralAH(creaQry, datname, fout);
 		appendPQExpBufferStr(creaQry, ";\n");
+		appendPQExpBufferChar(creaQry, '\x00');
 	}
 
 	if (creaQry->len > 0)
@@ -3114,6 +3119,7 @@ dumpDatabase(Archive *fout)
 						  atooid(PQgetvalue(lo_res, 0, i_relfrozenxid)),
 						  atooid(PQgetvalue(lo_res, 0, i_relminmxid)),
 						  LargeObjectRelationId);
+		appendPQExpBufferChar(loOutQry, '\x00');
 		ArchiveEntry(fout, nilCatalogId, createDumpId(),
 					 ARCHIVE_OPTS(.tag = "pg_largeobject",
 								  .description = "pg_largeobject",
@@ -3221,6 +3227,7 @@ dumpEncoding(Archive *AH)
 	appendPQExpBufferStr(qry, "SET client_encoding = ");
 	appendStringLiteralAH(qry, encname, AH);
 	appendPQExpBufferStr(qry, ";\n");
+	appendPQExpBufferChar(qry, '\x00');
 
 	ArchiveEntry(AH, nilCatalogId, createDumpId(),
 				 ARCHIVE_OPTS(.tag = "ENCODING",
@@ -3246,6 +3253,7 @@ dumpStdStrings(Archive *AH)
 
 	appendPQExpBuffer(qry, "SET standard_conforming_strings = '%s';\n",
 					  stdstrings);
+	appendPQExpBufferChar(qry, '\x00');
 
 	ArchiveEntry(AH, nilCatalogId, createDumpId(),
 				 ARCHIVE_OPTS(.tag = "STDSTRINGS",
@@ -3298,6 +3306,7 @@ dumpSearchPath(Archive *AH)
 	appendPQExpBufferStr(qry, "SELECT pg_catalog.set_config('search_path', ");
 	appendStringLiteralAH(qry, path->data, AH);
 	appendPQExpBufferStr(qry, ", false);\n");
+	appendPQExpBufferChar(qry, '\x00');
 
 	pg_log_info("saving search_path = %s", path->data);
 
@@ -3468,6 +3477,7 @@ dumpBlob(Archive *fout, BlobInfo *binfo)
 	appendPQExpBuffer(cquery,
 					  "SELECT pg_catalog.lo_create('%s');\n",
 					  binfo->dobj.name);
+	appendPQExpBufferChar(cquery, '\x00');
 
 	appendPQExpBuffer(dquery,
 					  "SELECT pg_catalog.lo_unlink('%s');\n",
@@ -3769,6 +3779,7 @@ dumpPolicy(Archive *fout, PolicyInfo *polinfo)
 
 		appendPQExpBuffer(query, "ALTER TABLE %s ENABLE ROW LEVEL SECURITY;",
 						  fmtQualifiedDumpable(tbinfo));
+		appendPQExpBufferChar(query, '\x00');
 
 		/*
 		 * We must emit the ROW SECURITY object's dependency on its table
@@ -3828,6 +3839,7 @@ dumpPolicy(Archive *fout, PolicyInfo *polinfo)
 		appendPQExpBuffer(query, " WITH CHECK (%s)", polinfo->polwithcheck);
 
 	appendPQExpBufferStr(query, ";\n");
+	appendPQExpBufferChar(query, '\x00');
 
 	appendPQExpBuffer(delqry, "DROP POLICY %s", fmtId(polinfo->polname));
 	appendPQExpBuffer(delqry, " ON %s;\n", fmtQualifiedDumpable(tbinfo));
@@ -4033,6 +4045,7 @@ dumpPublication(Archive *fout, PublicationInfo *pubinfo)
 		appendPQExpBufferStr(query, ", publish_via_partition_root = true");
 
 	appendPQExpBufferStr(query, ");\n");
+	appendPQExpBufferChar(query, '\x00');
 
 	ArchiveEntry(fout, pubinfo->dobj.catId, pubinfo->dobj.dumpId,
 				 ARCHIVE_OPTS(.tag = pubinfo->dobj.name,
@@ -4172,6 +4185,7 @@ dumpPublicationTable(Archive *fout, PublicationRelInfo *pubrinfo)
 					  fmtId(pubrinfo->pubname));
 	appendPQExpBuffer(query, " %s;\n",
 					  fmtQualifiedDumpable(tbinfo));
+	appendPQExpBufferChar(query, '\x00');
 
 	/*
 	 * There is no point in creating drop query as the drop is done by table
@@ -4384,6 +4398,7 @@ dumpSubscription(Archive *fout, SubscriptionInfo *subinfo)
 		appendPQExpBuffer(query, ", synchronous_commit = %s", fmtId(subinfo->subsynccommit));
 
 	appendPQExpBufferStr(query, ");\n");
+	appendPQExpBufferChar(query, '\x00');
 
 	ArchiveEntry(fout, subinfo->dobj.catId, subinfo->dobj.dumpId,
 				 ARCHIVE_OPTS(.tag = subinfo->dobj.name,
@@ -9801,6 +9816,7 @@ dumpComment(Archive *fout, const char *type, const char *name,
 		appendPQExpBuffer(query, "%s IS ", name);
 		appendStringLiteralAH(query, comments->descr, fout);
 		appendPQExpBufferStr(query, ";\n");
+		appendPQExpBufferChar(query, '\x00');
 
 		appendPQExpBuffer(tag, "%s %s", type, name);
 
@@ -9877,6 +9893,7 @@ dumpTableComment(Archive *fout, TableInfo *tbinfo,
 							  fmtQualifiedDumpable(tbinfo));
 			appendStringLiteralAH(query, descr, fout);
 			appendPQExpBufferStr(query, ";\n");
+			appendPQExpBufferChar(query, '\x00');
 
 			ArchiveEntry(fout, nilCatalogId, createDumpId(),
 						 ARCHIVE_OPTS(.tag = tag->data,
@@ -9902,6 +9919,7 @@ dumpTableComment(Archive *fout, TableInfo *tbinfo,
 							  fmtId(tbinfo->attnames[objsubid - 1]));
 			appendStringLiteralAH(query, descr, fout);
 			appendPQExpBufferStr(query, ";\n");
+			appendPQExpBufferChar(query, '\x00');
 
 			ArchiveEntry(fout, nilCatalogId, createDumpId(),
 						 ARCHIVE_OPTS(.tag = tag->data,
@@ -10252,6 +10270,7 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
 	appendPQExpBuffer(delq, "DROP SCHEMA %s;\n", qnspname);
 
 	appendPQExpBuffer(q, "CREATE SCHEMA %s;\n", qnspname);
+	appendPQExpBufferChar(q, '\x00');
 
 	if (dopt->binary_upgrade)
 		binary_upgrade_extension_member(q, &nspinfo->dobj,
@@ -10392,6 +10411,7 @@ dumpExtension(Archive *fout, ExtensionInfo *extinfo)
 		appendPQExpBufferStr(q, ");\n");
 	}
 
+	appendPQExpBufferChar(q, '\x00');
 	if (extinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, extinfo->dobj.catId, extinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = extinfo->dobj.name,
@@ -10540,6 +10560,7 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
 										"TYPE", qtypname,
 										tyinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (tyinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tyinfo->dobj.name,
@@ -10666,6 +10687,7 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
 										"TYPE", qtypname,
 										tyinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (tyinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tyinfo->dobj.name,
@@ -10738,6 +10760,7 @@ dumpUndefinedType(Archive *fout, TypeInfo *tyinfo)
 										"TYPE", qtypname,
 										tyinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (tyinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tyinfo->dobj.name,
@@ -11019,6 +11042,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
 										"TYPE", qtypname,
 										tyinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (tyinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tyinfo->dobj.name,
@@ -11175,6 +11199,7 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
 										"DOMAIN", qtypname,
 										tyinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (tyinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tyinfo->dobj.name,
@@ -11396,6 +11421,7 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
 										"TYPE", qtypname,
 										tyinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (tyinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tyinfo->dobj.name,
@@ -11531,6 +11557,7 @@ dumpCompositeTypeColComments(Archive *fout, TypeInfo *tyinfo)
 			appendPQExpBuffer(query, "%s IS ", fmtId(attname));
 			appendStringLiteralAH(query, descr, fout);
 			appendPQExpBufferStr(query, ";\n");
+			appendPQExpBufferChar(query, '\x00');
 
 			ArchiveEntry(fout, nilCatalogId, createDumpId(),
 						 ARCHIVE_OPTS(.tag = target->data,
@@ -11587,6 +11614,7 @@ dumpShellType(Archive *fout, ShellTypeInfo *stinfo)
 	appendPQExpBuffer(q, "CREATE TYPE %s;\n",
 					  fmtQualifiedDumpable(stinfo));
 
+	appendPQExpBufferChar(q, '\x00');
 	if (stinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, stinfo->dobj.catId, stinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = stinfo->dobj.name,
@@ -11697,6 +11725,7 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
 		binary_upgrade_extension_member(defqry, &plang->dobj,
 										"LANGUAGE", qlanname, NULL);
 
+	appendPQExpBufferChar(defqry, '\x00');
 	if (plang->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, plang->dobj.catId, plang->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = plang->dobj.name,
@@ -12318,6 +12347,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
 										keyword, funcsig,
 										finfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (finfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, finfo->dobj.catId, finfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = funcsig_tag,
@@ -12452,6 +12482,7 @@ dumpCast(Archive *fout, CastInfo *cast)
 		binary_upgrade_extension_member(defqry, &cast->dobj,
 										"CAST", castargs->data, NULL);
 
+	appendPQExpBufferChar(defqry, '\x00');
 	if (cast->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, cast->dobj.catId, cast->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = labelq->data,
@@ -12579,6 +12610,7 @@ dumpTransform(Archive *fout, TransformInfo *transform)
 		binary_upgrade_extension_member(defqry, &transform->dobj,
 										"TRANSFORM", transformargs->data, NULL);
 
+	appendPQExpBufferChar(defqry, '\x00');
 	if (transform->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, transform->dobj.catId, transform->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = labelq->data,
@@ -12799,6 +12831,7 @@ dumpOpr(Archive *fout, OprInfo *oprinfo)
 										"OPERATOR", oprid->data,
 										oprinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (oprinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, oprinfo->dobj.catId, oprinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = oprinfo->dobj.name,
@@ -12971,6 +13004,7 @@ dumpAccessMethod(Archive *fout, AccessMethodInfo *aminfo)
 		binary_upgrade_extension_member(q, &aminfo->dobj,
 										"ACCESS METHOD", qamname, NULL);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (aminfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, aminfo->dobj.catId, aminfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = aminfo->dobj.name,
@@ -13334,6 +13368,7 @@ dumpOpclass(Archive *fout, OpclassInfo *opcinfo)
 										"OPERATOR CLASS", nameusing->data,
 										opcinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (opcinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, opcinfo->dobj.catId, opcinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = opcinfo->dobj.name,
@@ -13600,6 +13635,7 @@ dumpOpfamily(Archive *fout, OpfamilyInfo *opfinfo)
 										"OPERATOR FAMILY", nameusing->data,
 										opfinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (opfinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, opfinfo->dobj.catId, opfinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = opfinfo->dobj.name,
@@ -13732,6 +13768,7 @@ dumpCollation(Archive *fout, CollInfo *collinfo)
 										"COLLATION", qcollname,
 										collinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (collinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, collinfo->dobj.catId, collinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = collinfo->dobj.name,
@@ -13826,6 +13863,7 @@ dumpConversion(Archive *fout, ConvInfo *convinfo)
 										"CONVERSION", qconvname,
 										convinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (convinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, convinfo->dobj.catId, convinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = convinfo->dobj.name,
@@ -14200,6 +14238,7 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
 										"AGGREGATE", aggsig,
 										agginfo->aggfn.dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (agginfo->aggfn.dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, agginfo->aggfn.dobj.catId,
 					 agginfo->aggfn.dobj.dumpId,
@@ -14298,6 +14337,7 @@ dumpTSParser(Archive *fout, TSParserInfo *prsinfo)
 										"TEXT SEARCH PARSER", qprsname,
 										prsinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (prsinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, prsinfo->dobj.catId, prsinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = prsinfo->dobj.name,
@@ -14376,6 +14416,7 @@ dumpTSDictionary(Archive *fout, TSDictInfo *dictinfo)
 										"TEXT SEARCH DICTIONARY", qdictname,
 										dictinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (dictinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, dictinfo->dobj.catId, dictinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = dictinfo->dobj.name,
@@ -14436,6 +14477,7 @@ dumpTSTemplate(Archive *fout, TSTemplateInfo *tmplinfo)
 										"TEXT SEARCH TEMPLATE", qtmplname,
 										tmplinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (tmplinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, tmplinfo->dobj.catId, tmplinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tmplinfo->dobj.name,
@@ -14554,6 +14596,7 @@ dumpTSConfig(Archive *fout, TSConfigInfo *cfginfo)
 										"TEXT SEARCH CONFIGURATION", qcfgname,
 										cfginfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (cfginfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, cfginfo->dobj.catId, cfginfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = cfginfo->dobj.name,
@@ -14619,6 +14662,7 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
 										"FOREIGN DATA WRAPPER", qfdwname,
 										NULL);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (fdwinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, fdwinfo->dobj.catId, fdwinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = fdwinfo->dobj.name,
@@ -14708,6 +14752,7 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
 		binary_upgrade_extension_member(q, &srvinfo->dobj,
 										"SERVER", qsrvname, NULL);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (srvinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, srvinfo->dobj.catId, srvinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = srvinfo->dobj.name,
@@ -14825,6 +14870,7 @@ dumpUserMappings(Archive *fout,
 		appendPQExpBuffer(tag, "USER MAPPING %s SERVER %s",
 						  usename, servername);
 
+		appendPQExpBufferChar(q, '\x00');
 		ArchiveEntry(fout, nilCatalogId, createDumpId(),
 					 ARCHIVE_OPTS(.tag = tag->data,
 								  .namespace = namespace,
@@ -14901,6 +14947,7 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
 		fatal("could not parse default ACL list (%s)",
 			  daclinfo->defaclacl);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (daclinfo->dobj.dump & DUMP_COMPONENT_ACL)
 		ArchiveEntry(fout, daclinfo->dobj.catId, daclinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tag->data,
@@ -15011,6 +15058,7 @@ dumpACL(Archive *fout, DumpId objDumpId, DumpId altDumpId,
 
 		aclDumpId = createDumpId();
 
+		appendPQExpBufferChar(sql, '\x00');
 		ArchiveEntry(fout, nilCatalogId, aclDumpId,
 					 ARCHIVE_OPTS(.tag = tag->data,
 								  .namespace = nspname,
@@ -15103,6 +15151,7 @@ dumpSecLabel(Archive *fout, const char *type, const char *name,
 		PQExpBuffer tag = createPQExpBuffer();
 
 		appendPQExpBuffer(tag, "%s %s", type, name);
+		appendPQExpBufferChar(query, '\x00');
 		ArchiveEntry(fout, nilCatalogId, createDumpId(),
 					 ARCHIVE_OPTS(.tag = tag->data,
 								  .namespace = namespace,
@@ -15186,6 +15235,7 @@ dumpTableSecLabel(Archive *fout, TableInfo *tbinfo, const char *reltypename)
 		resetPQExpBuffer(target);
 		appendPQExpBuffer(target, "%s %s", reltypename,
 						  fmtId(tbinfo->dobj.name));
+		appendPQExpBufferChar(query, '\x00');
 		ArchiveEntry(fout, nilCatalogId, createDumpId(),
 					 ARCHIVE_OPTS(.tag = target->data,
 								  .namespace = tbinfo->dobj.namespace->dobj.name,
@@ -15917,12 +15967,12 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 			PQExpBuffer result;
 
 			result = createViewAsClause(fout, tbinfo);
-			appendPQExpBuffer(q, " AS\n%s\n  WITH NO DATA;\n",
-							  result->data);
+			appendPQExpBuffer(q, " AS\n%s\n  WITH NO DATA;%c\n",
+							  result->data, '\x00');
 			destroyPQExpBuffer(result);
 		}
 		else
-			appendPQExpBufferStr(q, ";\n");
+			appendPQExpBuffer(q, ";%c\n", '\x00');
 
 		/* Materialized views can depend on extensions */
 		if (tbinfo->relkind == RELKIND_MATVIEW)
@@ -16279,6 +16329,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 			tbinfo->relkind == RELKIND_MATVIEW)
 			tableam = tbinfo->amname;
 
+		appendPQExpBufferChar(q, '\x00');
 		ArchiveEntry(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tbinfo->dobj.name,
 								  .namespace = tbinfo->dobj.namespace->dobj.name,
@@ -16360,6 +16411,7 @@ dumpAttrDef(Archive *fout, AttrDefInfo *adinfo)
 
 	tag = psprintf("%s %s", tbinfo->dobj.name, tbinfo->attnames[adnum - 1]);
 
+	appendPQExpBufferChar(q, '\x00');
 	if (adinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, adinfo->dobj.catId, adinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tag,
@@ -16523,6 +16575,7 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
 
 		appendPQExpBuffer(delq, "DROP INDEX %s;\n", qqindxname);
 
+		appendPQExpBufferChar(q, '\x00');
 		if (indxinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 			ArchiveEntry(fout, indxinfo->dobj.catId, indxinfo->dobj.dumpId,
 						 ARCHIVE_OPTS(.tag = indxinfo->dobj.name,
@@ -16544,6 +16597,7 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
 		appendIndexCollationVersion(q, indxinfo, fout->encoding,
 									dopt->coll_unknown, fout);
 
+		appendPQExpBufferChar(q, '\x00');
 		if (indxinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 			ArchiveEntry(fout, indxinfo->dobj.catId, indxinfo->dobj.dumpId,
 						 ARCHIVE_OPTS(.tag = indxinfo->dobj.name,
@@ -16589,6 +16643,7 @@ dumpIndexAttach(Archive *fout, IndexAttachInfo *attachinfo)
 		appendPQExpBuffer(q, "ATTACH PARTITION %s;\n",
 						  fmtQualifiedDumpable(attachinfo->partitionIdx));
 
+		appendPQExpBufferChar(q, '\x00');
 		ArchiveEntry(fout, attachinfo->dobj.catId, attachinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = attachinfo->dobj.name,
 								  .namespace = attachinfo->dobj.namespace->dobj.name,
@@ -16652,6 +16707,7 @@ dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
 	appendPQExpBuffer(delq, "DROP STATISTICS %s;\n",
 					  fmtQualifiedDumpable(statsextinfo));
 
+	appendPQExpBufferChar(q, '\x00');
 	if (statsextinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, statsextinfo->dobj.catId,
 					 statsextinfo->dobj.dumpId,
@@ -16822,6 +16878,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 
 		tag = psprintf("%s %s", tbinfo->dobj.name, coninfo->dobj.name);
 
+		appendPQExpBufferChar(q, '\x00');
 		if (coninfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 			ArchiveEntry(fout, coninfo->dobj.catId, coninfo->dobj.dumpId,
 						 ARCHIVE_OPTS(.tag = tag,
@@ -16862,6 +16919,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 
 		tag = psprintf("%s %s", tbinfo->dobj.name, coninfo->dobj.name);
 
+		appendPQExpBufferChar(q, '\x00');
 		if (coninfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 			ArchiveEntry(fout, coninfo->dobj.catId, coninfo->dobj.dumpId,
 						 ARCHIVE_OPTS(.tag = tag,
@@ -16893,6 +16951,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 
 			tag = psprintf("%s %s", tbinfo->dobj.name, coninfo->dobj.name);
 
+			appendPQExpBufferChar(q, '\x00');
 			if (coninfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 				ArchiveEntry(fout, coninfo->dobj.catId, coninfo->dobj.dumpId,
 							 ARCHIVE_OPTS(.tag = tag,
@@ -16925,6 +16984,7 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 
 			tag = psprintf("%s %s", tyinfo->dobj.name, coninfo->dobj.name);
 
+			appendPQExpBufferChar(q, '\x00');
 			if (coninfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 				ArchiveEntry(fout, coninfo->dobj.catId, coninfo->dobj.dumpId,
 							 ARCHIVE_OPTS(.tag = tag,
@@ -17201,6 +17261,7 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
 										"SEQUENCE", qseqname,
 										tbinfo->dobj.namespace->dobj.name);
 
+	appendPQExpBufferChar(query, '\x00');
 	if (tbinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tbinfo->dobj.name,
@@ -17241,6 +17302,7 @@ dumpSequence(Archive *fout, TableInfo *tbinfo)
 			appendPQExpBuffer(query, ".%s;\n",
 							  fmtId(owning_tab->attnames[tbinfo->owning_col - 1]));
 
+			appendPQExpBufferChar(query, '\x00');
 			if (tbinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 				ArchiveEntry(fout, nilCatalogId, createDumpId(),
 							 ARCHIVE_OPTS(.tag = tbinfo->dobj.name,
@@ -17309,6 +17371,7 @@ dumpSequenceData(Archive *fout, TableDataInfo *tdinfo)
 	appendPQExpBuffer(query, ", %s, %s);\n",
 					  last, (called ? "true" : "false"));
 
+	appendPQExpBufferChar(query, '\x00');
 	if (tdinfo->dobj.dump & DUMP_COMPONENT_DATA)
 		ArchiveEntry(fout, nilCatalogId, createDumpId(),
 					 ARCHIVE_OPTS(.tag = tbinfo->dobj.name,
@@ -17516,6 +17579,7 @@ dumpTrigger(Archive *fout, TriggerInfo *tginfo)
 
 	tag = psprintf("%s %s", tbinfo->dobj.name, tginfo->dobj.name);
 
+	appendPQExpBufferChar(query, '\x00');
 	if (tginfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, tginfo->dobj.catId, tginfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tag,
@@ -17605,6 +17669,7 @@ dumpEventTrigger(Archive *fout, EventTriggerInfo *evtinfo)
 		binary_upgrade_extension_member(query, &evtinfo->dobj,
 										"EVENT TRIGGER", qevtname, NULL);
 
+	appendPQExpBufferChar(query, '\x00');
 	if (evtinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, evtinfo->dobj.catId, evtinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = evtinfo->dobj.name,
@@ -17763,6 +17828,7 @@ dumpRule(Archive *fout, RuleInfo *rinfo)
 
 	tag = psprintf("%s %s", tbinfo->dobj.name, rinfo->dobj.name);
 
+	appendPQExpBufferChar(cmd, '\x00');
 	if (rinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, rinfo->dobj.catId, rinfo->dobj.dumpId,
 					 ARCHIVE_OPTS(.tag = tag,
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index ec63662060..9eab65e4f3 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2351,7 +2351,7 @@ my %tests = (
 			\s+\QCONSTRAINT measurement_peaktemp_check CHECK ((peaktemp >= '-460'::integer))\E\n
 			\)\n
 			\QPARTITION BY RANGE (logdate);\E\n
-			/xm,
+			/xm, # XXX: how to test that it's in a separate txn ?
 		like =>
 		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
 		unlike => {
-- 
2.17.0

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Justin Pryzby (#5)
Re: pg_dump, ATTACH, and independently restorable child partitions

Justin Pryzby <pryzby@telsasoft.com> writes:

1. Maybe pg_restore ExecuteSqlCommandBuf() should (always?) call
ExecuteSimpleCommands() instead of ExecuteSqlCommand(). It doesn't seem to
break anything (although that surprised me).

That certainly does break everything, which I imagine is the reason
why the cfbot shows that this patch is failing the pg_upgrade tests.
Note the comments for ExecuteSimpleCommands:

* We have to lex the data to the extent of identifying literals and quoted
* identifiers, so that we can recognize statement-terminating semicolons.
* We assume that INSERT data will not contain SQL comments, E'' literals,
* or dollar-quoted strings, so this is much simpler than a full SQL lexer.

IOW, where that says "Simple", it means *simple* --- in practice,
we only risk using it on commands that we know pg_dump itself built
earlier. There is no reasonable prospect of getting pg_restore to
split arbitrary SQL at command boundaries. We'd need something
comparable to psql's lexer, which is huge, and from a future-proofing
standpoint it would be just awful. (The worst that happens if psql
misparses your string is that it won't send the command when you
expected. If pg_restore misparses stuff, your best case is that the
restore fails cleanly; the worst case could easily result in
SQL-injection compromises.) So I think we cannot follow this
approach.

What we'd need to do if we want this to work with direct-to-DB restore
is to split off the ATTACH PARTITION command as a separate TOC entry.
That doesn't seem amazingly difficult, and it would even offer the
possibility that you could extract the partition standalone without
having to ignore errors. (You'd use -l/-L to select the CREATE TABLE,
the data, etc, but not the ATTACH object.)

That would possibly come out as a larger patch than you have here,
but maybe not by much. I don't think there's too much more involved
than setting up the proper command strings and calling ArchiveEntry().
You'd need to do some testing to verify that cases like --clean
work sanely.

Also, I read the 0002 patch briefly and couldn't make heads or tails
of it, except that it seemed to be abusing the PQExpBuffer abstraction
well beyond anything I'd consider acceptable. If you want separate
strings, make a PQExpBuffer for each one.

regards, tom lane

#7Justin Pryzby
pryzby@telsasoft.com
In reply to: Tom Lane (#6)
1 attachment(s)
Re: pg_dump, ATTACH, and independently restorable child partitions

On Wed, Nov 25, 2020 at 06:35:19PM -0500, Tom Lane wrote:

Justin Pryzby <pryzby@telsasoft.com> writes:

1. Maybe pg_restore ExecuteSqlCommandBuf() should (always?) call
ExecuteSimpleCommands() instead of ExecuteSqlCommand(). It doesn't seem to
break anything (although that surprised me).

That certainly does break everything, which I imagine is the reason
why the cfbot shows that this patch is failing the pg_upgrade tests.

Thanks for looking, I have tried this.

What we'd need to do if we want this to work with direct-to-DB restore
is to split off the ATTACH PARTITION command as a separate TOC entry.
That doesn't seem amazingly difficult, and it would even offer the
possibility that you could extract the partition standalone without
having to ignore errors. (You'd use -l/-L to select the CREATE TABLE,
the data, etc, but not the ATTACH object.)

--
Justin

Attachments:

v4-0001-pg_dump-output-separate-object-for-ALTER-TABLE.AT.patchtext/x-diff; charset=us-asciiDownload
From 0d0b013930199158306fd295eebbf36c4c4cb5b4 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 26 Nov 2020 22:02:07 -0600
Subject: [PATCH v4] pg_dump: output separate "object" for ALTER TABLE..ATTACH
 PARTITION

..allowing table partitions to be restored separately and independently, even
if there's an error attaching to parent table.
---
 src/bin/pg_dump/common.c       | 28 +++++++++++++
 src/bin/pg_dump/pg_dump.c      | 76 ++++++++++++++++++++++++----------
 src/bin/pg_dump/pg_dump.h      |  8 ++++
 src/bin/pg_dump/pg_dump_sort.c | 48 +++++++++++----------
 4 files changed, 118 insertions(+), 42 deletions(-)

diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 634ca86cfb..8450e22327 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -320,6 +320,34 @@ flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables,
 			for (j = 0; j < numParents; j++)
 				parents[j]->interesting = true;
 		}
+
+		if (tblinfo[i].dobj.dump && tblinfo[i].ispartition)
+		{
+			/* We don't even need to store this anywhere, but maybe there's
+			 * some utility in making a single, large allocation earlier ? */
+			TableAttachInfo *attachinfo = palloc(sizeof(*attachinfo));
+
+			attachinfo->dobj.objType = DO_TABLE_ATTACH;
+			attachinfo->dobj.catId.tableoid = 0;
+			attachinfo->dobj.catId.oid = 0;
+			AssignDumpId(&attachinfo->dobj);
+			attachinfo->dobj.name = pg_strdup(tblinfo[i].dobj.name);
+			attachinfo->dobj.namespace = tblinfo[i].dobj.namespace;
+			attachinfo->parentTbl = tblinfo[i].parents[0];
+			attachinfo->partitionTbl = &tblinfo[i];
+
+			/*
+			 * We must state the DO_TABLE_ATTACH object's dependencies
+			 * explicitly, since it will not match anything in pg_depend.
+			 *
+			 * Give it dependencies on both the partition table and the parent
+			 * table, so that it will not be executed till both of those
+			 * exist.  (There's no need to care what order those are created
+			 * in.)
+			 */
+			addObjectDependency(&attachinfo->dobj, tblinfo[i].dobj.dumpId);
+			addObjectDependency(&attachinfo->dobj, tblinfo[i].parents[0]->dobj.dumpId);
+		}
 	}
 }
 
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index dc1d41dd8d..f66182bc73 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -201,6 +201,7 @@ static void dumpAgg(Archive *fout, AggInfo *agginfo);
 static void dumpTrigger(Archive *fout, TriggerInfo *tginfo);
 static void dumpEventTrigger(Archive *fout, EventTriggerInfo *evtinfo);
 static void dumpTable(Archive *fout, TableInfo *tbinfo);
+static void dumpTableAttach(Archive *fout, TableAttachInfo *tbinfo);
 static void dumpTableSchema(Archive *fout, TableInfo *tbinfo);
 static void dumpAttrDef(Archive *fout, AttrDefInfo *adinfo);
 static void dumpSequence(Archive *fout, TableInfo *tbinfo);
@@ -10110,6 +10111,9 @@ dumpDumpableObject(Archive *fout, DumpableObject *dobj)
 		case DO_TABLE:
 			dumpTable(fout, (TableInfo *) dobj);
 			break;
+		case DO_TABLE_ATTACH:
+			dumpTableAttach(fout, (TableAttachInfo *) dobj);
+			break;
 		case DO_ATTRDEF:
 			dumpAttrDef(fout, (AttrDefInfo *) dobj);
 			break;
@@ -15573,6 +15577,55 @@ createDummyViewAsClause(Archive *fout, TableInfo *tbinfo)
 	return result;
 }
 
+/*
+ * dumpTableAttach
+ *	  write to fout the commands to attach a child partition
+ *
+ * For partitioned tables, emit the ATTACH PARTITION clause.  Note
+ * that we always want to create partitions this way instead of using
+ * CREATE TABLE .. PARTITION OF, mainly to preserve a possible column
+ * layout discrepancy with the parent, but also to ensure it gets the
+ * correct tablespace setting if it differs from the parent's.
+ */
+static void
+dumpTableAttach(Archive *fout, TableAttachInfo *attachinfo)
+{
+	DumpOptions *dopt = fout->dopt;
+	char	   *qualrelname;
+	PQExpBuffer q;
+
+	if (dopt->dataOnly)
+		return;
+
+	if (attachinfo->partitionTbl->dobj.dump & DUMP_COMPONENT_DEFINITION == 0)
+		return;
+
+	/* With partitions there can only be one parent */
+	if (attachinfo->partitionTbl->numParents != 1)
+		fatal("invalid number of parents %d for table \"%s\"",
+				attachinfo->partitionTbl->numParents,
+				attachinfo->partitionTbl->dobj.name);
+
+	qualrelname = pg_strdup(fmtQualifiedDumpable(attachinfo));
+	q = createPQExpBuffer();
+
+	/* Perform ALTER TABLE on the parent */
+	appendPQExpBuffer(q,
+					  "ALTER TABLE ONLY %s ATTACH PARTITION %s %s;\n",
+					  fmtQualifiedDumpable(attachinfo->parentTbl),
+					  qualrelname, attachinfo->partitionTbl->partbound);
+
+	ArchiveEntry(fout, attachinfo->dobj.catId, attachinfo->dobj.dumpId,
+				 ARCHIVE_OPTS(.tag = attachinfo->dobj.name,
+							  .namespace = attachinfo->dobj.namespace->dobj.name,
+							  .description = "TABLE ATTACH",
+							  .section = SECTION_PRE_DATA,
+							  .createStmt = q->data));
+
+	free(qualrelname);
+	destroyPQExpBuffer(q);
+}
+
 /*
  * dumpTableSchema
  *	  write the declaration (not data) of one user-defined table or view
@@ -16069,27 +16122,6 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 			}
 		}
 
-		/*
-		 * For partitioned tables, emit the ATTACH PARTITION clause.  Note
-		 * that we always want to create partitions this way instead of using
-		 * CREATE TABLE .. PARTITION OF, mainly to preserve a possible column
-		 * layout discrepancy with the parent, but also to ensure it gets the
-		 * correct tablespace setting if it differs from the parent's.
-		 */
-		if (tbinfo->ispartition)
-		{
-			/* With partitions there can only be one parent */
-			if (tbinfo->numParents != 1)
-				fatal("invalid number of parents %d for table \"%s\"",
-					  tbinfo->numParents, tbinfo->dobj.name);
-
-			/* Perform ALTER TABLE on the parent */
-			appendPQExpBuffer(q,
-							  "ALTER TABLE ONLY %s ATTACH PARTITION %s %s;\n",
-							  fmtQualifiedDumpable(parents[0]),
-							  qualrelname, tbinfo->partbound);
-		}
-
 		/*
 		 * In binary_upgrade mode, arrange to restore the old relfrozenxid and
 		 * relminmxid of all vacuumable relations.  (While vacuum.c processes
@@ -16291,6 +16323,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 								  SECTION_POST_DATA : SECTION_PRE_DATA,
 								  .createStmt = q->data,
 								  .dropStmt = delq->data));
+
 	}
 
 	/* Dump Table Comments */
@@ -18280,6 +18313,7 @@ addBoundaryDependencies(DumpableObject **dobjs, int numObjs,
 			case DO_COLLATION:
 			case DO_CONVERSION:
 			case DO_TABLE:
+			case DO_TABLE_ATTACH:
 			case DO_ATTRDEF:
 			case DO_PROCLANG:
 			case DO_CAST:
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 317bb83970..b9311f6e19 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -50,6 +50,7 @@ typedef enum
 	DO_COLLATION,
 	DO_CONVERSION,
 	DO_TABLE,
+	DO_TABLE_ATTACH,
 	DO_ATTRDEF,
 	DO_INDEX,
 	DO_INDEX_ATTACH,
@@ -337,6 +338,13 @@ typedef struct _tableInfo
 	struct _triggerInfo *triggers;	/* array of TriggerInfo structs */
 } TableInfo;
 
+typedef struct _tableAttachInfo
+{
+	DumpableObject dobj;
+	TableInfo   *parentTbl;		/* link to partitioned table */
+	TableInfo   *partitionTbl;	/* link to partition */
+} TableAttachInfo;
+
 typedef struct _attrDefInfo
 {
 	DumpableObject dobj;		/* note: dobj.name is name of table */
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 654e2ec514..c004cf603d 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -54,36 +54,37 @@ static const int dbObjectTypePriority[] =
 	3,							/* DO_COLLATION */
 	11,							/* DO_CONVERSION */
 	18,							/* DO_TABLE */
-	20,							/* DO_ATTRDEF */
-	28,							/* DO_INDEX */
-	29,							/* DO_INDEX_ATTACH */
-	30,							/* DO_STATSEXT */
-	31,							/* DO_RULE */
-	32,							/* DO_TRIGGER */
-	27,							/* DO_CONSTRAINT */
-	33,							/* DO_FK_CONSTRAINT */
+	19,							/* DO_TABLE_ATTACH */
+	21,							/* DO_ATTRDEF */
+	29,							/* DO_INDEX */
+	30,							/* DO_INDEX_ATTACH */
+	31,							/* DO_STATSEXT */
+	32,							/* DO_RULE */
+	33,							/* DO_TRIGGER */
+	28,							/* DO_CONSTRAINT */
+	34,							/* DO_FK_CONSTRAINT */
 	2,							/* DO_PROCLANG */
 	10,							/* DO_CAST */
-	23,							/* DO_TABLE_DATA */
-	24,							/* DO_SEQUENCE_SET */
-	19,							/* DO_DUMMY_TYPE */
+	24,							/* DO_TABLE_DATA */
+	25,							/* DO_SEQUENCE_SET */
+	21,							/* DO_DUMMY_TYPE */
 	12,							/* DO_TSPARSER */
 	14,							/* DO_TSDICT */
 	13,							/* DO_TSTEMPLATE */
 	15,							/* DO_TSCONFIG */
 	16,							/* DO_FDW */
 	17,							/* DO_FOREIGN_SERVER */
-	38,							/* DO_DEFAULT_ACL --- done in ACL pass */
+	39,							/* DO_DEFAULT_ACL --- done in ACL pass */
 	3,							/* DO_TRANSFORM */
-	21,							/* DO_BLOB */
-	25,							/* DO_BLOB_DATA */
-	22,							/* DO_PRE_DATA_BOUNDARY */
-	26,							/* DO_POST_DATA_BOUNDARY */
-	39,							/* DO_EVENT_TRIGGER --- next to last! */
-	40,							/* DO_REFRESH_MATVIEW --- last! */
-	34,							/* DO_POLICY */
-	35,							/* DO_PUBLICATION */
-	36,							/* DO_PUBLICATION_REL */
+	22,							/* DO_BLOB */
+	26,							/* DO_BLOB_DATA */
+	23,							/* DO_PRE_DATA_BOUNDARY */
+	27,							/* DO_POST_DATA_BOUNDARY */
+	40,							/* DO_EVENT_TRIGGER --- next to last! */
+	41,							/* DO_REFRESH_MATVIEW --- last! */
+	35,							/* DO_POLICY */
+	36,							/* DO_PUBLICATION */
+	37,							/* DO_PUBLICATION_REL */
 	37							/* DO_SUBSCRIPTION */
 };
 
@@ -1275,6 +1276,11 @@ describeDumpableObject(DumpableObject *obj, char *buf, int bufsize)
 					 "TABLE %s  (ID %d OID %u)",
 					 obj->name, obj->dumpId, obj->catId.oid);
 			return;
+		case DO_TABLE_ATTACH:
+			snprintf(buf, bufsize,
+					 "TABLE ATTACH %s  (ID %d)",
+					 obj->name, obj->dumpId);
+			return;
 		case DO_ATTRDEF:
 			snprintf(buf, bufsize,
 					 "ATTRDEF %s.%s  (ID %d OID %u)",
-- 
2.17.0

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Justin Pryzby (#7)
Re: pg_dump, ATTACH, and independently restorable child partitions

Justin Pryzby <pryzby@telsasoft.com> writes:

[ v4-0001-pg_dump-output-separate-object-for-ALTER-TABLE.AT.patch ]

The cfbot is being picky about this:

3218pg_dump.c: In function ‘dumpTableAttach’:
3219pg_dump.c:15600:42: error: suggest parentheses around comparison in operand of ‘&’ [-Werror=parentheses]
3220 if (attachinfo->partitionTbl->dobj.dump & DUMP_COMPONENT_DEFINITION == 0)
3221 ^
3222cc1: all warnings being treated as errors

which if I've got the precedence straight is indeed a bug.

Personally I'd probably write

if (!(attachinfo->partitionTbl->dobj.dump & DUMP_COMPONENT_DEFINITION))

as it seems like a boolean test to me.

regards, tom lane

#9Justin Pryzby
pryzby@telsasoft.com
In reply to: Tom Lane (#8)
1 attachment(s)
Re: pg_dump, ATTACH, and independently restorable child partitions

On Fri, Dec 04, 2020 at 12:13:05PM -0500, Tom Lane wrote:

Justin Pryzby <pryzby@telsasoft.com> writes:

[ v4-0001-pg_dump-output-separate-object-for-ALTER-TABLE.AT.patch ]

The cfbot is being picky about this:

3218pg_dump.c: In function ‘dumpTableAttach’:
3219pg_dump.c:15600:42: error: suggest parentheses around comparison in operand of ‘&’ [-Werror=parentheses]
3220 if (attachinfo->partitionTbl->dobj.dump & DUMP_COMPONENT_DEFINITION == 0)
3221 ^
3222cc1: all warnings being treated as errors

which if I've got the precedence straight is indeed a bug.

Oops - from a last-minute edit.
I missed it due to cfboot being slow, and clogged up with duplicate entries.
This also adds/updates comments.

--
Justin

Attachments:

v5-0001-pg_dump-output-separate-object-for-ALTER-TABLE.AT.patchtext/x-diff; charset=us-asciiDownload
From 67b5d61a45bdded00661c13c71f901750e4b48a9 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Thu, 26 Nov 2020 22:02:07 -0600
Subject: [PATCH v5] pg_dump: output separate "object" for ALTER TABLE..ATTACH
 PARTITION

..allowing table partitions to be restored separately and independently, even
if there's an error attaching to parent table.
---
 src/bin/pg_dump/common.c       | 33 ++++++++++++++-
 src/bin/pg_dump/pg_dump.c      | 76 ++++++++++++++++++++++++----------
 src/bin/pg_dump/pg_dump.h      |  8 ++++
 src/bin/pg_dump/pg_dump_sort.c | 48 +++++++++++----------
 4 files changed, 122 insertions(+), 43 deletions(-)

diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 634ca86cfb..5c8aa80cc4 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -261,7 +261,9 @@ getSchemaData(Archive *fout, int *numTablesPtr)
 
 /* flagInhTables -
  *	 Fill in parent link fields of tables for which we need that information,
- *	 and mark parents of target tables as interesting
+ *	 mark parents of target tables as interesting, and create
+ *	 TableAttachInfo objects for partitioned tables with appropriate
+ *	 dependency links.
  *
  * Note that only direct ancestors of targets are marked interesting.
  * This is sufficient; we don't much care whether they inherited their
@@ -320,6 +322,35 @@ flagInhTables(Archive *fout, TableInfo *tblinfo, int numTables,
 			for (j = 0; j < numParents; j++)
 				parents[j]->interesting = true;
 		}
+
+		/* Dump ALTER TABLE .. ATTACH PARTITION */
+		if (tblinfo[i].dobj.dump && tblinfo[i].ispartition)
+		{
+			/* We don't even need to store this anywhere, but maybe there's
+			 * some utility in making a single, large allocation earlier ? */
+			TableAttachInfo *attachinfo = palloc(sizeof(*attachinfo));
+
+			attachinfo->dobj.objType = DO_TABLE_ATTACH;
+			attachinfo->dobj.catId.tableoid = 0;
+			attachinfo->dobj.catId.oid = 0;
+			AssignDumpId(&attachinfo->dobj);
+			attachinfo->dobj.name = pg_strdup(tblinfo[i].dobj.name);
+			attachinfo->dobj.namespace = tblinfo[i].dobj.namespace;
+			attachinfo->parentTbl = tblinfo[i].parents[0];
+			attachinfo->partitionTbl = &tblinfo[i];
+
+			/*
+			 * We must state the DO_TABLE_ATTACH object's dependencies
+			 * explicitly, since it will not match anything in pg_depend.
+			 *
+			 * Give it dependencies on both the partition table and the parent
+			 * table, so that it will not be executed till both of those
+			 * exist.  (There's no need to care what order those are created
+			 * in.)
+			 */
+			addObjectDependency(&attachinfo->dobj, tblinfo[i].dobj.dumpId);
+			addObjectDependency(&attachinfo->dobj, tblinfo[i].parents[0]->dobj.dumpId);
+		}
 	}
 }
 
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index dc1d41dd8d..1efc642280 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -201,6 +201,7 @@ static void dumpAgg(Archive *fout, AggInfo *agginfo);
 static void dumpTrigger(Archive *fout, TriggerInfo *tginfo);
 static void dumpEventTrigger(Archive *fout, EventTriggerInfo *evtinfo);
 static void dumpTable(Archive *fout, TableInfo *tbinfo);
+static void dumpTableAttach(Archive *fout, TableAttachInfo *tbinfo);
 static void dumpTableSchema(Archive *fout, TableInfo *tbinfo);
 static void dumpAttrDef(Archive *fout, AttrDefInfo *adinfo);
 static void dumpSequence(Archive *fout, TableInfo *tbinfo);
@@ -10110,6 +10111,9 @@ dumpDumpableObject(Archive *fout, DumpableObject *dobj)
 		case DO_TABLE:
 			dumpTable(fout, (TableInfo *) dobj);
 			break;
+		case DO_TABLE_ATTACH:
+			dumpTableAttach(fout, (TableAttachInfo *) dobj);
+			break;
 		case DO_ATTRDEF:
 			dumpAttrDef(fout, (AttrDefInfo *) dobj);
 			break;
@@ -15573,6 +15577,55 @@ createDummyViewAsClause(Archive *fout, TableInfo *tbinfo)
 	return result;
 }
 
+/*
+ * dumpTableAttach
+ *	  write to fout the commands to attach a child partition
+ *
+ * For partitioned tables, emit the ATTACH PARTITION clause.  Note
+ * that we always want to create partitions this way instead of using
+ * CREATE TABLE .. PARTITION OF, mainly to preserve a possible column
+ * layout discrepancy with the parent, but also to ensure it gets the
+ * correct tablespace setting if it differs from the parent's.
+ */
+static void
+dumpTableAttach(Archive *fout, TableAttachInfo *attachinfo)
+{
+	DumpOptions *dopt = fout->dopt;
+	char	   *qualrelname;
+	PQExpBuffer q;
+
+	if (dopt->dataOnly)
+		return;
+
+	if (!(attachinfo->partitionTbl->dobj.dump & DUMP_COMPONENT_DEFINITION))
+		return;
+
+	/* With partitions there can only be one parent */
+	if (attachinfo->partitionTbl->numParents != 1)
+		fatal("invalid number of parents %d for table \"%s\"",
+				attachinfo->partitionTbl->numParents,
+				attachinfo->partitionTbl->dobj.name);
+
+	qualrelname = pg_strdup(fmtQualifiedDumpable(attachinfo));
+	q = createPQExpBuffer();
+
+	/* Perform ALTER TABLE on the parent */
+	appendPQExpBuffer(q,
+					  "ALTER TABLE ONLY %s ATTACH PARTITION %s %s;\n",
+					  fmtQualifiedDumpable(attachinfo->parentTbl),
+					  qualrelname, attachinfo->partitionTbl->partbound);
+
+	ArchiveEntry(fout, attachinfo->dobj.catId, attachinfo->dobj.dumpId,
+				 ARCHIVE_OPTS(.tag = attachinfo->dobj.name,
+							  .namespace = attachinfo->dobj.namespace->dobj.name,
+							  .description = "TABLE ATTACH",
+							  .section = SECTION_PRE_DATA,
+							  .createStmt = q->data));
+
+	free(qualrelname);
+	destroyPQExpBuffer(q);
+}
+
 /*
  * dumpTableSchema
  *	  write the declaration (not data) of one user-defined table or view
@@ -16069,27 +16122,6 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 			}
 		}
 
-		/*
-		 * For partitioned tables, emit the ATTACH PARTITION clause.  Note
-		 * that we always want to create partitions this way instead of using
-		 * CREATE TABLE .. PARTITION OF, mainly to preserve a possible column
-		 * layout discrepancy with the parent, but also to ensure it gets the
-		 * correct tablespace setting if it differs from the parent's.
-		 */
-		if (tbinfo->ispartition)
-		{
-			/* With partitions there can only be one parent */
-			if (tbinfo->numParents != 1)
-				fatal("invalid number of parents %d for table \"%s\"",
-					  tbinfo->numParents, tbinfo->dobj.name);
-
-			/* Perform ALTER TABLE on the parent */
-			appendPQExpBuffer(q,
-							  "ALTER TABLE ONLY %s ATTACH PARTITION %s %s;\n",
-							  fmtQualifiedDumpable(parents[0]),
-							  qualrelname, tbinfo->partbound);
-		}
-
 		/*
 		 * In binary_upgrade mode, arrange to restore the old relfrozenxid and
 		 * relminmxid of all vacuumable relations.  (While vacuum.c processes
@@ -16291,6 +16323,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 								  SECTION_POST_DATA : SECTION_PRE_DATA,
 								  .createStmt = q->data,
 								  .dropStmt = delq->data));
+
 	}
 
 	/* Dump Table Comments */
@@ -18280,6 +18313,7 @@ addBoundaryDependencies(DumpableObject **dobjs, int numObjs,
 			case DO_COLLATION:
 			case DO_CONVERSION:
 			case DO_TABLE:
+			case DO_TABLE_ATTACH:
 			case DO_ATTRDEF:
 			case DO_PROCLANG:
 			case DO_CAST:
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 317bb83970..b9311f6e19 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -50,6 +50,7 @@ typedef enum
 	DO_COLLATION,
 	DO_CONVERSION,
 	DO_TABLE,
+	DO_TABLE_ATTACH,
 	DO_ATTRDEF,
 	DO_INDEX,
 	DO_INDEX_ATTACH,
@@ -337,6 +338,13 @@ typedef struct _tableInfo
 	struct _triggerInfo *triggers;	/* array of TriggerInfo structs */
 } TableInfo;
 
+typedef struct _tableAttachInfo
+{
+	DumpableObject dobj;
+	TableInfo   *parentTbl;		/* link to partitioned table */
+	TableInfo   *partitionTbl;	/* link to partition */
+} TableAttachInfo;
+
 typedef struct _attrDefInfo
 {
 	DumpableObject dobj;		/* note: dobj.name is name of table */
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 654e2ec514..c004cf603d 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -54,36 +54,37 @@ static const int dbObjectTypePriority[] =
 	3,							/* DO_COLLATION */
 	11,							/* DO_CONVERSION */
 	18,							/* DO_TABLE */
-	20,							/* DO_ATTRDEF */
-	28,							/* DO_INDEX */
-	29,							/* DO_INDEX_ATTACH */
-	30,							/* DO_STATSEXT */
-	31,							/* DO_RULE */
-	32,							/* DO_TRIGGER */
-	27,							/* DO_CONSTRAINT */
-	33,							/* DO_FK_CONSTRAINT */
+	19,							/* DO_TABLE_ATTACH */
+	21,							/* DO_ATTRDEF */
+	29,							/* DO_INDEX */
+	30,							/* DO_INDEX_ATTACH */
+	31,							/* DO_STATSEXT */
+	32,							/* DO_RULE */
+	33,							/* DO_TRIGGER */
+	28,							/* DO_CONSTRAINT */
+	34,							/* DO_FK_CONSTRAINT */
 	2,							/* DO_PROCLANG */
 	10,							/* DO_CAST */
-	23,							/* DO_TABLE_DATA */
-	24,							/* DO_SEQUENCE_SET */
-	19,							/* DO_DUMMY_TYPE */
+	24,							/* DO_TABLE_DATA */
+	25,							/* DO_SEQUENCE_SET */
+	21,							/* DO_DUMMY_TYPE */
 	12,							/* DO_TSPARSER */
 	14,							/* DO_TSDICT */
 	13,							/* DO_TSTEMPLATE */
 	15,							/* DO_TSCONFIG */
 	16,							/* DO_FDW */
 	17,							/* DO_FOREIGN_SERVER */
-	38,							/* DO_DEFAULT_ACL --- done in ACL pass */
+	39,							/* DO_DEFAULT_ACL --- done in ACL pass */
 	3,							/* DO_TRANSFORM */
-	21,							/* DO_BLOB */
-	25,							/* DO_BLOB_DATA */
-	22,							/* DO_PRE_DATA_BOUNDARY */
-	26,							/* DO_POST_DATA_BOUNDARY */
-	39,							/* DO_EVENT_TRIGGER --- next to last! */
-	40,							/* DO_REFRESH_MATVIEW --- last! */
-	34,							/* DO_POLICY */
-	35,							/* DO_PUBLICATION */
-	36,							/* DO_PUBLICATION_REL */
+	22,							/* DO_BLOB */
+	26,							/* DO_BLOB_DATA */
+	23,							/* DO_PRE_DATA_BOUNDARY */
+	27,							/* DO_POST_DATA_BOUNDARY */
+	40,							/* DO_EVENT_TRIGGER --- next to last! */
+	41,							/* DO_REFRESH_MATVIEW --- last! */
+	35,							/* DO_POLICY */
+	36,							/* DO_PUBLICATION */
+	37,							/* DO_PUBLICATION_REL */
 	37							/* DO_SUBSCRIPTION */
 };
 
@@ -1275,6 +1276,11 @@ describeDumpableObject(DumpableObject *obj, char *buf, int bufsize)
 					 "TABLE %s  (ID %d OID %u)",
 					 obj->name, obj->dumpId, obj->catId.oid);
 			return;
+		case DO_TABLE_ATTACH:
+			snprintf(buf, bufsize,
+					 "TABLE ATTACH %s  (ID %d)",
+					 obj->name, obj->dumpId);
+			return;
 		case DO_ATTRDEF:
 			snprintf(buf, bufsize,
 					 "ATTRDEF %s.%s  (ID %d OID %u)",
-- 
2.17.0

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Justin Pryzby (#9)
Re: pg_dump, ATTACH, and independently restorable child partitions

Justin Pryzby <pryzby@telsasoft.com> writes:

[ v5-0001-pg_dump-output-separate-object-for-ALTER-TABLE.AT.patch ]

Pushed with mostly cosmetic edits.

One thing I changed that isn't cosmetic is that I set the ArchiveEntry's
owner to be the owner of the child table. Although we aren't going to
do any sort of ALTER OWNER on this, it's still important that the owner
be marked as someone who has the right permissions to issue the ALTER.
The default case is that the original user will issue the ATTACH, which
basically only works if you run the restore as superuser. It looks to
me like you copied this decision from the INDEX ATTACH code, which is
just as broken --- I'm going to go fix/backpatch that momentarily.

Another thing that bothers me still is that it's not real clear that
this code plays nicely with selective dumps, because it's not doing
anything to set the dobj.dump field in a non-default way (which in
turn means that the dobj.dump test in dumpTableAttach can never fire).
It seems like it might be possible to emit a TABLE ATTACH object
even though one or both of the referenced tables didn't get dumped.
In some desultory testing I couldn't get that to actually happen, but
maybe I just didn't push on it in the right way. I'd be happier about
this if we set the flags with something along the lines of

attachinfo->dobj.dump = attachinfo->parentTbl->dobj.dump &
attachinfo->partitionTbl->dobj.dump;

regards, tom lane

#11Justin Pryzby
pryzby@telsasoft.com
In reply to: Tom Lane (#10)
Re: pg_dump, ATTACH, and independently restorable child partitions

On Wed, Nov 25, 2020 at 06:35:19PM -0500, Tom Lane wrote:

What we'd need to do if we want this to work with direct-to-DB restore
is to split off the ATTACH PARTITION command as a separate TOC entry.
That doesn't seem amazingly difficult, and it would even offer the
possibility that you could extract the partition standalone without
having to ignore errors. (You'd use -l/-L to select the CREATE TABLE,
the data, etc, but not the ATTACH object.)

On Mon, Jan 11, 2021 at 09:28:18PM -0500, Tom Lane wrote:

Justin Pryzby <pryzby@telsasoft.com> writes:

[ v5-0001-pg_dump-output-separate-object-for-ALTER-TABLE.AT.patch ]

Pushed with mostly cosmetic edits.

Thanks for pushing (9a4c0e36f).

Should this be included in the release notes ?

It's a user-visible change visible in pg_restore -l. Someone might be
surprised that the attach "object" needs to be included for restore -L to
behave the same as it use to.

--
-- Name: cdrs_2021_08_22; Type: TABLE ATTACH; Schema: child; Owner: telsasoft
--

7949; 1259 1635139558 TABLE child cdrs_2021_08_24 telsasoft
62164; 0 0 TABLE ATTACH child cdrs_2021_08_24 telsasoft
; depends on: 7949

--
Justin