On Wed, Dec 19, 2018 at 01:29:48PM -0500, Tom Lane wrote: > Alvaro Herrera <alvhe...@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.ge27...@paquier.xyz I'll just adapt if need be, or perhaps you will need to patch an extra line depending on the timing ;) -- Michael
signature.asc
Description: PGP signature
