On 12 June 2017 at 17:51, Joe Conway <m...@joeconway.com> wrote: > On 06/12/2017 07:40 AM, Joe Conway wrote: >> On 06/12/2017 01:49 AM, Amit Langote wrote: >>> As he mentioned in his reply, Ashutosh's proposal to abstract away the >>> relkind checks is interesting in this regard. >>> >> I have not looked at Ashutosh's patch yet, but it sounds like a good >> idea to me. > > After looking I remain convinced - +1 in general. >
Yes, I think this will probably help, but I worry that it will turn into quite a large and invasive patch, and there are a number of design choices to be made over the naming and precise set of macros. Is this really PG10 material? My initial thought, looking at the patch, is that it might be better to have all the macros in one file to make them easier to maintain. > One thing I would change -- in many cases there are ERROR messages > enumerating the relkinds. E.g.: > 8<----------------- > if (!RELKIND_CAN_HAVE_COLUMN_COMMENT(relation->rd_rel->relkind)) > ereport(ERROR, > (errcode(ERRCODE_WRONG_OBJECT_TYPE), > errmsg("\"%s\" is not a table, view, materialized view, > composite type, or foreign table", > 8<----------------- > > I think those messages should also be rewritten to make them less > relkind specific and more clear, otherwise we still risk getting out of > sync in the future. Maybe something like this: > 8<----------------- > "\"%s\" is not a kind of relation that can have column comments" > 8<----------------- > Thoughts? > -1. I think the existing error messages provide useful information that you'd be removing. If you're worried about the error messages getting out of sync, then wouldn't it be better to centralise them along with the corresponding macros? > In any case, unless another committer else wants it, and assuming no > complaints with the concept, I'd be happy to take this. > OK, have at it. Barring objections, I'll push my original patch and work up patches for the other couple of issues I found. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers