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

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

From
PG Bug reporting form
Date:
The following bug has been logged on the website:

Bug reference:      15602
Logged by:          Tom Cassidy
Email address:      tcassidy@mossridge.com.au
PostgreSQL version: 11.1
Operating system:   Devuan 2.0 ASCII
Description:

Operating System: Devuan 2.0 ASCII (derivative of Debian 9/stretch)
Hardware: KVM Virtual Machine
PostgreSQL version: PostgreSQL 11.1 (Debian 11.1-1.pgdg90+1) on
x86_64-pc-linux-gnu, compiled by gcc (Debian 6.3.0-18+deb9u1) 6.3.0
20170516, 64-bit
pg_dump version: pg_dump (PostgreSQL) 11.1 (Debian 11.1-1.pgdg90+1)
Kernel: Linux 4.9.0-8-amd64
System Memory: 1GB
C library: libc6 (2.24-11+deb9u3)
Installation: following the quickstart instrunctions on
https://wiki.postgresql.org/wiki/Apt, installing the postgresql-11 package
from apt.postgresql.org for Debian 9/stretch


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:

postgres@devuan:/tmp$ cat test_case.sql 
CREATE DATABASE test_mv TEMPLATE template0;

\c test_mv

CREATE TABLE table1
(
    id bigint NOT NULL,
    name text NOT NULL,
    CONSTRAINT table1_pkey PRIMARY KEY (id)
);

CREATE TABLE table2
(
    id bigint NOT NULL,
    CONSTRAINT table2_pkey PRIMARY KEY (id)
);

CREATE MATERIALIZED VIEW mv_2
AS
 SELECT table1.name,
    table1.id
   FROM table1
  GROUP BY table1.id;

CREATE MATERIALIZED VIEW mv_1
AS
 SELECT z.name,
    a.id AS a_id,
    z.id AS z_id
   FROM table1 a
     JOIN ( SELECT mv_2.name,
            mv_2.id
           FROM mv_2) z ON z.id = a.id;
postgres@devuan:/tmp$


I have tested this on a new installation of PostgreSQL 11.1 using the
following commands:

postgres@devuan:/tmp$ psql -f test_case.sql
postgres@devuan:/tmp$ pg_dump test_mv > test_mv.sql
pg_dump: [archiver] WARNING: archive items not in correct section order
postgres@devuan:/tmp$


Verbose pg_dump:

