Re: Cache lookup errors with functions manipulation object addresses

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
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-12-12 17:21:29
Message-ID: 20181212172129.coqcoifrbjuiwpbg@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2018-Sep-18, Michael Paquier wrote:

> 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.

Agreed.

I think defining bit flags in an enum is weird; I'd use defines instead.
And (a purely stylistic point) I'd use bits32 rather than uint32. (I'm
probably alone in this opinion, since almost every interface passing
flags uses unsigned int of some size. But "bits" types are defined
precisely for this purpose!) Compare a61f5ab98638.

I support 0001 other than those two points.

> > 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?

Yeah, these two cases:

+-- Keep those checks in the same order as getObjectTypeDescription()
+SELECT * FROM pg_identify_object_as_address('pg_class'::regclass, 0, 0); -- no relation
+ type | object_names | object_args
+------+--------------+-------------
+ | |
+(1 row)

+SELECT * FROM pg_identify_object_as_address('pg_proc'::regclass, 0, 0); -- no function
+ type | object_names | object_args
+------+--------------+-------------
+ | {} | {}
+(1 row)

All the other cases you add have a non-empty value in "type".

> 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.

I think this is our chance to fix the mess. Previously (before I added
the SQL-access of the object addressing mechanism in 9.5 I mean) it
wasn't possible to get at this code at all with arbitrary input, so this
is all a "new" problem (I mean new in the last 5 years).

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2018-12-12 17:48:35 Re: Reorganize collation lookup time and place
Previous Message Tom Lane 2018-12-12 15:57:07 Upgrading pg_statistic to handle collation honestly