Re: Some RELKIND macro refactoring - Mailing list pgsql-hackers

On 2021-Aug-16, Peter Eisentraut wrote:

> +        if (rel->rd_rel->relkind == RELKIND_INDEX ||
> +            rel->rd_rel->relkind == RELKIND_SEQUENCE)
> +            RelationCreateStorage(rel->rd_node, relpersistence);
> +        else if (RELKIND_HAS_TABLE_AM(rel->rd_rel->relkind))
> +            table_relation_set_new_filenode(rel, &rel->rd_node,
> +                                            relpersistence,
> +                                            relfrozenxid, relminmxid);
> +        else
> +            Assert(false);

I think you could turn this one (and the one in RelationSetNewRelfilenode) around:

    if (RELKIND_HAS_TABLE_AM(rel->rd_rel->relkind))
        table_relation_set_new_filenode(...);
    else if (RELKIND_HAS_STORAGE(rel->rd_rel->relkind))
        RelationCreateStorage(...);

> +/*
> + * Relation kinds with a table access method (rd_tableam).  Although sequences
> + * use the heap table AM, they are enough of a special case in most uses that
> + * they are not included here.
> + */
> +#define RELKIND_HAS_TABLE_AM(relkind) \
> +    ((relkind) == RELKIND_RELATION || \
> +     (relkind) == RELKIND_TOASTVALUE || \
> +     (relkind) == RELKIND_MATVIEW)

Partitioned tables are not listed here, but IIRC there's a patch to let
partitioned tables have a table AM so that their partitions inherit it.
I'm wondering if that patch is going to have to change this spot; and if
it does, how will that affect the callers of this macro that this patch
creates.  I think a few places assume that HAS_TABLE_AM means that the
table has storage, but perhaps it would be better to spell that out
explicitly:

@@ -303,9 +303,7 @@ verify_heapam(PG_FUNCTION_ARGS)
     /*
      * Check that a relation's relkind and access method are both supported.
      */
-    if (ctx.rel->rd_rel->relkind != RELKIND_RELATION &&
-        ctx.rel->rd_rel->relkind != RELKIND_MATVIEW &&
-        ctx.rel->rd_rel->relkind != RELKIND_TOASTVALUE)
+    if (!(RELKIND_HAS_TABLE_AM(ctx.rel->rd_rel->relkind) && RELKIND_HAS_STOAGE(ctx.rel->rd_rel->relkind)))
         ereport(ERROR,
                 (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                  errmsg("cannot check relation \"%s\"",

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/