On Fri, Jul 5, 2024, at 01:18, Michael Paquier wrote:
> I would still stick to only one function, with arguments coming from
> scanning pg_[sh]depend.
>
>> I found some code in aclchk.c on line 4452-4468 that seems useful,
>> but not sure. Maybe there is some other existing code that is better
>> as an inspiration?
>
> My first feelings about that was that subobjids are only used for
> pg_attribute, so if we use them as a base, it would look like:
> - Adding a new AttrNumber in ObjectProperty to track to which column
> the subobjid should apply and its catcache number (ATTNUM for the
> attribute number for fast lookup).
> - Extend get_catalog_object_by_oid() with more data: the attribute
> column for the subobjid scan stored in ObjectProperty, the subobjid
> value given by the caller. If get_catalog_object_by_oid()'s caller
> defines a subobjid, use get_object_catcache_oid() on it over the main
> OID one. If it cannot be found in the cache, use a scan key based on
> the class OID and the subobjid.
>
>> I guess we need to handle the RelationRelationId separately,
>> and handle all other classes using the current code in pg_get_acl()?
>
> But most of that simply blows away because we track the dependency to
> the attribute ACLs in pg_shdepend based on pg_class, not pg_attribute.
> get_attname() itself relies on ATTNUM, so perhaps that's OK to just
> add an exception based on RelationRelationId in the code path of
> pg_get_acl(). That makes me a bit uncomfortable to derive more from
> the other routines for the object descriptions, but we've been living
> with that in getObjectDescription() for years now, as well.
OK, I made an attempt to implement this, based on adapting code from
recordExtObjInitPriv() in aclchk.c, which retrieves ACL based on ATTNUM.
There are now two different code paths for columns and non-columns.
It sounds like a bigger refactoring in this area would be nice,
which would enable cleaning up code in other functions as well,
but maybe that's better to do as a separate project.
I've also updated func.sgml, adjusted pg_proc.dat, and added
regression tests for column privileges.
Regards,
Joel
