0003-Relieve-RelationGetPartitionDispatchInfo-of-doing-an.patch

text/plain

Filename: 0003-Relieve-RelationGetPartitionDispatchInfo-of-doing-an.patch
Type: text/plain
Part: 2
Message: Re: expanding inheritance in partition bound order

Patch

Format: format-patch
Series: patch 0003
Subject: Relieve RelationGetPartitionDispatchInfo() of doing any locking
File+
src/backend/catalog/partition.c 20 22
src/backend/executor/execMain.c 11 9
src/include/catalog/partition.h 2 2
From 6ae18ec3456b2a3fedd239059687873ae91ddbee Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 9 Aug 2017 15:34:45 +0900
Subject: [PATCH 3/5] Relieve RelationGetPartitionDispatchInfo() of doing any
 locking

Anyone who wants to call RelationGetPartitionDispatchInfo() must first
acquire locks using find_all_inheritors.
---
 src/backend/catalog/partition.c | 42 ++++++++++++++++++++---------------------
 src/backend/executor/execMain.c | 20 +++++++++++---------
 src/include/catalog/partition.h |  4 ++--
 3 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 465e4fc097..4c16bf143b 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -1003,14 +1003,12 @@ get_partition_qual_relid(Oid relid)
  * one member, that is, one for the root partitioned table), *leaf_part_oids
  * contains a list of the OIDs of of all the leaf partitions.
  *
- * Note that we lock only those partitions that are partitioned tables, because
- * we need to look at its relcache entry to get its PartitionKey and its
- * PartitionDesc. It's the caller's responsibility to lock the leaf partitions
- * that will actually be accessed during a given query.
+ * It is assumed that the caller has locked at least all the partitioned tables
+ * in the tree, because we need to look at their relcache entries.
  */
 void
-RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
-								 List **ptinfos, List **leaf_part_oids)
+RelationGetPartitionDispatchInfo(Relation rel, List **ptinfos,
+								 List **leaf_part_oids)
 {
 	List	   *all_parts,
 			   *all_parents;
@@ -1025,16 +1023,10 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
 	 * Starting with the root partitioned table for which we already have the
 	 * relcache entry, we look at its partition descriptor to get the
 	 * partition OIDs.  For partitions that are themselves partitioned tables,
-	 * we get their relcache entries after locking them with lockmode and
-	 * queue their partitions to be looked at later.  Leaf partitions are
-	 * added to the result list without locking.  For each partitioned table,
-	 * we build a PartitionedTableInfo object and add it to the other result
-	 * list.
-	 *
-	 * Since RelationBuildPartitionDescriptor() puts partitions in a canonical
-	 * order determined by comparing partition bounds, we can rely that
-	 * concurrent backends see the partitions in the same order, ensuring that
-	 * there are no deadlocks when locking the partitions.
+	 * we get their relcache entries and queue their partitions to be looked at
+	 * later.  For each leaf partition, we simply add its OID to the result
+	 * list and for each partitioned table, we build a PartitionedTableInfo
+	 * object and add it to the other result list.
 	 */
 	i = offset = 0;
 	*ptinfos = *leaf_part_oids = NIL;
@@ -1057,8 +1049,14 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
 			PartitionedTableInfo   *ptinfo;
 			PartitionDispatch		pd;
 
+			/*
+			 * All the relations in the partition tree must be locked
+			 * by the caller.
+			 *
+			 * XXX - Add RelationLockHeldByMe(partrelid) check here!
+			 */
 			if (partrelid != RelationGetRelid(rel))
-				partrel = heap_open(partrelid, lockmode);
+				partrel = heap_open(partrelid, NoLock);
 			else
 				partrel = rel;
 
@@ -1077,7 +1075,8 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
 			/*
 			 * XXX- do we need a pinning mechanism for partition descriptors
 			 * so that there references can be managed independently of
-			 * the parent relcache entry? Like PinPartitionDesc(partdesc)?
+			 * the fate of parent relcache entry?
+			 * Like PinPartitionDesc(partdesc)?
 			 */
 			pd->partdesc = partdesc;
 
@@ -1141,10 +1140,9 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
 
 			/*
 			 * Release the relation descriptor.  Lock that we have on the
-			 * table will keep the PartitionDesc that is pointing into
-			 * RelationData intact, a pointer to which hope to keep
-			 * through this transaction's commit.
-			 * (XXX - how true is that?)
+			 * table will keep PartitionDesc (that is pointing into
+			 * RelationData) intact, a reference to which want to keep through
+			 * this transaction's commit. (XXX - how true is that?)
 			 */
 			if (partrel != rel)
 				heap_close(partrel, NoLock);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 0379e489d9..3dd620fc8a 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -43,6 +43,7 @@
 #include "access/xact.h"
 #include "catalog/namespace.h"
 #include "catalog/partition.h"
+#include "catalog/pg_inherits_fn.h"
 #include "catalog/pg_publication.h"
 #include "commands/matview.h"
 #include "commands/trigger.h"
@@ -3250,14 +3251,16 @@ ExecSetupPartitionTupleRouting(Relation rel,
 	int			i;
 	ResultRelInfo *leaf_part_rri;
 	Relation	parent;
+	List	   *all_parts;
 
 	/*
-	 * Get information about the partition tree.  All the partitioned
-	 * tables in the tree are locked, but not the leaf partitions.  We
-	 * lock them while building their ResultRelInfos below.
+	 * Get information about the partition tree.  First lock all the
+	 * partitions using find_all_inheritors().
 	 */
-	RelationGetPartitionDispatchInfo(rel, RowExclusiveLock,
-									 &ptinfos, &leaf_parts);
+	all_parts = find_all_inheritors(RelationGetRelid(rel), RowExclusiveLock,
+									NULL, NULL);
+	list_free(all_parts);
+	RelationGetPartitionDispatchInfo(rel, &ptinfos, &leaf_parts);
 
 	/*
 	 * The ptinfos list contains PartitionedTableInfo objects for all the
@@ -3396,11 +3399,10 @@ ExecSetupPartitionTupleRouting(Relation rel,
 		TupleDesc	part_tupdesc;
 
 		/*
-		 * RelationGetPartitionDispatchInfo didn't lock the leaf partitions,
-		 * so lock here.  Note that each of the relations in *partitions are
-		 * eventually closed (when the plan is shut down, for instance).
+		 * Note that each of the relations in *partitions are eventually
+		 * closed (when the plan is shut down, for instance).
 		 */
-		partrel = heap_open(lfirst_oid(cell), RowExclusiveLock);
+		partrel = heap_open(lfirst_oid(cell), NoLock);	/* already locked */
 		part_tupdesc = RelationGetDescr(partrel);
 
 		/*
diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h
index 6a0c81b3bd..9e63020c82 100644
--- a/src/include/catalog/partition.h
+++ b/src/include/catalog/partition.h
@@ -72,8 +72,8 @@ extern List *map_partition_varattnos(List *expr, int target_varno,
 extern List *RelationGetPartitionQual(Relation rel);
 extern Expr *get_partition_qual_relid(Oid relid);
 
-extern void RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
-								 List **ptinfos, List **leaf_part_oids);
+extern void RelationGetPartitionDispatchInfo(Relation rel, List **ptinfos,
+								 List **leaf_part_oids);
 
 /* For tuple routing */
 extern void FormPartitionKeyDatum(PartitionKeyInfo *keyinfo,
-- 
2.11.0