Date: Mon, 3 Sep 2018 18:39:16 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: pointless check in RelationBuildPartitionDesc
Message-ID: <20180903213916.hh6wasnrdg6xv2ud@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="wytugto5aytronr4"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
User-Agent: NeoMutt/20170306-137-4415bd-dirty (1.8.0)
Precedence: bulk


--wytugto5aytronr4
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

I noticed this strange hack in RelationBuildPartitionDesc:

       /*
        * It is possible that the pg_class tuple of a partition has not been
        * updated yet to set its relpartbound field.  The only case where
        * this happens is when we open the parent relation to check using its
        * partition descriptor that a new partition's bound does not overlap
        * some existing partition.
        */
       if (!((Form_pg_class) GETSTRUCT(tuple))->relispartition)
       {
           ReleaseSysCache(tuple);
           continue;
       }

After looking, it seems that this is just self-inflicted pain: for some
reason, we store the pg_inherits row for a partition, and immediately
afterwards compute and store its partition bound, which requires the
above hack.  But if we do things in the opposite order, this is no
longer needed.  I propose to remove it, as in the attached patch.

-- 
Álvaro Herrera                         Developer, https://www.PostgreSQL.org/

--wytugto5aytronr4
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="ispartition.patch"

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f46af41b56..089e9f1b20 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -773,9 +773,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 										  InvalidOid,
 										  typaddress);
 
-	/* Store inheritance information for new rel. */
-	StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
-
 	/*
 	 * We must bump the command counter to make the newly-created relation
 	 * tuple visible for opening.
@@ -869,6 +866,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		heap_close(parent, NoLock);
 	}
 
+	/* Store inheritance information for new rel. */
+	StoreCatalogInheritance(relationId, inheritOids, stmt->partbound != NULL);
+
 	/*
 	 * Process the partitioning specification (if any) and store the partition
 	 * key information into the catalog.
diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c
index 115a9fe78f..e3cd65b33a 100644
--- a/src/backend/utils/cache/partcache.c
+++ b/src/backend/utils/cache/partcache.c
@@ -302,19 +302,6 @@ RelationBuildPartitionDesc(Relation rel)
 		if (!HeapTupleIsValid(tuple))
 			elog(ERROR, "cache lookup failed for relation %u", inhrelid);
 
-		/*
-		 * It is possible that the pg_class tuple of a partition has not been
-		 * updated yet to set its relpartbound field.  The only case where
-		 * this happens is when we open the parent relation to check using its
-		 * partition descriptor that a new partition's bound does not overlap
-		 * some existing partition.
-		 */
-		if (!((Form_pg_class) GETSTRUCT(tuple))->relispartition)
-		{
-			ReleaseSysCache(tuple);
-			continue;
-		}
-
 		datum = SysCacheGetAttr(RELOID, tuple,
 								Anum_pg_class_relpartbound,
 								&isnull);

--wytugto5aytronr4--