From 1c4aa31ecb862a28089bee6a502f77dea03013b6 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Wed, 19 Dec 2018 11:22:15 -0500 Subject: [PATCH] Postpone free. --- src/backend/commands/tablecmds.c | 22 +++++++++++++++++++++- src/backend/utils/cache/relcache.c | 24 ++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index eef3b3a26c..9f92efaac1 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -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 */ diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index c3071db1cd..5ec20767de 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -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 -- 2.39.5