agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: amit <amitlangote09@gmail.com>
Subject: [PATCH 2/3] Fix pg_dump to handle partition inheritance sanely
Date: Wed, 12 Apr 2017 14:53:09 +0900
With the current coding, partitions completely miss the flagInhAttr()
treatment, which results in partition's attributes to be unnecessarily
emitted.
Update 002_pg_dump.pl to modify the expected output.
---
src/bin/pg_dump/common.c | 19 ++++++++++++++-----
src/bin/pg_dump/pg_dump.c | 36 ++++++++++++++++++++++++++----------
src/bin/pg_dump/pg_dump.h | 5 +++--
src/bin/pg_dump/t/002_pg_dump.pl | 2 --
4 files changed, 43 insertions(+), 19 deletions(-)
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index e2bc3576dc..8009b7c1e7 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -381,12 +381,15 @@ flagPartitions(TableInfo *tblinfo, int numTables,
/* Find the parent TableInfo and save */
findPartitionParentByOid(&tblinfo[i], partinfo, numPartitions);
- /* Mark the parent as interesting for getTableAttrs */
- if (tblinfo[i].partitionOf)
+ /*
+ * If a partition indeed, mark the only parent as interesting for
+ * getTableAttrs.
+ */
+ if (tblinfo[i].partitiondef)
{
- tblinfo[i].partitionOf->interesting = true;
+ tblinfo[i].parents[0]->interesting = true;
addObjectDependency(&tblinfo[i].dobj,
- tblinfo[i].partitionOf->dobj.dumpId);
+ tblinfo[i].parents[0]->dobj.dumpId);
}
}
}
@@ -1008,6 +1011,12 @@ findPartitionParentByOid(TableInfo *self, PartInfo *partinfo,
{
TableInfo *parent;
+ /* Alright, self is a partition */
+ self->ispartition = true;
+
+ /* Partitions have exactly one parent. */
+ self->numParents = 1;
+ self->parents = (TableInfo **) pg_malloc(sizeof(TableInfo *));
parent = findTableByOid(partinfo[i].partparent);
if (parent == NULL)
{
@@ -1017,7 +1026,7 @@ findPartitionParentByOid(TableInfo *self, PartInfo *partinfo,
oid);
exit_nicely(1);
}
- self->partitionOf = parent;
+ self->parents[0] = parent;
/* While we're at it, also save the partdef */
self->partitiondef = partinfo[i].partdef;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index e9b5c8a448..2917db89e5 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -15196,9 +15196,15 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
if (tbinfo->reloftype && !dopt->binary_upgrade)
appendPQExpBuffer(q, " OF %s", tbinfo->reloftype);
- if (tbinfo->partitionOf && !dopt->binary_upgrade)
+ /*
+ * If the table is a partition, dump it as such; except in the case
+ * of a binary upgrade, we dump the table normally and attach it to
+ * the parent afterward.
+ */
+ if (tbinfo->ispartition && !dopt->binary_upgrade)
{
- TableInfo *parentRel = tbinfo->partitionOf;
+ /* Unlike the INHERITS case, only one parent here. */
+ TableInfo *parentRel = tbinfo->parents[0];
appendPQExpBuffer(q, " PARTITION OF ");
if (parentRel->dobj.namespace != tbinfo->dobj.namespace)
@@ -15239,7 +15245,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* Skip column if fully defined by reloftype or the
* partition parent.
*/
- if ((tbinfo->reloftype || tbinfo->partitionOf) &&
+ if ((tbinfo->reloftype || tbinfo->ispartition) &&
!has_default && !has_notnull && !dopt->binary_upgrade)
continue;
@@ -15267,8 +15273,13 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
continue;
}
- /* Attribute type */
- if ((tbinfo->reloftype || tbinfo->partitionOf) &&
+ /*
+ * Attribute type. If this is not a binary upgrade dump,
+ * we need not dump the type for typed tables and
+ * partitions. Because in that case, we are merely
+ * dumping column's options, not defining a new column.
+ */
+ if ((tbinfo->reloftype || tbinfo->ispartition) &&
!dopt->binary_upgrade)
{
appendPQExpBufferStr(q, " WITH OPTIONS");
@@ -15327,7 +15338,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
if (actual_atts)
appendPQExpBufferStr(q, "\n)");
- else if (!((tbinfo->reloftype || tbinfo->partitionOf) &&
+ else if (!((tbinfo->reloftype || tbinfo->ispartition) &&
!dopt->binary_upgrade))
{
/*
@@ -15337,13 +15348,15 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
appendPQExpBufferStr(q, " (\n)");
}
- if (tbinfo->partitiondef && !dopt->binary_upgrade)
+ if (tbinfo->ispartition && !dopt->binary_upgrade)
{
appendPQExpBufferStr(q, "\n");
appendPQExpBufferStr(q, tbinfo->partitiondef);
}
- if (numParents > 0 && !dopt->binary_upgrade)
+ /* Emit the INHERITS clause unless this is a partition. */
+ if (numParents > 0 && !tbinfo->ispartition &&
+ !dopt->binary_upgrade)
{
appendPQExpBufferStr(q, "\nINHERITS (");
for (k = 0; k < numParents; k++)
@@ -15419,6 +15432,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
* using an INHERITS clause --- the latter would possibly mess up the
* column order. That also means we have to take care about setting
* attislocal correctly, plus fix up any inherited CHECK constraints.
+ * The same applies to partitions, so we set up partitions using
+ * ALTER TABLE / ATTACH PARTITION instead of PARTITION OF.
* Analogously, we set up typed tables using ALTER TABLE / OF here.
*/
if (dopt->binary_upgrade &&
@@ -15512,11 +15527,12 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
tbinfo->reloftype);
}
- if (tbinfo->partitionOf)
+ if (tbinfo->ispartition)
{
appendPQExpBufferStr(q, "\n-- For binary upgrade, set up partitions this way.\n");
+ /* Note that there is only one parent in this case. */
appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
- fmtId(tbinfo->partitionOf->dobj.name));
+ fmtId(tbinfo->parents[0]->dobj.name));
appendPQExpBuffer(q, "ATTACH PARTITION %s %s;\n",
fmtId(tbinfo->dobj.name),
tbinfo->partitiondef);
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 471cfce92a..658f736478 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -295,6 +295,8 @@ typedef struct _tableInfo
bool dummy_view; /* view's real definition must be postponed */
bool postponed_def; /* matview must be postponed into post-data */
+ bool ispartition; /* is table a partition? */
+
/*
* These fields are computed only if we decide the table is interesting
* (it's either a table to dump, or a direct parent of a dumpable table).
@@ -329,8 +331,7 @@ typedef struct _tableInfo
struct _tableDataInfo *dataObj; /* TableDataInfo, if dumping its data */
int numTriggers; /* number of triggers for table */
struct _triggerInfo *triggers; /* array of TriggerInfo structs */
- struct _tableInfo *partitionOf; /* TableInfo for the partition parent */
- char *partitiondef; /* partition key definition */
+ char *partitiondef; /* partition bound definition */
} TableInfo;
typedef struct _attrDefInfo
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index bebb2f4f5d..7a003c7286 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4768,8 +4768,6 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
\s+\QCONSTRAINT measurement_y2006m2_unitsales_check CHECK ((unitsales >= 0))\E\n
\)\n
\QFOR VALUES FROM ('2006-02-01') TO ('2006-03-01');\E\n
- \QALTER TABLE ONLY measurement_y2006m2 ALTER COLUMN city_id SET NOT NULL;\E\n
- \QALTER TABLE ONLY measurement_y2006m2 ALTER COLUMN logdate SET NOT NULL;\E\n
/xm,
like => {
clean => 1,
--
2.11.0
--------------0DD22B2EDE2CB9977D4EBB9F
Content-Type: text/x-diff;
name="0003-Do-not-emit-WITH-OPTIONS-for-partition-s-columns.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0003-Do-not-emit-WITH-OPTIONS-for-partition-s-columns.patch"
view thread (3+ messages) latest in thread
Message-ID: <no-message-id-790267@localhost>
Permalink: ../../no-message-id-790267@localhost/
Also on: postgresql.org/message-id/no-message-id-790267@localhost
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: amitlangote09@gmail.com
Subject: Re: [PATCH 2/3] Fix pg_dump to handle partition inheritance sanely
In-Reply-To: <no-message-id-790267@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox