Subject: [PATCH 2/3] Fix pg_dump to handle partition inheritance sanely

From: amit <[email-masked]>
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 5bd4af22c9..7560e528d2 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -15141,9 +15141,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)
@@ -15184,7 +15190,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;
 
@@ -15212,8 +15218,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");
@@ -15272,7 +15283,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))
 			{
 				/*
@@ -15282,13 +15293,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++)
@@ -15364,6 +15377,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 &&
@@ -15457,11 +15472,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 61097e6d99..10c3e109f5 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 cccad04ab6..d01b1d25a2 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -4686,8 +4686,6 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL WITH FUNCTION pg_catalog
 			\Q--\E\n\n
 			\QCREATE TABLE measurement_y2006m2 PARTITION OF dump_test.measurement\E\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

--------------1EB207AE8AA19271D50234A3
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"