Simplify partitioned table creation vs. relcache

In the original code, we were storing the pg_inherits row for a
partitioned table too early: enough that we had a hack for relcache to
avoid falling flat on its face while reading such a partial entry.  If
we finish the pg_class creation first and *then* store the pg_inherits
entry, we don't need that hack.

Also recognize that pg_class.relpartbound is not marked NOT NULL and
therefore it's entirely possible to read null values, so having only
Assert() protection isn't enough.  Change those to if/elog tests
instead.  This qualifies as a robustness fix, so backpatch to pg11.

In passing, remove one access that wasn't actually needed, and reword
one message to be like all the others that check for the same thing.

Reviewed-by: Amit Langote
Discussion: https://postgr.es/m/20180903213916.hh6wasnrdg6xv2ud@alvherre.pgsql

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f9e83c24565637dba0d5ea1ed9b854134fe5d723..a1cb15ca302310e4c13b587f6b52d80d379a7a95 100644 (file)
--- 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.
@@ -14703,10 +14703,6 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
             RelationGetRelid(partRel));
    Assert(((Form_pg_class) GETSTRUCT(tuple))->relispartition);
 
-   (void) SysCacheGetAttr(RELOID, tuple, Anum_pg_class_relpartbound,
-                          &isnull);
-   Assert(!isnull);
-
    /* Clear relpartbound and reset relispartition */
    memset(new_val, 0, sizeof(new_val));
    memset(new_null, false, sizeof(new_null));

diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 9015a05d323a4a083ec64f3b87ad6229f9398042..4ed99206181120171a96f7788639a173014a5fe3 100644 (file)
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -1641,8 +1641,9 @@ get_qual_for_range(Relation parent, PartitionBoundSpec *spec,
            datum = SysCacheGetAttr(RELOID, tuple,
                                    Anum_pg_class_relpartbound,
                                    &isnull);
+           if (isnull)
+               elog(ERROR, "null relpartbound for relation %u", inhrelid);
 
-           Assert(!isnull);
            bspec = (PartitionBoundSpec *)
                stringToNode(TextDatumGetCString(datum));
            if (!IsA(bspec, PartitionBoundSpec))

diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c
index 115a9fe78ff2ae42878a7ed11a82d296e4d448d8..e35a43405eb5d82395af8b257d44680ccac75be5 100644 (file)
--- a/src/backend/utils/cache/partcache.c
+++ b/src/backend/utils/cache/partcache.c
@@ -302,23 +302,11 @@ 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);
-       Assert(!isnull);
+       if (isnull)
+           elog(ERROR, "null relpartbound for relation %u", inhrelid);
        boundspec = (Node *) stringToNode(TextDatumGetCString(datum));
 
        /*
@@ -883,9 +871,8 @@ generate_partition_qual(Relation rel)
    boundDatum = SysCacheGetAttr(RELOID, tuple,
                                 Anum_pg_class_relpartbound,
                                 &isnull);
-   if (isnull)                 /* should not happen */
-       elog(ERROR, "relation \"%s\" has relpartbound = null",
-            RelationGetRelationName(rel));
+   if (isnull)
+       elog(ERROR, "null relpartbound for relation %u", RelationGetRelid(rel));
    bound = castNode(PartitionBoundSpec,
                     stringToNode(TextDatumGetCString(boundDatum)));
    ReleaseSysCache(tuple);