On Wed, Dec 19, 2018 at 01:29:48PM -0500, Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>> I put the new function at the bottom of heapam.c but I think it probably
>> needs a better place.
>
> catalog/catalog.c contains some functions with roughly this kind of
> knowledge, so maybe there?
+1 for the proposed wrapper. errdetail_unsuitable_relkind() could be
used in a couple of in-core modules on top of the rest.
> Also, please declare the argument as "const char *relname"; and
> shouldn't we use quotes around the relname in the messages?
Yes, these should be quoted.
>> BTW are there other opinions on the RELKIND_HAS_STORAGE vs.
>> RELKIND_CAN_HAVE_STORAGE debate? I'm inclined to change it to the
>> former.
>
> I like RELKIND_HAS_STORAGE. The other seems to imply that some rels
> of a relkind have storage and others don't, which seems like a mess.
> (Although maybe foreign tables would act like that?)
RELKIND_CAN_HAVE_STORAGE implies a possibility of not having storage,
which does not sound right for this stuff.
Please note that I have a patch making use of RELKIND_CAN_HAVE_STORAGE
on my stack:
https://www.postgresql.org/message-id/20181220003140.GE27104@paquier.xyz
I'll just adapt if need be, or perhaps you will need to patch an extra
line depending on the timing ;)
--
Michael