Re: Some RELKIND macro refactoring

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/