Re: Cache lookup errors with functions manipulation object addresses

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Moshe Jacobson <moshe(at)neadwerx(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Subject: Re: Cache lookup errors with functions manipulation object addresses
Date: 2017-07-20 13:59:37
Message-ID: CAB7nPqRTUOJhZdwSNjn7MvT-mJrBhv7ax-y5==tZ4vP+ggmBog@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jul 19, 2017 at 7:29 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Jul 19, 2017 at 2:25 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> Would we want to improve the error handling of such objects?
>
> +1 for such an improvement.

Attached is a patch for all that. Here are some notes:
- format_type_be and friends use allow_invalid. I have added a flag to
control the NULL-ness as many code paths rely on existing APIs, and
introduced an _extended version of this API. I would argue for the
removal of allow_invalid to give more flexibility to callers, but this
impacts extensions :(
- A similar thing is needed for format_operator().
- We could really add a missing_ok to get_attname, but that does not
seem worth the refactoring with modules potentially calling it..
- GetForeignDataWrapper is extended with a missing_ok, unfortunately
not saving one cache lookup in GetForeignDataWrapperByName.
- Same remark as the previous one for GetForeignServer.
- get_publication_name and get_subscription_name gain a missing_ok.
- getObjectDescription and getObjectIdentity are called in quite a
couple of places. We could have those have a kind of missing_ok, but
as the status is just for adding cache lookup errors I have kept the
interface simple as this keeps the code in objectaddress.c more simple
as well. getObjectIdentity is used mainly in sepgsql, which I have not
compiled yet so I may have missed something :) getObjectDescription is
used in more places in the backend code, but I am not much into
complicating the objaddr API with this patch more.
- I have added tests for all the OCLASS objects, for a total more or
less 120 cache lookup errors that a user can face.
- Some docs are present as well, but I think that they are a bit
incomplete. I'll review them a bit later.
- The patch is large, 800 lines are used for the tests which is very mechanical:
32 files changed, 1721 insertions(+), 452 deletions(-)

Thanks,
--
Michael

Attachment Content-Type Size
objaddr_nullness_v1.patch.gz application/x-gzip 20.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2017-07-20 14:04:02 Re: Cache lookup errors with functions manipulation object addresses
Previous Message Tom Lane 2017-07-20 13:53:23 Re: pg_upgrade failed if view contain natural left join condition