Re: Macros bundling RELKIND_* conditions

On Wed, Aug 2, 2017 at 11:15 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> Alvaro Herrera wrote:
>> I think pg_class is a reasonable place to put more generic relkind lists
>> alongside a matching error message for each, rather than specialized
>> "does this relkind have storage" macros. What about something like a
>> struct list in pg_class.h,
>
> I just noticed that this doesn't help at all with the initial problem
> statement, which is that some of the relkind checks failed to notice
> that partitioned tables needed to be added to the set. Maybe it still
> helps because you have something to grep for, as Tom proposed elsewhere.

Having something like relkind_i_t_T, though correct, doesn't make the
test readable. That's another improvement because of using such
macros. The macro name tells the purpose of the test, which is what a
reader would be interested in, rather than the actual values used.

>
> However, if there are multiple places that should be kept in sync
> regarding which relkinds to check, then I don't understand Robert's
> objection that only one place requires the check. Surely we're having
> this discussion precisely because more than one place needs the check,
> and finding those places is not obvious?
>

A new relkind may be required to be added in tests at multiple places.
But each place may have different set of relkinds in that test, which
gets converted into a macro. Given that we have now 9 relkinds, we
could theoretically have 2^9 tests and those many macros. Maintaining
all those would be more cumbersome than grepping RELKIND_ in code.
From that perspective Robert's concern is valid, but my feeling is
that we are actually using far lesser combinations than that. As I
said, I haven't gone through all the places, so, I don't know the
whole picture yet. But given the number of places where we added
RELKIND_PARTITIONED_TABLE, I guess, there are more than one places
where at least some of those tests are used.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company