Fix dumping of a materialized view that depends on a table's primary key.

It is possible for a view or materialized view to depend on a table's
primary key, if the view query relies on functional dependency to
abbreviate a GROUP BY list.  This is problematic for pg_dump since we
ordinarily want to dump view definitions in the pre-data section but
indexes in post-data.  pg_dump knows how to deal with this situation for
regular views, by breaking the view's ON SELECT rule apart from the view
proper.  But it had not been taught what to do about materialized views,
and in fact mistakenly dumped them as regular views in such cases, as
seen in bug #9616 from Jesse Denardo.

If we had CREATE OR REPLACE MATERIALIZED VIEW, we could fix this in a
manner analogous to what's done for regular views; but we don't yet,
and we'd not back-patch such a thing into 9.3 anyway.  As a hopefully-
temporary workaround, break the circularity by postponing the matview
into post-data altogether when this case occurs.

Author: tglsfdc

src/bin/pg_dump/pg_dump.c

@@ -4796,6 +4796,8 @@ getTables(Archive *fout, int *numTables)
 			selectDumpableTable(&tblinfo[i]);
 		tblinfo[i].interesting = tblinfo[i].dobj.dump;
 
+		tblinfo[i].postponed_def = false; /* might get set during sort */
+
 		/*
 		 * Read-lock target tables to make sure they aren't DROPPED or altered
 		 * in schema before we get around to dumping them.
@@ -13611,7 +13613,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 			(tbinfo->relkind == RELKIND_VIEW) ? NULL : tbinfo->reltablespace,
 				 tbinfo->rolname,
 			   (strcmp(reltypename, "TABLE") == 0) ? tbinfo->hasoids : false,
-				 reltypename, SECTION_PRE_DATA,
+				 reltypename,
+				 tbinfo->postponed_def ? SECTION_POST_DATA : SECTION_PRE_DATA,
 				 q->data, delq->data, NULL,
 				 NULL, 0,
 				 NULL, NULL);

src/bin/pg_dump/pg_dump.h

@@ -241,7 +241,7 @@ typedef struct _tableInfo
 	char	   *reltablespace;	/* relation tablespace */
 	char	   *reloptions;		/* options specified by WITH (...) */
 	char	   *checkoption;	/* WITH CHECK OPTION */
-	char	   *toast_reloptions;		/* ditto, for the TOAST table */
+	char	   *toast_reloptions;		/* WITH options for the TOAST table */
 	bool		hasindex;		/* does it have any indexes? */
 	bool		hasrules;		/* does it have any rules? */
 	bool		hastriggers;	/* does it have any triggers? */
@@ -254,9 +254,10 @@ typedef struct _tableInfo
 	/* these two are set only if table is a sequence owned by a column: */
 	Oid			owning_tab;		/* OID of table owning sequence */
 	int			owning_col;		/* attr # of column owning sequence */
-	int			relpages;
+	int			relpages;		/* table's size in pages (from pg_class) */
 
 	bool		interesting;	/* true if need to collect more data */
