From 56163004b8b2151db279744b77138d4d90e2d5cb Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Wed, 21 Mar 2018 12:03:35 -0300 Subject: Fix relcache handling of the 'default' partition My commit 4dba331cb3dc that moved around CommandCounterIncrement calls in partitioning DDL code unearthed a problem with the relcache handling for the 'default' partition: the construction of a correct relcache entry for the partitioned table was at the mercy of lack of CCI calls in non-trivial amounts of code. This was prone to creating problems later on, as the code develops. This was visible as a test failure in a compile with RELCACHE_FORCE_RELASE (buildfarm member prion). The problem is that after the mentioned commit it was possible to create a relcache entry that had incomplete information regarding the default partition because I introduced a CCI between adding the catalog entries for the default partition (StorePartitionBound) and the update of pg_partitioned_table entry for its parent partitioned table (update_default_partition_oid). It seems the best fix is to move the latter so that it occurs inside the former; the purposeful lack of intervening CCI should be more obvious, and harder to break. I also remove a check in RelationBuildPartitionDesc that returns NULL if the key is not set. I couldn't find any place that needs this hack anymore; probably it was required because of bugs that have since been fixed. Fix a few typos I noticed while reviewing the code involved. Discussion: https://postgr.es/m/20180320182659.nyzn3vqtjbbtfgwq@alvherre.pgsql --- src/backend/commands/tablecmds.c | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) (limited to 'src/backend/commands/tablecmds.c') diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 2ec99f99f93..f5c744b9f5a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -860,10 +860,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, /* Update the pg_class entry. */ StorePartitionBound(rel, parent, bound); - /* Update the default partition oid */ - if (bound->is_default) - update_default_partition_oid(RelationGetRelid(parent), relationId); - heap_close(parent, NoLock); } @@ -14021,11 +14017,6 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) /* OK to create inheritance. Rest of the checks performed there */ CreateInheritance(attachrel, rel); - /* Update the default partition oid */ - if (cmd->bound->is_default) - update_default_partition_oid(RelationGetRelid(rel), - RelationGetRelid(attachrel)); - /* * Check that the new partition's bound is valid and does not overlap any * of existing partitions of the parent - note that it does not return on @@ -14286,7 +14277,7 @@ ATExecDetachPartition(Relation rel, RangeVar *name) /* * We must lock the default partition, because detaching this partition - * will changing its partition constrant. + * will change its partition constraint. */ defaultPartOid = get_default_oid_from_partdesc(RelationGetPartitionDesc(rel)); @@ -14329,19 +14320,17 @@ ATExecDetachPartition(Relation rel, RangeVar *name) if (OidIsValid(defaultPartOid)) { /* - * If the detach relation is the default partition itself, invalidate - * its entry in pg_partitioned_table. + * If the relation being detached is the default partition itself, + * remove it from the parent's pg_partitioned_table entry. + * + * If not, we must invalidate default partition's relcache entry, as + * in StorePartitionBound: its partition constraint depends on every + * other partition's partition constraint. */ if (RelationGetRelid(partRel) == defaultPartOid) update_default_partition_oid(RelationGetRelid(rel), InvalidOid); else - { - /* - * We must invalidate default partition's relcache, for the same - * reasons explained in StorePartitionBound(). - */ CacheInvalidateRelcacheByRelid(defaultPartOid); - } } /* detach indexes too */ -- cgit v1.2.3