Re: [HACKERS] Cache lookup errors with functions manipulation object addresses

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