+	bool		postponed_def;	/* matview must be postponed into post-data */
 
 	/*
 	 * These fields are computed only if we decide the table is interesting

src/bin/pg_dump/pg_dump_sort.c

@@ -798,6 +798,7 @@ repairTypeFuncLoop(DumpableObject *typeobj, DumpableObject *funcobj)
  * will be an implicit dependency in the other direction, we need to break
  * the loop.  If there are no other objects in the loop then we can remove
  * the implicit dependency and leave the ON SELECT rule non-separate.
+ * This applies to matviews, as well.
  */
 static void
 repairViewRuleLoop(DumpableObject *viewobj,
@@ -814,7 +815,9 @@ repairViewRuleLoop(DumpableObject *viewobj,
  * Because findLoop() finds shorter cycles before longer ones, it's likely
  * that we will have previously fired repairViewRuleLoop() and removed the
  * rule's dependency on the view.  Put it back to ensure the rule won't be
- * emitted before the view...
+ * emitted before the view.
+ *
+ * Note: this approach does *not* work for matviews, at the moment.
  */
 static void
 repairViewRuleMultiLoop(DumpableObject *viewobj,
@@ -841,6 +844,30 @@ repairViewRuleMultiLoop(DumpableObject *viewobj,
 	addObjectDependency(ruleobj, postDataBoundId);
 }
 
+/*
+ * If a matview is involved in a multi-object loop, we can't currently fix
+ * that by splitting off the rule.	As a stopgap, we try to fix it by
+ * dropping the constraint that the matview be dumped in the pre-data section.
+ * This is sufficient to handle cases where a matview depends on some unique
+ * index, as can happen if it has a GROUP BY for example.
+ *
+ * Note that the "next object" is not necessarily the matview itself;
+ * it could be the matview's rowtype, for example.  We may come through here
+ * several times while removing all the pre-data linkages.
+ */
+static void
+repairMatViewBoundaryMultiLoop(DumpableObject *matviewobj,
+							   DumpableObject *boundaryobj,
+							   DumpableObject *nextobj)
+{
+	TableInfo  *matviewinfo = (TableInfo *) matviewobj;
+
+	/* remove boundary's dependency on object after it in loop */
+	removeObjectDependency(boundaryobj, nextobj->dumpId);
+	/* mark matview as postponed into post-data section */
+	matviewinfo->postponed_def = true;
+}
+
 /*
  * Because we make tables depend on their CHECK constraints, while there
  * will be an automatic dependency in the other direction, we need to break
@@ -956,10 +983,12 @@ repairDependencyLoop(DumpableObject **loop,
 		return;
 	}
 
-	/* View and its ON SELECT rule */
+	/* View (including matview) and its ON SELECT rule */
 	if (nLoop == 2 &&
 		loop[0]->objType == DO_TABLE &&
 		loop[1]->objType == DO_RULE &&
+		(((TableInfo *) loop[0])->relkind == 'v' ||		/* RELKIND_VIEW */
+		 ((TableInfo *) loop[0])->relkind == 'm') &&	/* RELKIND_MATVIEW */
 		((RuleInfo *) loop[1])->ev_type == '1' &&
 		((RuleInfo *) loop[1])->is_instead &&
 		((RuleInfo *) loop[1])->ruletable == (TableInfo *) loop[0])
@@ -970,6 +999,8 @@ repairDependencyLoop(DumpableObject **loop,
 	if (nLoop == 2 &&
 		loop[1]->objType == DO_TABLE &&
 		loop[0]->objType == DO_RULE &&
+		(((TableInfo *) loop[1])->relkind == 'v' ||		/* RELKIND_VIEW */
+		 ((TableInfo *) loop[1])->relkind == 'm') &&	/* RELKIND_MATVIEW */
 		((RuleInfo *) loop[0])->ev_type == '1' &&
 		((RuleInfo *) loop[0])->is_instead &&
 		((RuleInfo *) loop[0])->ruletable == (TableInfo *) loop[1])
@@ -978,12 +1009,13 @@ repairDependencyLoop(DumpableObject **loop,
 		return;
 	}
 
-	/* Indirect loop involving view and ON SELECT rule */
+	/* Indirect loop involving view (but not matview) and ON SELECT rule */
 	if (nLoop > 2)
 	{
 		for (i = 0; i < nLoop; i++)
 		{
-			if (loop[i]->objType == DO_TABLE)
+			if (loop[i]->objType == DO_TABLE &&
+				((TableInfo *) loop[i])->relkind == 'v')		/* RELKIND_VIEW */
 			{
 				for (j = 0; j < nLoop; j++)
 				{
@@ -1000,6 +1032,30 @@ repairDependencyLoop(DumpableObject **loop,
 		}
 	}
 
+	/* Indirect loop involving matview and data boundary */
+	if (nLoop > 2)
+	{
+		for (i = 0; i < nLoop; i++)
+		{
+			if (loop[i]->objType == DO_TABLE &&
+				((TableInfo *) loop[i])->relkind == 'm')		/* RELKIND_MATVIEW */
+			{
+				for (j = 0; j < nLoop; j++)
+				{
+					if (loop[j]->objType == DO_PRE_DATA_BOUNDARY)
+					{
+						DumpableObject *nextobj;
+
+						nextobj = (j < nLoop - 1) ? loop[j + 1] : loop[0];
+						repairMatViewBoundaryMultiLoop(loop[i], loop[j],
+													   nextobj);
+						return;
+					}
+				}
+			}
+		}
+	}
+
 	/* Table and CHECK constraint */
 	if (nLoop == 2 &&
 		loop[0]->objType == DO_TABLE &&