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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Cache lookup errors with functions manipulation object addresses
Date: 2018-02-13 03:51:07
Message-ID: 20180213035107.GA2915@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Extend-lookup-routines-for-FDW-and-foreign-server-wi.patch text/x-diff 14.5 KB
0002-Refactor-routines-for-subscription-and-publication-l.patch text/x-diff 6.0 KB
0003-Refactor-format_type-APIs-to-be-more-modular.patch text/x-diff 8.0 KB
0004-Eliminate-user-visible-cache-lookup-errors-for-objad.patch text/x-diff 119.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-02-13 03:52:38 Re: rename sgml files?
Previous Message Peter Eisentraut 2018-02-13 03:40:34 Re: Typo in origin.c