On Wed, Nov 25, 2020 at 06:35:19PM -0500, Tom Lane wrote:
> Justin Pryzby <pry...@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
>From 0d0b013930199158306fd295eebbf36c4c4cb5b4 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@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
