Date: Wed, 12 Aug 2020 17:49:18 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Dependencies for partitioned indexes are still a mess
Message-ID: <20200812214918.GA30353@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="BOKacYhQ+x31HxR3"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <3170626.1594842723@sss.pgh.pa.us>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


--BOKacYhQ+x31HxR3
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2020-Jul-15, Tom Lane wrote:

> Issue #1: "--clean" does not work
> 
> 1. createdb r2
> 2. pg_restore -d r2 regression.dump
> 3. pg_restore --clean -d r2 regression.dump
> 
> pg_restore: while PROCESSING TOC:
> pg_restore: from TOC entry 6606; 1259 35458 INDEX idxpart32_a_idx postgres
> pg_restore: error: could not execute query: ERROR:  cannot drop index public.idxpart32_a_idx because index public.idxpart3_a_idx requires it
> HINT:  You can drop index public.idxpart3_a_idx instead.
> Command was: DROP INDEX public.idxpart32_a_idx;

I think this problem is just that we're trying to drop a partition index
that's not droppable.  This seems fixed with just leaving the dropStmt
empty, as in the attached.

One issue is that if you previously restored only that particular
partition and its indexes, but not the ATTACH command that would make it
dependent on the parent index, there would not be a DROP command to get
rid of it.  Do we need to be concerned about that case?  I'm inclined to
think not.

> (There seem to be some other problems as well, but most of the 54 complaints
> are related to partitioned indexes/constraints.)

In my run of it there's a good dozen remaining problems, all alike: we
do DROP TYPE widget CASCADE (which works) followed by DROP FUNCTION
public.widget_out(widget), which fails complaining that type widget
doesn't exist.  But in reality the error is innocuous, since that
function was dropped by the DROP TYPE CASCADE anyway.  You could say
that the same thing is happening with these noisy DROP INDEX of index
partitions: the complaints are right in that each partition's DROP INDEX
command doesn't actually work, but the indexes are dropped later anyway,
so the effect is the same.

> Issue #2: parallel restore does not work

Looking.

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

--BOKacYhQ+x31HxR3
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-Don-t-dump-DROP-stmts-for-index-partitions.patch"

From 8334445705c53bb0abff407ebb92ac67975a5898 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 12 Aug 2020 17:36:37 -0400
Subject: [PATCH] Don't dump DROP stmts for index partitions

They would fail to run in --clean mode anyway
---
 src/bin/pg_dump/pg_dump.c | 14 +++++++++-----
 src/bin/pg_dump/pg_dump.h |  2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 9c8436dde6..42391b0b2c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -16416,7 +16416,8 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
 							  qindxname);
 		}
 
-		appendPQExpBuffer(delq, "DROP INDEX %s;\n", qqindxname);
+		if (indxinfo->parentidx == InvalidOid)
+			appendPQExpBuffer(delq, "DROP INDEX %s;\n", qqindxname);
 
 		if (indxinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 			ArchiveEntry(fout, indxinfo->dobj.catId, indxinfo->dobj.dumpId,
@@ -16695,10 +16696,13 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 									"pg_catalog.pg_class", "INDEX",
 									fmtQualifiedDumpable(indxinfo));
 
-		appendPQExpBuffer(delq, "ALTER %sTABLE ONLY %s ", foreign,
-						  fmtQualifiedDumpable(tbinfo));
-		appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n",
-						  fmtId(coninfo->dobj.name));
+		if (indxinfo->parentidx == InvalidOid)
+		{
+			appendPQExpBuffer(delq, "ALTER %sTABLE ONLY %s ", foreign,
+							  fmtQualifiedDumpable(tbinfo));
+			appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n",
+							  fmtId(coninfo->dobj.name));
+		}
 
 		tag = psprintf("%s %s", tbinfo->dobj.name, coninfo->dobj.name);
 
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index da97b731b1..2f051b83d9 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -368,7 +368,7 @@ typedef struct _indxInfo
 								 * contains both key and nonkey attributes */
 	bool		indisclustered;
 	bool		indisreplident;
-	Oid			parentidx;		/* if partitioned, parent index OID */
+	Oid			parentidx;		/* if a partition, parent index OID */
 	SimplePtrList partattaches; /* if partitioned, partition attach objects */
 
 	/* if there is an associated constraint object, its dumpId: */
-- 
2.20.1


--BOKacYhQ+x31HxR3--