On Mon, Feb 12, 2018 at 07:57:34PM -0300, Alvaro Herrera wrote:
> Pushed 0001, which was easy enough to deal with.
Thanks.
> I think 0002 and 0003 should be changed similarly: the elog(ERROR)
> code should be inside "if" and the "return NULL" case the straight
> path, rather than the other way around. That seems more robust than
> the compiler relying on knowledge that elog(ERROR) does not return.
OK, I updated the patches to do so and rebased. Those are now 0001 and
0002. For 0002, I have added more adapted comments at the top of
get_publication_name and get_subscription_name.
> As far as format_type_extended() is concerned, IMO we've gone far enough
> with the number of variants of format_type(). Making the function
> public makes sense to me, but let's add a bits32 flags argument instead
> of exposing the messy set of booleans. We can add compatibility
> wrappers for the flag combinations most used in core code, and maybe
> take the opportunity phase out the uncommon ones.
OK, I was a bit hesitant to propose that without more input, so I
definitely agree with this API interface. I have tackled that in 0003,
with the following changes:
- let's get rid of format_type_with_typemod_qualified. This is only
used by postgres_fdw in one place.
- format_type_be_qualified is also rather localized, but I have kept
it. Perhaps this could be nuked as well. Input is welcome.
- let's keep format_type_be and format_type_with_typemod. Those are
largely more spread in the core code, so I don't think that we need to
invade things more than necessary.
Attached is a rebased and updated patch set. I have also reworked the
dance with elog calls and missing_ok to match with what you have already
committed.
--
Michael