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