0002-Fix-lock-upgrade-deadlock-hazard-in-ATExecAttachPart.patch

text/plain

Filename: 0002-Fix-lock-upgrade-deadlock-hazard-in-ATExecAttachPart.patch
Type: text/plain
Part: 1
Message: Re: A bug in mapping attributes in ATExecAttachPartition()

Patch

Format: format-patch
Series: patch 0002
Subject: Fix lock-upgrade deadlock hazard in ATExecAttachPartition
File+
src/backend/commands/tablecmds.c 13 10
From 925b450cc69e8ccdbb191cb6b6d29f97aad2945f Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Tue, 1 Aug 2017 10:31:00 +0900
Subject: [PATCH 2/4] Fix lock-upgrade deadlock hazard in ATExecAttachPartition

Currently, we needless call find_all_inheritors twice to get the
partitions of the table being attached, with a share lock request
during the first call and exclusive lock in the second.

Fix to call find_all_inheritors() only once and request exclusive
lock on children.  We need the exclusive lock, because might have
to scan the individual partitions to validate the partition
constraint being added.
---
 src/backend/commands/tablecmds.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1307dc5893..1d879fa61a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13489,9 +13489,20 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 	/*
 	 * Prevent circularity by seeing if rel is a partition of attachRel. (In
 	 * particular, this disallows making a rel a partition of itself.)
+	 *
+	 * We do that by checking if rel is a member of the list of attachRel's
+	 * partitions provided the latter is partitioned at all.  We want to avoid
+	 * having to construct this list again, so we request the strongest lock
+	 * on all partitions.  We need the strongest lock, because we may decide
+	 * to scan them if we find out that the table being attached (or its leaf
+	 * partitions) may contain rows that violate the partition constraint.
+	 * If the table has a constraint that would prevent such rows, which by
+	 * definition is present in all the partitions, we need not scan the
+	 * table, nor its partitions.  But we cannot risk a deadlock by taking a
+	 * weaker lock now and the stronger one only when needed.
 	 */
 	attachRel_children = find_all_inheritors(RelationGetRelid(attachRel),
-											 AccessShareLock, NULL);
+											 AccessExclusiveLock, NULL);
 	if (list_member_oid(attachRel_children, RelationGetRelid(rel)))
 		ereport(ERROR,
 				(errcode(ERRCODE_DUPLICATE_TABLE),
@@ -13694,17 +13705,9 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
 	else
 	{
 		/* Constraints proved insufficient, so we need to scan the table. */
-		List	   *all_parts;
 		ListCell   *lc;
 
-		/* Take an exclusive lock on the partitions to be checked */
-		if (attachRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-			all_parts = find_all_inheritors(RelationGetRelid(attachRel),
-											AccessExclusiveLock, NULL);
-		else
-			all_parts = list_make1_oid(RelationGetRelid(attachRel));
-
-		foreach(lc, all_parts)
+		foreach(lc, attachRel_children)
 		{
 			AlteredTableInfo *tab;
 			Oid			part_relid = lfirst_oid(lc);
-- 
2.11.0