Re: Cache lookup errors with functions manipulation object addresses

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