Alvaro,
* Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
> This is extracted from the DDL deparse series. These patches add
> get_object_address support for the following object types:
>
> - user mappings
> - default ACLs
> - operators and functions of operator families
I took a (relatively quick) look through these patches.
> Subject: [PATCH 1/3] deparse/core: get_object_address support user mappings
[...]
I thought this looked fine. One minor nit-pick is that the function added
doesn't have a single comment, but it's a pretty short too.
> Subject: [PATCH 2/3] deparse/core: get_object_address support default ACLs
[...]
> + char *stuff;
Nit-pick, but 'stuff' isn't really a great variable name. :) Perhaps
'defacltype_name'? It's longer, sure, but it's not used a lot..
> Subject: [PATCH 3/3] deparse/core: get_object_address support opfamily members
> @@ -661,7 +664,8 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
> ObjectAddress domaddr;
> char *constrname;
>
> - domaddr = get_object_address_type(OBJECT_DOMAIN, objname, missing_ok);
> + domaddr = get_object_address_type(OBJECT_DOMAIN,
> + list_head(objname), missing_ok);
> constrname = strVal(linitial(objargs));
>
> address.classId = ConstraintRelationId;
I don't really care for how all the get_object_address stuff uses lists
for arguments instead of using straight-forward arguments, but it's how
it's been done and I can kind of see the reasoning behind it. I'm not
following why you're switching this case (get_object_address_type) to
using a ListCell though..?
I thought the rest of it looked alright. I agree it's a bit odd how the
opfamily is handled but I agree with your assessment that there's not
much better we can do with this object representation.
Thanks!
Stephen