Re: Cache lookup errors with functions manipulation object addresses

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Cache lookup errors with functions manipulation object addresses
Date: 2018-09-18 05:51:42
Message-ID: 20180918055142.GK31460@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Fri, Sep 14, 2018 at 12:07:23PM -0300, Alvaro Herrera wrote:
> On 2018-Sep-14, Alvaro Herrera wrote:
>> I haven't looked at 0003 yet.

Thanks for the review. I have pushed 0002 for now. I had one doubt
about 0001 though. So as to avoid redesigning the APIs for FDWs and
servers again in the future, wouldn't we want to give them a uint32
flags which can be in with more than one option. There would be only
one option for now, which is what I have done in the attached.

> It's strange that pg_identify_object returns empty type in only some
> cases (as seen in the regression test output) ...

Are you referring to something in particular? All the routines used in
objectaddress.c are part of what exists in core for some time now,
particularly handling for operators, functions and types has been sort
of messy. It is of course possible to reach a state in the code where
we have options to get NULL results for all those things. For example,
format_procedure_internal could be refactored in a way similar to what
happens for format_type_extended. And format_type_extended could gain a
FORCE_NULL flag which makes sure that on an undefined object
objectaddress.c sees a NULL object. That was one of the points raised
upthread that I still wanted to discuss.. One thing is that I am not
sure if it is worth complicating the existing interface even more just
to deal with undefined objects as pure NULLs. Thoughts are welcome.

> And this one definitely does not make sense:
>
> +SELECT * FROM pg_identify_object('pg_class'::regclass, 'pg_class'::regclass, -8); -- no column for relation
> + type | schema | name | identity
> +--------------+------------+----------+---------------------
> + table column | pg_catalog | pg_class | pg_catalog.pg_class
> +(1 row)

Indeed, this one is not intentional, and can be easily addressed by
checking first for the attribute existence. This is corrected in the
attached version.
--
Michael

Attachment Content-Type Size
0001-Extend-lookup-routines-for-FDW-and-foreign-server-wi.patch text/x-diff 6.0 KB
0002-Eliminate-user-visible-cache-lookup-errors-for-objad.patch text/x-diff 116.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2018-09-18 06:34:57 Re: Problem while setting the fpw with SIGHUP
Previous Message amul sul 2018-09-18 05:49:44 Re: Multiple primary key on partition table?