pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Justin Pryzby <pryzby@telsasoft.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: David Rowley <drowley@postgresql.org>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: pg_dump, ATTACH, and independently restorable child partitions
Date: Fri, 4 Dec 2020 12:02:31 -0600
Message-ID: <20201204180231.GD24052@telsasoft.com> (raw )
In-Reply-To: <2168110.1607101985@sss.pgh.pa.us >
References: <20201024195949.GL9241@telsasoft.com >
<20201107021835.GA13348@alvherre.pgsql >
<20201120152055.GF24784@telsasoft.com >
<577158.1606347319@sss.pgh.pa.us >
<20201202225013.GJ24052@telsasoft.com >
<2168110.1607101985@sss.pgh.pa.us >
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.patch (text/x-diff, 10.1K) download
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
view thread (11+ messages) latest in thread
Message-ID: <20201204180231.GD24052@telsasoft.com>
Permalink: ../20201204180231.GD24052@telsasoft.com/
Also on: postgresql.org/message-id/20201204180231.GD24052@telsasoft.com
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, tgl@sss.pgh.pa.us, alvherre@alvh.no-ip.org, drowley@postgresql.org, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_dump, ATTACH, and independently restorable child partitions
In-Reply-To: <20201204180231.GD24052@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox