diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index ba03e8aa8f..42ff175bc8 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -3840,7 +3840,7 @@ StorePartitionBound(Relation rel, Relation parent, PartitionBoundSpec *bound) * removed. */ defaultPartOid = - get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, false)); + get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, true)); if (OidIsValid(defaultPartOid)) CacheInvalidateRelcacheByRelid(defaultPartOid); diff --git a/src/backend/catalog/pg_inherits.c b/src/backend/catalog/pg_inherits.c index bb8b2249b1..98bf48d1e2 100644 --- a/src/backend/catalog/pg_inherits.c +++ b/src/backend/catalog/pg_inherits.c @@ -52,13 +52,19 @@ typedef struct SeenRelsEntry * then no locks are acquired, but caller must beware of race conditions * against possible DROPs of child relations. * - * include_detached says to include all partitions, even if they're marked - * detached. Passing it as false means they might or might not be included, - * depending on the visibility of the pg_inherits row for the active snapshot. + * If a partition's pg_inherits row is marked "detach pending", + * *detached_exist (if not null) is set true, otherwise it is set false. + * + * If omit_detached is true and there is an active snapshot (not the same as + * the catalog snapshot used to scan pg_inherits!) and a pg_inherits tuple + * marked "detach pending" is visible to that snapshot, then that partition is + * omitted from the output list. This makes partitions invisible depending on + * whether the transaction that marked those partitions as detached appears + * committed to the active snapshot. */ List * -find_inheritance_children(Oid parentrelId, bool include_detached, - LOCKMODE lockmode) +find_inheritance_children(Oid parentrelId, bool omit_detached, + LOCKMODE lockmode, bool *detached_exist) { List *list = NIL; Relation relation; @@ -78,6 +84,9 @@ find_inheritance_children(Oid parentrelId, bool include_detached, if (!has_subclass(parentrelId)) return NIL; + if (detached_exist) + *detached_exist = false; + /* * Scan pg_inherits and build a working array of subclass OIDs. */ @@ -99,29 +108,35 @@ find_inheritance_children(Oid parentrelId, bool include_detached, { /* * Cope with partitions concurrently being detached. When we see a - * partition marked "detach pending", we only include it in the set of - * visible partitions if caller requested all detached partitions, or - * if its pg_inherits tuple's xmin is still visible to the active - * snapshot. + * partition marked "detach pending", we omit it from the returned set + * of visible partitions if caller requested that and the tuple's xmin + * does not appear in progress to the active snapshot. (If there's no + * active snapshot set, that means we're not running a user query, so + * it's OK to always include detached partitions in that case; if the + * xmin is still running to the active snapshot, then the partition + * has not been detached yet and so we include it.) * - * The reason for this check is that we want to avoid seeing the + * The reason for this hack is that we want to avoid seeing the * partition as alive in RI queries during REPEATABLE READ or - * SERIALIZABLE transactions. (If there's no active snapshot set, - * that means we're not running a user query, so it's OK to always - * include detached partitions in that case.) + * SERIALIZABLE transactions: such queries use a different snapshot + * than the one used by regular (user) queries. */ - if (((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhdetachpending && - !include_detached && - ActiveSnapshotSet()) + if (((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhdetachpending) { - TransactionId xmin; - Snapshot snap; + if (detached_exist) + *detached_exist = true; - xmin = HeapTupleHeaderGetXmin(inheritsTuple->t_data); - snap = GetActiveSnapshot(); + if (omit_detached && ActiveSnapshotSet()) + { + TransactionId xmin; + Snapshot snap; - if (!XidInMVCCSnapshot(xmin, snap)) - continue; + xmin = HeapTupleHeaderGetXmin(inheritsTuple->t_data); + snap = GetActiveSnapshot(); + + if (!XidInMVCCSnapshot(xmin, snap)) + continue; + } } inhrelid = ((Form_pg_inherits) GETSTRUCT(inheritsTuple))->inhrelid; @@ -235,8 +250,8 @@ find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **numparents) ListCell *lc; /* Get the direct children of this rel */ - currentchildren = find_inheritance_children(currentrel, false, - lockmode); + currentchildren = find_inheritance_children(currentrel, true, + lockmode, NULL); /* * Add to the queue only those children not already seen. This avoids diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 166374cc0c..3edf61993a 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -1123,7 +1123,7 @@ DefineIndex(Oid relationId, */ if (partitioned && stmt->relation && !stmt->relation->inh) { - PartitionDesc pd = RelationGetPartitionDesc(rel, false); + PartitionDesc pd = RelationGetPartitionDesc(rel, true); if (pd->nparts != 0) flags |= INDEX_CREATE_INVALID; @@ -1180,7 +1180,7 @@ DefineIndex(Oid relationId, * * If we're called internally (no stmt->relation), recurse always. */ - partdesc = RelationGetPartitionDesc(rel, false); + partdesc = RelationGetPartitionDesc(rel, true); if ((!stmt->relation || stmt->relation->inh) && partdesc->nparts > 0) { int nparts = partdesc->nparts; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 97cc9fd6ec..7d00f4eb25 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1041,7 +1041,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, */ defaultPartOid = get_default_oid_from_partdesc(RelationGetPartitionDesc(parent, - false)); + true)); if (OidIsValid(defaultPartOid)) defaultRel = table_open(defaultPartOid, AccessExclusiveLock); @@ -3507,7 +3507,7 @@ renameatt_internal(Oid myrelid, * expected_parents will only be 0 if we are not already recursing. */ if (expected_parents == 0 && - find_inheritance_children(myrelid, false, NoLock) != NIL) + find_inheritance_children(myrelid, true, NoLock, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("inherited column \"%s\" must be renamed in child tables too", @@ -3706,7 +3706,7 @@ rename_constraint_internal(Oid myrelid, else { if (expected_parents == 0 && - find_inheritance_children(myrelid, false, NoLock) != NIL) + find_inheritance_children(myrelid, true, NoLock, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("inherited constraint \"%s\" must be renamed in child tables too", @@ -6580,7 +6580,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, */ if (colDef->identity && recurse && - find_inheritance_children(myrelid, false, NoLock) != NIL) + find_inheritance_children(myrelid, true, NoLock, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("cannot recursively add identity column to table that has child tables"))); @@ -6826,7 +6826,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, * use find_all_inheritors to do it in one pass. */ children = - find_inheritance_children(RelationGetRelid(rel), false, lockmode); + find_inheritance_children(RelationGetRelid(rel), true, lockmode, NULL); /* * If we are told not to recurse, there had better not be any child @@ -6980,7 +6980,7 @@ ATPrepDropNotNull(Relation rel, bool recurse, bool recursing) */ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc partdesc = RelationGetPartitionDesc(rel, false); + PartitionDesc partdesc = RelationGetPartitionDesc(rel, true); Assert(partdesc != NULL); if (partdesc->nparts > 0 && !recurse && !recursing) @@ -7689,7 +7689,7 @@ ATPrepDropExpression(Relation rel, AlterTableCmd *cmd, bool recurse, bool recurs * resulting state can be properly dumped and restored. */ if (!recurse && - find_inheritance_children(RelationGetRelid(rel), false, lockmode)) + find_inheritance_children(RelationGetRelid(rel), true, lockmode, NULL)) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("ALTER TABLE / DROP EXPRESSION must be applied to child tables too"))); @@ -8297,7 +8297,7 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, * use find_all_inheritors to do it in one pass. */ children = - find_inheritance_children(RelationGetRelid(rel), false, lockmode); + find_inheritance_children(RelationGetRelid(rel), true, lockmode, NULL); if (children) { @@ -8785,7 +8785,7 @@ ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel, * use find_all_inheritors to do it in one pass. */ children = - find_inheritance_children(RelationGetRelid(rel), false, lockmode); + find_inheritance_children(RelationGetRelid(rel), true, lockmode, NULL); /* * Check if ONLY was specified with ALTER TABLE. If so, allow the @@ -9400,7 +9400,7 @@ addFkRecurseReferenced(List **wqueue, Constraint *fkconstraint, Relation rel, */ if (pkrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc pd = RelationGetPartitionDesc(pkrel, false); + PartitionDesc pd = RelationGetPartitionDesc(pkrel, true); for (int i = 0; i < pd->nparts; i++) { @@ -9534,7 +9534,7 @@ addFkRecurseReferencing(List **wqueue, Constraint *fkconstraint, Relation rel, } else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc pd = RelationGetPartitionDesc(rel, false); + PartitionDesc pd = RelationGetPartitionDesc(rel, true); /* * Recurse to take appropriate action on each partition; either we @@ -11318,8 +11318,8 @@ ATExecDropConstraint(Relation rel, const char *constrName, * use find_all_inheritors to do it in one pass. */ if (!is_no_inherit_constraint) - children = - find_inheritance_children(RelationGetRelid(rel), false, lockmode); + children = find_inheritance_children(RelationGetRelid(rel), true, + lockmode, NULL); else children = NIL; @@ -11703,8 +11703,8 @@ ATPrepAlterColumnType(List **wqueue, } } else if (!recursing && - find_inheritance_children(RelationGetRelid(rel), false, - NoLock) != NIL) + find_inheritance_children(RelationGetRelid(rel), true, + NoLock, NULL) != NIL) ereport(ERROR, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("type of inherited column \"%s\" must be changed in child tables too", @@ -16875,7 +16875,7 @@ QueuePartitionConstraintValidation(List **wqueue, Relation scanrel, } else if (scanrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { - PartitionDesc partdesc = RelationGetPartitionDesc(scanrel, false); + PartitionDesc partdesc = RelationGetPartitionDesc(scanrel, true); int i; for (i = 0; i < partdesc->nparts; i++) @@ -16935,7 +16935,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd, * new partition will change its partition constraint. */ defaultPartOid = - get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, false)); + get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, true)); if (OidIsValid(defaultPartOid)) LockRelationOid(defaultPartOid, AccessExclusiveLock); @@ -17551,7 +17551,7 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, * will change its partition constraint. */ defaultPartOid = - get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, false)); + get_default_oid_from_partdesc(RelationGetPartitionDesc(rel, true)); if (OidIsValid(defaultPartOid)) { /* @@ -18148,7 +18148,7 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name) RelationGetRelationName(partIdx)))); /* Make sure it indexes a partition of the other index's table */ - partDesc = RelationGetPartitionDesc(parentTbl, false); + partDesc = RelationGetPartitionDesc(parentTbl, true); found = false; for (i = 0; i < partDesc->nparts; i++) { @@ -18302,7 +18302,7 @@ validatePartitionedIndex(Relation partedIdx, Relation partedTbl) * If we found as many inherited indexes as the partitioned table has * partitions, we're good; update pg_index to set indisvalid. */ - if (tuples == RelationGetPartitionDesc(partedTbl, false)->nparts) + if (tuples == RelationGetPartitionDesc(partedTbl, true)->nparts) { Relation idxRel; HeapTuple newtup; diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 3421014e47..d8393aa4de 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -1119,7 +1119,7 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, */ if (partition_recurse) { - PartitionDesc partdesc = RelationGetPartitionDesc(rel, false); + PartitionDesc partdesc = RelationGetPartitionDesc(rel, true); List *idxs = NIL; List *childTbls = NIL; ListCell *l; @@ -1141,8 +1141,8 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, ListCell *l; List *idxs = NIL; - idxs = find_inheritance_children(indexOid, false, - ShareRowExclusiveLock); + idxs = find_inheritance_children(indexOid, true, + ShareRowExclusiveLock, NULL); foreach(l, idxs) childTbls = lappend_oid(childTbls, IndexGetRelation(lfirst_oid(l), diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 99780ebb96..8afddca73a 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -991,19 +991,16 @@ ExecInitPartitionDispatchInfo(EState *estate, /* * For data modification, it is better that executor does not include - * partitions being detached, except in snapshot-isolation mode. This - * means that a read-committed transaction immediately gets a "no - * partition for tuple" error when a tuple is inserted into a partition - * that's being detached concurrently, but a transaction in repeatable- - * read mode can still use the partition. Note that because partition - * detach uses ShareLock on the partition (which conflicts with DML), - * we're certain that the detach won't be able to complete until any - * inserting transaction is done. + * partitions being detached, except when running in snapshot-isolation + * mode. This means that a read-committed transaction immediately gets a + * "no partition for tuple" error when a tuple is inserted into a + * partition that's being detached concurrently, but a transaction in + * repeatable-read mode can still use such a partition. */ if (estate->es_partition_directory == NULL) estate->es_partition_directory = CreatePartitionDirectory(estate->es_query_cxt, - IsolationUsesXactSnapshot()); + !IsolationUsesXactSnapshot()); oldcxt = MemoryContextSwitchTo(proute->memcxt); @@ -1571,10 +1568,10 @@ ExecCreatePartitionPruneState(PlanState *planstate, ListCell *lc; int i; - /* Executor must always include detached partitions */ + /* For data reading, executor always omits detached partitions */ if (estate->es_partition_directory == NULL) estate->es_partition_directory = - CreatePartitionDirectory(estate->es_query_cxt, true); + CreatePartitionDirectory(estate->es_query_cxt, false); n_part_hierarchies = list_length(partitionpruneinfo->prune_infos); Assert(n_part_hierarchies > 0); diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 345c7425f6..295ce11450 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -2200,7 +2200,7 @@ set_relation_partition_info(PlannerInfo *root, RelOptInfo *rel, if (root->glob->partition_directory == NULL) { root->glob->partition_directory = - CreatePartitionDirectory(CurrentMemoryContext, false); + CreatePartitionDirectory(CurrentMemoryContext, true); } partdesc = PartitionDirectoryLookup(root->glob->partition_directory, diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c index 1290d45963..c9c789297d 100644 --- a/src/backend/partitioning/partbounds.c +++ b/src/backend/partitioning/partbounds.c @@ -2798,7 +2798,7 @@ check_new_partition_bound(char *relname, Relation parent, PartitionBoundSpec *spec, ParseState *pstate) { PartitionKey key = RelationGetPartitionKey(parent); - PartitionDesc partdesc = RelationGetPartitionDesc(parent, true); + PartitionDesc partdesc = RelationGetPartitionDesc(parent, false); PartitionBoundInfo boundinfo = partdesc->boundinfo; int with = -1; bool overlap = false; @@ -3991,7 +3991,7 @@ get_qual_for_list(Relation parent, PartitionBoundSpec *spec) { int i; int ndatums = 0; - PartitionDesc pdesc = RelationGetPartitionDesc(parent, true); /* XXX correct? */ + PartitionDesc pdesc = RelationGetPartitionDesc(parent, false); PartitionBoundInfo boundinfo = pdesc->boundinfo; if (boundinfo) @@ -4191,7 +4191,7 @@ get_qual_for_range(Relation parent, PartitionBoundSpec *spec, if (spec->is_default) { List *or_expr_args = NIL; - PartitionDesc pdesc = RelationGetPartitionDesc(parent, true); /* XXX correct? */ + PartitionDesc pdesc = RelationGetPartitionDesc(parent, false); Oid *inhoids = pdesc->oids; int nparts = pdesc->nparts, i; diff --git a/src/backend/partitioning/partdesc.c b/src/backend/partitioning/partdesc.c index 58570fecfd..12ef36a73e 100644 --- a/src/backend/partitioning/partdesc.c +++ b/src/backend/partitioning/partdesc.c @@ -37,7 +37,7 @@ typedef struct PartitionDirectoryData { MemoryContext pdir_mcxt; HTAB *pdir_hash; - bool include_detached; + bool omit_detached; } PartitionDirectoryData; typedef struct PartitionDirectoryEntry @@ -47,7 +47,8 @@ typedef struct PartitionDirectoryEntry PartitionDesc pd; } PartitionDirectoryEntry; -static void RelationBuildPartitionDesc(Relation rel, bool include_detached); +static PartitionDesc RelationBuildPartitionDesc(Relation rel, + bool omit_detached); /* @@ -60,18 +61,29 @@ static void RelationBuildPartitionDesc(Relation rel, bool include_detached); * for callers to continue to use that pointer as long as (a) they hold the * relation open, and (b) they hold a relation lock strong enough to ensure * that the data doesn't become stale. + * + * The above applies to partition descriptors that are complete regarding + * partitions concurrently being detached. When a descriptor that omits + * partitions being detached is requested (and such partitions are present), + * said descriptor is not part of relcache and so it isn't freed by + * invalidations either. Caller must not use such a descriptor beyond the + * current Portal. */ PartitionDesc -RelationGetPartitionDesc(Relation rel, bool include_detached) +RelationGetPartitionDesc(Relation rel, bool omit_detached) { - if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) - return NULL; + Assert(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE); - if (unlikely(rel->rd_partdesc == NULL || - rel->rd_partdesc->includes_detached != include_detached)) - RelationBuildPartitionDesc(rel, include_detached); + /* + * If relcache has a partition descriptor, use that. However, we can only + * do so when we are asked to include all partitions including detached; + * and also when we know that there are no detached partitions. + */ + if (likely(rel->rd_partdesc && + (!rel->rd_partdesc->detached_exist || !omit_detached))) + return rel->rd_partdesc; - return rel->rd_partdesc; + return RelationBuildPartitionDesc(rel, omit_detached); } /* @@ -88,9 +100,15 @@ RelationGetPartitionDesc(Relation rel, bool include_detached) * context the current context except in very brief code sections, out of fear * that some of our callees allocate memory on their own which would be leaked * permanently. + * + * As a special case, partition descriptors that are requested to omit + * partitions being detached (and which contain such partitions) are transient + * and are not associated with the relcache entry. Such descriptors only last + * through the requesting Portal, so we use the corresponding memory context + * for them. */ -static void -RelationBuildPartitionDesc(Relation rel, bool include_detached) +static PartitionDesc +RelationBuildPartitionDesc(Relation rel, bool omit_detached) { PartitionDesc partdesc; PartitionBoundInfo boundinfo = NULL; @@ -98,6 +116,7 @@ RelationBuildPartitionDesc(Relation rel, bool include_detached) PartitionBoundSpec **boundspecs = NULL; Oid *oids = NULL; bool *is_leaf = NULL; + bool detached_exist; ListCell *cell; int i, nparts; @@ -112,8 +131,8 @@ RelationBuildPartitionDesc(Relation rel, bool include_detached) * concurrently, whatever this function returns will be accurate as of * some well-defined point in time. */ - inhoids = find_inheritance_children(RelationGetRelid(rel), include_detached, - NoLock); + inhoids = find_inheritance_children(RelationGetRelid(rel), omit_detached, + NoLock, &detached_exist); nparts = list_length(inhoids); /* Allocate working arrays for OIDs, leaf flags, and boundspecs. */ @@ -234,6 +253,7 @@ RelationBuildPartitionDesc(Relation rel, bool include_detached) partdesc = (PartitionDescData *) MemoryContextAllocZero(new_pdcxt, sizeof(PartitionDescData)); partdesc->nparts = nparts; + partdesc->detached_exist = detached_exist; /* If there are no partitions, the rest of the partdesc can stay zero */ if (nparts > 0) { @@ -241,7 +261,6 @@ RelationBuildPartitionDesc(Relation rel, bool include_detached) partdesc->boundinfo = partition_bounds_copy(boundinfo, key); partdesc->oids = (Oid *) palloc(nparts * sizeof(Oid)); partdesc->is_leaf = (bool *) palloc(nparts * sizeof(bool)); - partdesc->includes_detached = include_detached; /* * Assign OIDs from the original array into mapped indexes of the @@ -261,22 +280,41 @@ RelationBuildPartitionDesc(Relation rel, bool include_detached) } /* - * We have a fully valid partdesc ready to store into the relcache. - * Reparent it so it has the right lifespan. + * We have a fully valid partdesc. Reparent it so that it has the right + * lifespan, and if appropriate put it into the relation's relcache entry. */ - MemoryContextSetParent(new_pdcxt, CacheMemoryContext); + if (omit_detached && detached_exist) + { + /* + * A transient partition descriptor is only good for the current + * statement, so make it a child of the current portal's context. + */ + MemoryContextSetParent(new_pdcxt, PortalContext); + } + else + { + /* + * This partdesc goes into relcache. + */ - /* - * But first, a kluge: if there's an old rd_pdcxt, it contains an old - * partition descriptor that may still be referenced somewhere. Preserve - * it, while not leaking it, by reattaching it as a child context of the - * new rd_pdcxt. Eventually it will get dropped by either RelationClose - * or RelationClearRelation. - */ - if (rel->rd_pdcxt != NULL) - MemoryContextSetParent(rel->rd_pdcxt, new_pdcxt); - rel->rd_pdcxt = new_pdcxt; - rel->rd_partdesc = partdesc; + MemoryContextSetParent(new_pdcxt, CacheMemoryContext); + + /* + * But first, a kluge: if there's an old rd_pdcxt, it contains an old + * partition descriptor that may still be referenced somewhere. + * Preserve it, while not leaking it, by reattaching it as a child + * context of the new rd_pdcxt. Eventually it will get dropped by + * either RelationClose or RelationClearRelation. + */ + if (rel->rd_pdcxt != NULL) + MemoryContextSetParent(rel->rd_pdcxt, new_pdcxt); + rel->rd_pdcxt = new_pdcxt; + + /* Store it into relcache */ + rel->rd_partdesc = partdesc; + } + + return partdesc; } /* @@ -284,7 +322,7 @@ RelationBuildPartitionDesc(Relation rel, bool include_detached) * Create a new partition directory object. */ PartitionDirectory -CreatePartitionDirectory(MemoryContext mcxt, bool include_detached) +CreatePartitionDirectory(MemoryContext mcxt, bool omit_detached) { MemoryContext oldcontext = MemoryContextSwitchTo(mcxt); PartitionDirectory pdir; @@ -299,7 +337,7 @@ CreatePartitionDirectory(MemoryContext mcxt, bool include_detached) pdir->pdir_hash = hash_create("partition directory", 256, &ctl, HASH_ELEM | HASH_BLOBS | HASH_CONTEXT); - pdir->include_detached = include_detached; + pdir->omit_detached = omit_detached; MemoryContextSwitchTo(oldcontext); return pdir; @@ -332,7 +370,7 @@ PartitionDirectoryLookup(PartitionDirectory pdir, Relation rel) */ RelationIncrementReferenceCount(rel); pde->rel = rel; - pde->pd = RelationGetPartitionDesc(rel, pdir->include_detached); + pde->pd = RelationGetPartitionDesc(rel, pdir->omit_detached); Assert(pde->pd != NULL); } return pde->pd; diff --git a/src/include/catalog/pg_inherits.h b/src/include/catalog/pg_inherits.h index 6d07e1b302..4d28ede5a6 100644 --- a/src/include/catalog/pg_inherits.h +++ b/src/include/catalog/pg_inherits.h @@ -50,8 +50,8 @@ DECLARE_INDEX(pg_inherits_parent_index, 2187, on pg_inherits using btree(inhpare #define InheritsParentIndexId 2187 -extern List *find_inheritance_children(Oid parentrelId, bool include_detached, - LOCKMODE lockmode); +extern List *find_inheritance_children(Oid parentrelId, bool omit_detached, + LOCKMODE lockmode, bool *detached_exist); extern List *find_all_inheritors(Oid parentrelId, LOCKMODE lockmode, List **parents); extern bool has_subclass(Oid relationId); diff --git a/src/include/partitioning/partdesc.h b/src/include/partitioning/partdesc.h index 7f03ff4271..0792f48507 100644 --- a/src/include/partitioning/partdesc.h +++ b/src/include/partitioning/partdesc.h @@ -17,11 +17,19 @@ /* * Information about partitions of a partitioned table. + * + * For partitioned tables where detached partitions exist, we only cache + * descriptors that include all partitions, including detached; when we're + * requested a descriptor without the detached partitions, we create one + * afresh each time. (The reason for this is that the set of detached + * partitions that are visible to each caller depends on the snapshot it has, + * so it's pretty much impossible to evict a descriptor from cache at the + * right time.) */ typedef struct PartitionDescData { int nparts; /* Number of partitions */ - bool includes_detached; /* Does it include detached partitions */ + bool detached_exist; /* Are there any detached partitions? */ Oid *oids; /* Array of 'nparts' elements containing * partition OIDs in order of the their bounds */ bool *is_leaf; /* Array of 'nparts' elements storing whether @@ -31,9 +39,9 @@ typedef struct PartitionDescData } PartitionDescData; -extern PartitionDesc RelationGetPartitionDesc(Relation rel, bool include_detached); +extern PartitionDesc RelationGetPartitionDesc(Relation rel, bool omit_detached); -extern PartitionDirectory CreatePartitionDirectory(MemoryContext mcxt, bool include_detached); +extern PartitionDirectory CreatePartitionDirectory(MemoryContext mcxt, bool omit_detached); extern PartitionDesc PartitionDirectoryLookup(PartitionDirectory, Relation); extern void DestroyPartitionDirectory(PartitionDirectory pdir); diff --git a/src/test/isolation/expected/detach-partition-concurrently-4.out b/src/test/isolation/expected/detach-partition-concurrently-4.out index 90a75cb077..2167675374 100644 --- a/src/test/isolation/expected/detach-partition-concurrently-4.out +++ b/src/test/isolation/expected/detach-partition-concurrently-4.out @@ -324,6 +324,7 @@ a 1 2 step s1insert: insert into d4_fk values (1); +ERROR: insert or update on table "d4_fk" violates foreign key constraint "d4_fk_a_fkey" step s1c: commit; starting permutation: s2snitch s1b s1s s2detach s1cancel s3vacfreeze s1s s1insert s1c