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

1 files changed, 7 insertions, 18 deletions

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 */