Date: Tue, 20 Apr 2021 20:46:49 -0400
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Amit Langote <amitlangote09@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY
Message-ID: <20210421004649.GA1951@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="7AUc2qLy4jB3hD7Z"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20210420224107.GA11029@alvherre.pgsql>
User-Agent: Mutt/1.10.1 (2018-07-13)
Archived-At: 
 <https://www.postgresql.org/message-id/20210421004649.GA1951%40alvherre.pgsql>
Precedence: bulk


--7AUc2qLy4jB3hD7Z
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Actually I had a silly bug in the version that attempted to cache a
partdesc that omits detached partitions.  This one, while not fully
baked, seems to work correctly (on top of the previous one).

The thing that I don't fully understand is why we have to require to
have built the regular one first.

-- 
Álvaro Herrera                            39°49'30"S 73°17'W
"This is what I like so much about PostgreSQL.  Most of the surprises
are of the "oh wow!  That's cool" Not the "oh shit!" kind.  :)"
Scott Marlowe, http://archives.postgresql.org/pgsql-admin/2008-10/msg00152.php

--7AUc2qLy4jB3hD7Z
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-separately-cache-a-partdesc-that-omits-detached-part.patch"

From b45e9db9cd08282f7a7fabcef944880cc3a3d415 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 20 Apr 2021 20:42:30 -0400
Subject: [PATCH] separately cache a partdesc that omits detached partitions,
 too

---
 src/backend/partitioning/partdesc.c | 21 +++++++++++++++++++--
 src/backend/utils/cache/relcache.c  |  4 ++++
 src/include/utils/rel.h             |  1 +
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/backend/partitioning/partdesc.c b/src/backend/partitioning/partdesc.c
index 7d019a72be..f245cbd664 100644
--- a/src/backend/partitioning/partdesc.c
+++ b/src/backend/partitioning/partdesc.c
@@ -67,10 +67,25 @@ RelationGetPartitionDesc(Relation rel, bool include_detached)
 {
 	Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE);
 
+	/*
+	 * The common case is that there are no partitions pending detach and
+	 * that the appropriate descriptor has already been built; use that.
+	 */
 	if (likely(rel->rd_partdesc &&
 			   (!rel->rd_partdesc->detached_exist || include_detached)))
 		return rel->rd_partdesc;
 
+	/*
+	 * If we're asked for a descriptor with detached partitions omitted, and
+	 * we have already built it, return that.
+	 */
+	if (!include_detached &&
+		rel->rd_partdesc &&
+		rel->rd_partdesc->detached_exist &&
+		rel->rd_partdesc_nodetached)
+		return rel->rd_partdesc_nodetached;
+
+	/* Otherwise build one afresh and store it for next time */
 	return RelationBuildPartitionDesc(rel, include_detached);
 }
 
@@ -278,9 +293,11 @@ RelationBuildPartitionDesc(Relation rel, bool include_detached)
 		MemoryContextSetParent(rel->rd_pdcxt, new_pdcxt);
 	rel->rd_pdcxt = new_pdcxt;
 
-	/* Store it into relcache, but only if no detached partitions exist */
-	if (!detached_exist)
+	/* Store it into the appropriate member of the relcache entry */
+	if (!detached_exist || include_detached)
 		rel->rd_partdesc = partdesc;
+	else
+		rel->rd_partdesc_nodetached = partdesc;
 
 	return partdesc;
 }
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 29702d6eab..87a7613584 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1157,6 +1157,7 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
 	relation->rd_partkey = NULL;
 	relation->rd_partkeycxt = NULL;
 	relation->rd_partdesc = NULL;
+	relation->rd_partdesc_nodetached = NULL;
 	relation->rd_pdcxt = NULL;
 	relation->rd_partcheck = NIL;
 	relation->rd_partcheckvalid = false;
@@ -2672,12 +2673,14 @@ RelationClearRelation(Relation relation, bool rebuild)
 			 * newrel.
 			 */
 			relation->rd_partdesc = NULL;	/* ensure rd_partdesc is invalid */
+			relation->rd_partdesc_nodetached = NULL;
 			if (relation->rd_pdcxt != NULL) /* probably never happens */
 				MemoryContextSetParent(newrel->rd_pdcxt, relation->rd_pdcxt);
 			else
 				relation->rd_pdcxt = newrel->rd_pdcxt;
 			/* drop newrel's pointers so we don't destroy it below */
 			newrel->rd_partdesc = NULL;
+			newrel->rd_partdesc_nodetached = NULL;
 			newrel->rd_pdcxt = NULL;
 		}
 
@@ -5942,6 +5945,7 @@ load_relcache_init_file(bool shared)
 		rel->rd_partkey = NULL;
 		rel->rd_partkeycxt = NULL;
 		rel->rd_partdesc = NULL;
+		rel->rd_partdesc_nodetached = NULL;
 		rel->rd_pdcxt = NULL;
 		rel->rd_partcheck = NIL;
 		rel->rd_partcheckvalid = false;
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 34e25eb597..3d480a3ab2 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -127,6 +127,7 @@ typedef struct RelationData
 
 	/* data managed by RelationGetPartitionDesc: */
 	PartitionDesc rd_partdesc;	/* partition descriptor, or NULL */
+	PartitionDesc rd_partdesc_nodetached;	/* partition descriptor omitting detached partitions */
 	MemoryContext rd_pdcxt;		/* private context for rd_partdesc, if any */
 
 	/* data managed by RelationGetPartitionQual: */
-- 
2.20.1


--7AUc2qLy4jB3hD7Z--