Re: Towards easier AMs: Cleaning up inappropriate use of name "relkind"

This patch is way too large. Probably in part explained by the fact
that you seem to have run pgindent and absorbed a lot of unrelated
changes. This makes the patch essentially unreviewable.

I think you should define a RelationGetRelkind() static function that
returns the appropriate datatype without requiring a cast and assert in
every single place that processes a relation's relkind. Similarly
you've chosen to leave get_rel_relkind untouched, but that seems unwise.

I think the chr_ macros are pointless.

Reading back the thread, it seems that the whole point of your patch was
to change the tests that currently use 'if' tests to switch blocks. I
cannot understand what's the motivation for that, but it appears to me
that the approach is backwards: I'd counsel to *first* change the APIs
(get_rel_relkind and defining an enum, plus adding RelationGetRelkind)
so that everything is more sensible and safe, including appropriate
answers for the places where an "invalid" relkind is returned; and once
that's in place, replace if tests with switch blocks where it makes
sense to do so.

Also, I suggest that this thread is not a good one for this patch.
Subject is entirely not appropriate. Open a new thread perhaps?

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services