Postpone free.
authorRobert Haas <[email protected]>
Wed, 19 Dec 2018 16:22:15 +0000 (11:22 -0500)
committerRobert Haas <[email protected]>
Wed, 19 Dec 2018 16:55:58 +0000 (11:55 -0500)
src/backend/commands/tablecmds.c
src/backend/utils/cache/relcache.c

index eef3b3a26cbd8e34f794faced59216b25cc818e7..9f92efaac1456f1b9655ba69c21d3cdf48d164dc 100644 (file)
@@ -3627,7 +3627,27 @@ AlterTableGetLockLevel(List *cmds)
 
                        case AT_AttachPartition:
                        case AT_DetachPartition:
-                               cmd_lockmode = AccessExclusiveLock;
+                               /*
+                                * We can attach or detach a partition with only
+                                * ShareUpdateExclusiveLock on the partitioned table, but we require
+                                * AccessExclusiveLock on the partition itself and on any default
+                                * partition of the partitioned table.  If we didn't do this, we could
+                                * be in the middle of routing a tuple to a table and at the same time
+                                * its partition constraint could be changing under us, which would
+                                * possibly result in inserting a tuple that does not satisfy the
+                                * partition constraint.  Or, we could decide to prune the table from
+                                * the query while the partition constraint is changing in such a way
+                                * that the table should no longer be pruned.
+                                *
+                                * Note that attaching or detaching a partition becomes visible to
+                                * other sessions as soon as the transaction which performed the
+                                * operation commits. We can't use the query snapshot, which
+                                * might be older, to determine which partitions are visible to a
+                                * particular query, because the tables that were visible at that time
+                                * might no longer exist, might no longer have a matching tuple
+                                * descriptor, etc.
+                                */
+                               cmd_lockmode = ShareUpdateExclusiveLock;
                                break;
 
                        default:                        /* oops */
index c3071db1cdf90d1229b45f61885875037611d6f5..5ec20767de616bcbfdaab34ec2278a5ca17c2b9c 100644 (file)
@@ -2537,6 +2537,30 @@ RelationClearRelation(Relation relation, bool rebuild)
                        SWAPFIELD(PartitionDesc, rd_partdesc);
                        SWAPFIELD(MemoryContext, rd_pdcxt);
                }
+               else if (rebuild && newrel->rd_partdesc != NULL)
+               {
+                       /*
+                        * If this is a rebuild, that means that the reference count of this
+                        * relation is greater than 0, which means somebody is using it.  We want
+                        * to allow for the possibility that they might still have a pointer to the
+                        * old PartitionDesc, so we don't free it here. Instead, we reparent its
+                        * context under the context for the newly-build PartitionDesc, so that it
+                        * will get freed when that context is eventually destroyed.  While this
+                        * doesn't leak memory permanently, there's no upper limit to how long the
+                        * old PartitionDesc could stick around, so we might want to consider a
+                        * more clever strategy here at some point.  Note also that this strategy
+                        * relies on the fact that a relation which has a partition descriptor
+                        * will never cease having one after a rebuild, which is currently true
+                        * even if the table ends up with no partitions.
+                        *
+                        * NB: At this point in the code, the contents of 'relation' and 'newrel'
+                        * have been swapped and then partially unswapped, so, confusingly, it is
+                        * 'newrel' that points to the old data.
+                        */
+                       MemoryContextSetParent(newrel->rd_pdcxt, relation->rd_pdcxt);
+                       newrel->rd_pdcxt = NULL;
+                       newrel->rd_partdesc = NULL;
+               }
 
 #undef SWAPFIELD