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-12-13 05:55:07 |
Message-ID: | 20181213055507.GH9437@paquier.xyz |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Dec 12, 2018 at 02:21:29PM -0300, Alvaro Herrera wrote:
> I think defining bit flags in an enum is weird; I'd use defines
> instead.
Okay, I am fine with that.
> 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.
Fine with that as well, let's do as you suggest then.
> I support 0001 other than those two points.
Attached is an updated version for that as 0001. Thanks for the
review. Does that part look good to you now?
> 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)
"type" should show "relation" here, yes.
> +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".
On this one I would expect NULL for object_names and object_args, as
empty does not make sense for an undefined object, still "type" should
print... "type".
+SELECT * FROM pg_identify_object_as_address('pg_constraint'::regclass, 0, 0); -- no constraint
+ type | object_names | object_args
+------+--------------+-------------
+ | |
+(1 row)
Constraints also are empty.
+SELECT * FROM pg_identify_object_as_address('pg_largeobject'::regclass, 0, 0); -- no large object, no error
+ type | object_names | object_args
+--------------+--------------+-------------
+ large object | {0} | {}
+(1 row)
Large objects should return NULL for the last two fields.
There are a couple of other inconsistent cases with the existing sub-APIs:
+SELECT * FROM pg_identify_object_as_address('pg_type'::regclass, 0, 0); -- no type
+ type | object_names | object_args
+------+--------------+-------------
+ type | {-} | {}
+(1 row)
Here I would expect NULL for object_names and object_args.
> 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).
This requires a bit more work with the low-level APIs grabbing the
printed information though. And I think that the following guidelines
make sense to work on as a base definition for undefined objects:
- pg_identify_object returns the object type name, NULL for the other fields.
- pg_describe_object returns just NULL.
- pg_identify_object_as_address returns the object type name and NULL
for the other fields.
There is some more refactoring work still needed for constraints, large
objects and functions, in a way similar to a26116c6. I am pretty happy
with the shape of 0001, so this could be applied, 0002 still needs to be
reworked so as all undefined object types behave as described above in a
consistent manner. Do those definitions make sense?
--
Michael
Attachment | Content-Type | Size |
---|---|---|
0001-Introduce-new-extended-routines-for-FDW-and-foreign-.patch | text/x-diff | 6.1 KB |
0002-Eliminate-user-visible-cache-lookup-errors-for-objad.patch.gz | application/gzip | 16.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Kyotaro HORIGUCHI | 2018-12-13 05:59:00 | Re: Pluggable Storage - Andres's take |
Previous Message | Alexander Korotkov | 2018-12-13 04:28:30 | Re: gist microvacuum doesn't appear to care about hot standby? |