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

From Alvaro Herrera
Subject Re: Some RELKIND macro refactoring
Date
Msg-id 202111231509.rjfq4ajv3gsn@alvherre.pgsql
Whole thread Raw
In response to Re: Some RELKIND macro refactoring  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Some RELKIND macro refactoring
List pgsql-hackers
On 2021-Nov-19, Michael Paquier wrote:

> I think that you should keep this block as it is now.  The part where
> a relkind does not support table AMs and does not require storage
> would get uncovered, and this new code would just attempt to create
> storage, so it seems to me that the existing code is better when it
> comes to prevent mistakes from developers?  You could as well use an
> else if (RELKIND_INDEX || RELKIND_SEQUENCE) for
> RelationCreateStorage(), and fall back to Assert(false) in the else
> branch, to get the same result as the original without impacting the
> level of safety of the original code.

Hmm, I think the added condition and else would make it safe and clear:

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

This is the same coding the patch proposed to put in
RelationSetNewRelfilenode, which IMO validates the idea.


In init_fork(), there's a typo:

+        * For tables, the AM callback creates both the main and the init fork.
should read:
+        * For tables, the AM callback creates both the main and the init forks.


In heap_create_with_catalog, you have

+               if (!relid)
should be
+               if (!OidIsValid(relid))


Overall, LGTM.

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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Should rename "startup process" to something else?
Next
From: Alvaro Herrera
Date:
Subject: Re: Some RELKIND macro refactoring