From 972b6ec20bf090a18145624f8092d2cb1715ab83 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 3 Aug 2017 14:21:00 -0400 Subject: [PATCH] Fix lock upgrade hazard in ATExecAttachPartition. Amit Langote Discussion: http://postgr.es/m/CAFjFpReT_kq_uwU_B8aWDxR7jNGE=P0iELycdq5oupi=xSQTOw@mail.gmail.com --- 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 3c42e160b2..7859ef13ac 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.40.0