Re: pg_dump --if-exists --clean when drop index that is partition of a partitioned index - Mailing list pgsql-hackers

From Tom Lane
Subject Re: pg_dump --if-exists --clean when drop index that is partition of a partitioned index
Date
Msg-id 1795813.1744757515@sss.pgh.pa.us
Whole thread Raw
In response to Re: pg_dump --if-exists --clean when drop index that is partition of a partitioned index  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: pg_dump --if-exists --clean when drop index that is partition of a partitioned index
List pgsql-hackers
After thinking about this a bit more: the problem is basically one
of things being done in the wrong order.  And the way to ensure
that things get done in the right order is to set up the object
dependencies correctly.  So what we need to do is to switch the
dependency direction between the child and parent index objects,
as attached.

The effect of this is to switch both the creation order and the
drop order of the child and parent indexes.  Normally, flipping
the creation order would be problematic.  But for partitioned
indexes it doesn't matter because there's not actually any
connection between the child and parent indexes until we issue
INDEX ATTACH.  The INDEX ATTACH object has dependencies on both,
so it doesn't come out till after both, so all that still works.

However ... this is still only a partial fix.  It fixes the
problem as-presented, where you used both --clean and --if-exists.
But if you just say --clean, then the DROP command for the child
index fails because it's already gone thanks to dropping the
parent index first.  Possibly the least messy way to deal with
that is to always put IF EXISTS into the DROP command for a
child index.  This isn't totally problem-free, because then we
would have to teach RestoreArchive to not insert IF EXISTS if
one is already there, and that probably makes the fix not
back-patchable for fear of compatibility problems with unpatched
copies of pg_restore.

Or we could do what Jian suggested and just not emit any dropStmt
for child indexes.  I continue to fear that that will have
undesirable side-effects, but I have to admit that I'm not sure
what.  The fact that the backend will automatically drop the
child index if we drop either its parent index or its table
seems to cover a lot of the edge cases ... but does it cover
all of them?

            regards, tom lane

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index c6e6d3b2b86..961b8f05eef 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -19630,13 +19630,25 @@ getDependencies(Archive *fout)
          * Ordinarily, table rowtypes have implicit dependencies on their
          * tables.  However, for a composite type the implicit dependency goes
          * the other way in pg_depend; which is the right thing for DROP but
-         * it doesn't produce the dependency ordering we need. So in that one
+         * it doesn't produce the dependency ordering we need. So in that
          * case, we reverse the direction of the dependency.
+         *
+         * Similarly, the server's idea of the dependency direction for
+         * partitioned indexes is backwards for our purposes, so reverse it.
+         * This swap has no real effect during the object creation sequence,
+         * because the leaf and partitioned indexes are not connected until we
+         * issue INDEX ATTACH.  What it does for us is to ensure that when
+         * --clean mode is used, the DROP INDEX commands come out in an order
+         * that the server will accept.
          */
         if (deptype == 'i' &&
             dobj->objType == DO_TABLE &&
             refdobj->objType == DO_TYPE)
             addObjectDependency(refdobj, dobj->dumpId);
+        else if (deptype == 'P' &&
+                 dobj->objType == DO_INDEX &&
+                 refdobj->objType == DO_INDEX)
+            addObjectDependency(refdobj, dobj->dumpId);
         else
             /* normal case */
             addObjectDependency(dobj, refdobj->dumpId);

pgsql-hackers by date:

Previous
From: Nikolay Samokhvalov
Date:
Subject: Re: Built-in Raft replication
Next
From: Jeff Davis
Date:
Subject: Re: [18] Unintentional behavior change in commit e9931bfb75