Re: Some RELKIND macro refactoring

On Mon, Nov 22, 2021 at 11:21:52AM +0100, Peter Eisentraut wrote:
> On 19.11.21 08:31, Michael Paquier wrote:
>> Regarding 0001, I find the existing code a bit more self-documenting
>> if we keep those checks flagInhAttrs() and guessConstraintInheritance().
>> So I would rather leave these.
> 
> In that case, the existing check in guessConstraintInheritance() seems
> wrong, because it doesn't check for RELKIND_MATVIEW.  Should we fix that?
> It's dead code either way, but if the code isn't exercises, then these kinds
> of inconsistency come about.

Yeah, this one could be added.  Perhaps that comes down to one's taste
at the end, but I would add it.

> Maybe
> 
>     else
>     {
>         Assert(RELKIND_HAS_STORAGE(rel->rd_rel->relkind);
>         RelationCreateStorage(rel->rd_node, relpersistence);
>     }
> 
> create_storage is set earlier based on RELKIND_HAS_STORAGE(), so this would
> be consistent.

Sounds fine by me.  Perhaps you should apply the same style in
RelationGetNumberOfBlocksInFork(), then?
--
Michael