postgres@devuan:/tmp$ pg_dump --verbose test_mv > test_mv.sql
pg_dump: last built-in OID is 16383
pg_dump: reading extensions
pg_dump: identifying extension members
pg_dump: reading schemas
pg_dump: reading user-defined tables
pg_dump: reading user-defined functions
pg_dump: reading user-defined types
pg_dump: reading procedural languages
pg_dump: reading user-defined aggregate functions
pg_dump: reading user-defined operators
pg_dump: reading user-defined access methods
pg_dump: reading user-defined operator classes
pg_dump: reading user-defined operator families
pg_dump: reading user-defined text search parsers
pg_dump: reading user-defined text search templates
pg_dump: reading user-defined text search dictionaries
pg_dump: reading user-defined text search configurations
pg_dump: reading user-defined foreign-data wrappers
pg_dump: reading user-defined foreign servers
pg_dump: reading default privileges
pg_dump: reading user-defined collations
pg_dump: reading user-defined conversions
pg_dump: reading type casts
pg_dump: reading transforms
pg_dump: reading table inheritance information
pg_dump: reading event triggers
pg_dump: finding extension tables
pg_dump: finding inheritance relationships
pg_dump: reading column info for interesting tables
pg_dump: finding the columns and types of table "public.table1"
pg_dump: finding the columns and types of table "public.table2"
pg_dump: finding the columns and types of table "public.mv_2"
pg_dump: finding the columns and types of table "public.mv_1"
pg_dump: flagging inherited columns in subtables
pg_dump: reading indexes
pg_dump: reading indexes for table "public.table1"
pg_dump: reading indexes for table "public.table2"
pg_dump: flagging indexes in partitioned tables
pg_dump: reading extended statistics
pg_dump: reading constraints
pg_dump: reading triggers
pg_dump: reading rewrite rules
pg_dump: reading policies
pg_dump: reading row security enabled for table "public.table1"
pg_dump: reading policies for table "public.table1"
pg_dump: reading row security enabled for table "public.table2"
pg_dump: reading policies for table "public.table2"
pg_dump: reading row security enabled for table "public.mv_2"
pg_dump: reading policies for table "public.mv_2"
pg_dump: reading row security enabled for table "public.mv_1"
pg_dump: reading policies for table "public.mv_1"
pg_dump: reading publications
pg_dump: reading publication membership
pg_dump: reading publication membership for table "public.table1"
pg_dump: reading publication membership for table "public.table2"
pg_dump: reading subscriptions
pg_dump: reading large objects
pg_dump: reading dependency data
pg_dump: saving encoding = UTF8
pg_dump: saving standard_conforming_strings = on
pg_dump: saving search_path = 
pg_dump: [archiver] WARNING: archive items not in correct section order
pg_dump: creating TABLE "public.table1"
pg_dump: creating TABLE "public.table2"
pg_dump: processing data for table "public.table1"
pg_dump: dumping contents of table "public.table1"
pg_dump: processing data for table "public.table2"
pg_dump: dumping contents of table "public.table2"
pg_dump: creating CONSTRAINT "public.table1 table1_pkey"
pg_dump: creating MATERIALIZED VIEW "public.mv_2"
pg_dump: creating MATERIALIZED VIEW "public.mv_1"
pg_dump: creating CONSTRAINT "public.table2 table2_pkey"
pg_dump: creating MATERIALIZED VIEW DATA "public.mv_2"
pg_dump: creating MATERIALIZED VIEW DATA "public.mv_1"
postgres@devuan:/tmp$


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

From
Alvaro Herrera
Date:
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.

repairDependencyLoop sees this curious case involving ten objects:

repairing loop: 10 (mv_2 [type 12], _RETURN [type 17] (on mv_2), table1_pkey [type 19], POST-DATA BOUNDARY [type 37],
foo[type 23], PRE-DATA BOUNDARY [type 36], _mv_1 [type 25], mv_1 [type 25], mv_1 [type 12], _RETURN [type 17] (on
mv_1))

(I just patched it to print the loop objects as attached)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

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

From
Tom Lane
Date:
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;
                     }
                 }

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

From
Tom Cassidy
Date:
On 5/2/19 4:24 am, Tom Lane wrote:
> 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

Hi Tom,

I downloaded the git source and built postgres from REL_11_STABLE branch
using the instructions at
https://blog.2ndquadrant.com/testing-new-postgresql-versions-without-messing-up-your-install/.
  Your patch looked like it was already applied to this branch so I
didn't apply it.

Running pg_dump from the patched version I didn't notice any warning
output on either the original database or the simplified test case
database, so it appears that this patch solves the issue.

I ran the pg_dump as plain-text SQL output format, and there were no
differences in the generated SQL between the old pg_dump and the patched
pg_dump, except for the pg_dump version string.

Regards,
Tom


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

From
Tom Lane
Date:
Tom Cassidy <tcassidy@mossridge.com.au> writes:
> On 5/2/19 4:24 am, Tom Lane wrote:
>> 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?

> I downloaded the git source and built postgres from REL_11_STABLE branch
> using the instructions at
> https://blog.2ndquadrant.com/testing-new-postgresql-versions-without-messing-up-your-install/.
>   Your patch looked like it was already applied to this branch so I
> didn't apply it.

Yeah, I committed it a couple hours ago --- "git log" would confirm
whether you had it or not.

> Running pg_dump from the patched version I didn't notice any warning
> output on either the original database or the simplified test case
> database, so it appears that this patch solves the issue.

Cool, thanks for checking!

            regards, tom lane