]> granicus.if.org Git - postgresql/commitdiff
Revert lowering of lock level for ATTACH PARTITION
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Thu, 12 Apr 2018 19:53:27 +0000 (16:53 -0300)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Thu, 12 Apr 2018 19:53:27 +0000 (16:53 -0300)
I lowered the lock level for partitions being scanned from
AccessExclusive to ShareLock in the course of 72cf7f310c07, but that was
bogus, as pointed out by Robert Haas.  Revert that bit.  Doing this is
possible, but requires more work.

Discussion: https://postgr.es/m/CA+TgmobV7Nfmqv+TZXcdSsb9Bjc-OL-Anv6BNmCbfJVZLYPE4Q@mail.gmail.com

src/backend/commands/tablecmds.c

index 562c815c63fd52b5fbb796dc9d4c21cc25bd1291..f27684f96e4bbc1305e21b6076becbf8d2c36f63 100644 (file)
@@ -13997,10 +13997,9 @@ QueuePartitionConstraintValidation(List **wqueue, Relation scanrel,
                        List       *thisPartConstraint;
 
                        /*
-                        * This is the minimum lock we need to prevent concurrent data
-                        * additions.
+                        * This is the minimum lock we need to prevent deadlocks.
                         */
-                       part_rel = heap_open(partdesc->oids[i], ShareLock);
+                       part_rel = heap_open(partdesc->oids[i], AccessExclusiveLock);
 
                        /*
                         * Adjust the constraint for scanrel so that it matches this
@@ -14113,17 +14112,17 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
         *
         * 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 a lock on all
-        * partitions.  We need ShareLock, preventing data changes, 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
-        * weaker lock now and the stronger one only when needed.
+        * 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),
-                                                                                        ShareLock, NULL);
+                                                                                        AccessExclusiveLock, NULL);
        if (list_member_oid(attachrel_children, RelationGetRelid(rel)))
                ereport(ERROR,
                                (errcode(ERRCODE_DUPLICATE_TABLE),