Re: BUG #15602: pg_dump archive items not in correct section order

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2019-Jan-22, PG Bug reporting form wrote:
>> When I run pg_dump on a database, it reports the following warning:
>> pg_dump: [archiver] WARNING: archive items not in correct section order
>> 
>> The following SQL script replicates the warning when run on a new instance
>> of PostgreSQL 11.1:

> It does indeed ... curious.

Hmph.  What this shows is that commit 62215de29 was a few bricks shy
of a load.  It got rid of this symptom for the case of a matview that's
dependent on a table's primary key (meaning we have to postpone the
matview creation into the post-data section of the archive), but I did
not think about additional matviews that are dependent on the one with
the circularity problem.  They all have to get postponed, and the
dependency logic correctly fixes that ... but it doesn't mark all
of them as "postponed_def", so that they don't get labeled with
SECTION_POST_DATA and then ProcessArchiveRestoreOptions thinks
something is wrong.

Basically this is sloppy thinking in repairMatViewBoundaryMultiLoop:
the thing that's getting moved to post-data is whatever we just
removed the pre-data dependency for.  The attached seems to be
enough to fix it.

Tom, are you in a position to rebuild pg_dump with this fix applied,
and see if it cures your original case as well as the simplified one?

            regards, tom lane

PS: It's surprising that it took this long for anyone to notice.

PPS: We really ought to get off our duffs and invent CREATE OR REPLACE
for matviews, so that these loops can be fixed in a less unprincipled
fashion.

diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 08005c5..bb128c8 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -755,19 +755,26 @@ repairViewRuleMultiLoop(DumpableObject *viewobj,
  *
  * 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.
+ * several times while removing all the pre-data linkages.  In particular,
+ * if there are other matviews that depend on the one with the circularity
+ * problem, we'll come through here for each such matview and mark them all
+ * as postponed.  (This works because all MVs have pre-data dependencies
+ * to begin with, so each of them will get visited.)
  */
 static void
-repairMatViewBoundaryMultiLoop(DumpableObject *matviewobj,
-                               DumpableObject *boundaryobj,
+repairMatViewBoundaryMultiLoop(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;
+    /* if that object is a matview, mark it as postponed into post-data */
+    if (nextobj->objType == DO_TABLE)
+    {
+        TableInfo  *nextinfo = (TableInfo *) nextobj;
+
+        if (nextinfo->relkind == RELKIND_MATVIEW)
+            nextinfo->postponed_def = true;
+    }
 }
 
 /*
@@ -956,8 +963,7 @@ repairDependencyLoop(DumpableObject **loop,
                         DumpableObject *nextobj;
 
                         nextobj = (j < nLoop - 1) ? loop[j + 1] : loop[0];
-                        repairMatViewBoundaryMultiLoop(loop[i], loop[j],
-                                                       nextobj);
+                        repairMatViewBoundaryMultiLoop(loop[j], nextobj);
                         return;
                     }
                 }