Thanks for the report.

On 2018/10/04 3:58, Rajkumar Raghuwanshi wrote:
> Hi,
> 
> I am getting ERROR:  null relpartbound for relation 18159 while doing
> pg_upgrade from v11 to v11/master.
> 
> -- user-defined operator class in partition key
> CREATE FUNCTION my_int4_sort(int4,int4) RETURNS int LANGUAGE sql
>   AS $$ SELECT CASE WHEN $1 = $2 THEN 0 WHEN $1 > $2 THEN 1 ELSE -1 END; $$;
> CREATE OPERATOR CLASS test_int4_ops FOR TYPE int4 USING btree AS
>   OPERATOR 1 < (int4,int4), OPERATOR 2 <= (int4,int4),
>   OPERATOR 3 = (int4,int4), OPERATOR 4 >= (int4,int4),
>   OPERATOR 5 > (int4,int4), FUNCTION 1 my_int4_sort(int4,int4);
> CREATE TABLE partkey_t (a int4) PARTITION BY RANGE (a test_int4_ops);
> CREATE TABLE partkey_t_1 PARTITION OF partkey_t FOR VALUES FROM (0) TO
> (1000);
> INSERT INTO partkey_t VALUES (100);
> INSERT INTO partkey_t VALUES (200);
> 
> --ran pg_upgrade failed with below error.
> pg_restore: [archiver (db)] could not execute query: ERROR:  null
> relpartbound for relation 18159
> CONTEXT:  SQL function "my_int4_sort"

Interesting test case.

To reproduce, the following works too (after creating the objects as
described above):

alter table partkey_t detach partition partkey_t_1;
alter table partkey_t attach partition partkey_t_1 for values from (0) to
(1000);
ERROR:  null relpartbound for relation 16396
CONTEXT:  SQL function "my_int4_sort"

The stack at the time of the error:

(gdb) bt
#0  RelationBuildPartitionDesc
#1  0x00000000009bf04e in RelationBuildDesc
#2  0x00000000009c1784 in RelationClearRelation
#3  0x00000000009c1cc5 in RelationFlushRelation
#4  0x00000000009c1dd9 in RelationCacheInvalidateEntry
#5  0x00000000009b9496 in LocalExecuteInvalidationMessage
#6  0x00000000009b91ec in ProcessInvalidationMessages
#7  0x00000000009b9cdb in CommandEndInvalidationMessages
#8  0x00000000005346ef in AtCCI_LocalCache
#9  0x0000000000534124 in CommandCounterIncrement
#10 0x00000000006c579d in fmgr_sql
#11 0x00000000009de7c2 in FunctionCall2Coll
#12 0x000000000058ac9f in partition_rbound_cmp
#13 0x0000000000588059 in check_new_partition_bound
#14 0x000000000067f536 in ATExecAttachPartition
<snip>

So, the CommandCounterIncrement done in fmgr_sql causes partkey_t's
PartitionDesc to be recomputed, which counts partkey_t_1 as its child
because ATExecAttachPartition has already finished CreateInheritance which
would've sent out an invalidation message for partkey_t.

As of commit 2fbdf1b38bc [1], which has been applied in 11 and HEAD
branches, RelationBuildPartitionDesc emits an error if we don't find
relpartbound set for a child found by scanning pg_inherits, instead of
skipping such children.  While that commit switched the order of creating
pg_inherits entry and checking a new bound against existing bounds in
DefineRelation in light of aforementioned change, it didn't in
ATExecAttachPartition, hence this error.

Attached patch fixes that.

I thought we'd need to apply this to 10, 11, HEAD, but I couldn't
reproduce this in 10.  That's because the above commit wasn't applied to
10, so the child that causes this error is being skipped in 10's case.

Maybe, we should apply parts of the above commit that apply to 10 and then
this patch on top.  Attached for-10.patch file does that.

Thanks,
Amit

[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=2fbdf1b38bc
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c145385f84..2d0043d1db 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -14247,9 +14247,6 @@ ATExecAttachPartition(List **wqueue, Relation rel, 
PartitionCmd *cmd)
                                                trigger_name, 
RelationGetRelationName(attachrel)),
                                 errdetail("ROW triggers with transition tables 
are not supported on partitions")));
 
-       /* OK to create inheritance.  Rest of the checks performed there */
-       CreateInheritance(attachrel, rel);
-
        /*
         * 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
@@ -14258,6 +14255,9 @@ ATExecAttachPartition(List **wqueue, Relation rel, 
PartitionCmd *cmd)
        check_new_partition_bound(RelationGetRelationName(attachrel), rel,
                                                          cmd->bound);
 
+       /* OK to create inheritance.  Rest of the checks performed there */
+       CreateInheritance(attachrel, rel);
+
        /* Update the pg_class entry. */
        StorePartitionBound(attachrel, rel, cmd->bound);
 
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 8219d05d83..75e41ed21c 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -195,23 +195,12 @@ 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));
                boundspecs = lappend(boundspecs, boundspec);
                partoids = lappend_oid(partoids, inhrelid);
@@ -1856,8 +1845,7 @@ generate_partition_qual(Relation rel)
                                                                 
Anum_pg_class_relpartbound,
                                                                 &isnull);
        if (isnull)                                     /* should not happen */
-               elog(ERROR, "relation \"%s\" has relpartbound = null",
-                        RelationGetRelationName(rel));
+               elog(ERROR, "null relpartbound for relation %u", 
RelationGetRelid(rel));
        bound = castNode(PartitionBoundSpec,
                                         
stringToNode(TextDatumGetCString(boundDatum)));
        ReleaseSysCache(tuple);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f143101b5d..43b76812d2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -745,9 +745,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
                                                                                
  false,
                                                                                
  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.
@@ -809,6 +806,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
                CommandCounterIncrement();
        }
 
+       /* 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.
@@ -13611,9 +13611,6 @@ ATExecAttachPartition(List **wqueue, Relation rel, 
PartitionCmd *cmd)
                                                trigger_name, 
RelationGetRelationName(attachrel)),
                                 errdetail("ROW triggers with transition tables 
are not supported on partitions")));
 
-       /* OK to create inheritance.  Rest of the checks performed there */
-       CreateInheritance(attachrel, rel);
-
        /*
         * 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
@@ -13622,6 +13619,9 @@ ATExecAttachPartition(List **wqueue, Relation rel, 
PartitionCmd *cmd)
        check_new_partition_bound(RelationGetRelationName(attachrel), rel,
                                                          cmd->bound);
 
+       /* OK to create inheritance.  Rest of the checks performed there */
+       CreateInheritance(attachrel, rel);
+
        /* Update the pg_class entry. */
        StorePartitionBound(attachrel, rel, cmd->bound);
 
@@ -13837,10 +13837,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));

Reply via email to