Subject: Re: Add pg_get_acl() function get the ACL for a database object

From: "Joel Jacobson" <[email-masked]>
Date: Wed, 19 Jun 2024 15:26:03 +0200

Hi Ranier,

Thanks for looking at this.

I've double-checked the patch I sent, and it works fine.

I think I know the cause of your problem:

Since this is a catalog change, you need to run `make clean`, to ensure the catalog is rebuilt,
followed by the usual `make && make install`.

You also need to run `initdb` to create a new database cluster, with the new catalog version.

Let me know if you need more specific instructions.

Best,
Joel

On Wed, Jun 19, 2024, at 14:59, Ranier Vilela wrote:
> Em qua., 19 de jun. de 2024 às 08:35, Joel Jacobson <[email-masked]> 
> escreveu:
>> Hello hackers,
>> 
>> Currently, obtaining the Access Control List (ACL) for a database object
>> requires querying specific pg_catalog tables directly, where the user
>> needs to know the name of the ACL column for the object.
>> 
>> Consider:
>> 
>> ```
>> CREATE USER test_user;
>> CREATE USER test_owner;
>> CREATE SCHEMA test_schema AUTHORIZATION test_owner;
>> SET ROLE TO test_owner;
>> CREATE TABLE test_schema.test_table ();
>> GRANT SELECT ON TABLE test_schema.test_table TO test_user;
>> ```
>> 
>> To get the ACL we can do:
>> 
>> ```
>> SELECT relacl FROM pg_class WHERE oid = 'test_schema.test_table'::regclass::oid;
>> 
>>                          relacl
>> ---------------------------------------------------------
>>  {test_owner=arwdDxtm/test_owner,test_user=r/test_owner}
>> ```
>> 
>> Attached patch adds a new SQL-callable functoin `pg_get_acl()`, so we can do:
>> 
>> ```
>> SELECT pg_get_acl('pg_class'::regclass, 'test_schema.test_table'::regclass::oid);
>>                        pg_get_acl
>> ---------------------------------------------------------
>>  {test_owner=arwdDxtm/test_owner,test_user=r/test_owner}
>> ```
>> 
>> The original idea for this function came from Alvaro Herrera,
>> in this related discussion:
>> https://postgr.es/m/[email-masked]
>> 
>> On Thu, Mar 25, 2021, at 16:16, Alvaro Herrera wrote:
>> > On 2021-Mar-25, Joel Jacobson wrote:
>> >
>> >> pg_shdepend doesn't contain the aclitem info though,
>> >> so it won't work for pg_permissions if we want to expose
>> >> privilege_type, is_grantable and grantor.
>> >
>> > Ah, of course -- the only way to obtain the acl columns is by going
>> > through the catalogs individually, so it won't be possible.  I think
>> > this could be fixed with some very simple, quick function pg_get_acl()
>> > that takes a catalog OID and object OID and returns the ACL; then
>> > use aclexplode() to obtain all those details.
>> 
>> The pg_get_acl() function has been implemented by following
>> the guidance from Alvaro in the related dicussion:
>> 
>> On Fri, Mar 26, 2021, at 13:43, Alvaro Herrera wrote:
>> > AFAICS the way to do it is like AlterObjectOwner_internal obtains data
>> > -- first do get_catalog_object_by_oid (gives you the HeapTuple that
>> > represents the object), then
>> > heap_getattr( ..., get_object_attnum_acl(), ..), and there you have the
>> > ACL which you can "explode" (or maybe just return as-is).
>> >
>> > AFAICS if you do this, it's just one cache lookups per object, or
>> > one indexscan for the cases with no by-OID syscache.  It should be much
>> > cheaper than the UNION ALL query.  And you use pg_shdepend to guide
>> > this, so you only do it for the objects that you already know are
>> > interesting.
>> 
>> Many thanks Alvaro for the very helpful instructions.
>> 
>> This function would then allow users to e.g. create a view to show the privileges
>> for all database objects, like the pg_privileges system view suggested in the
>> related discussion.
>> 
>> Tests and docs are added.
> Hi,
> For some reason, the function pg_get_acl, does not exist in generated fmgrtab.c
>
> So, when install postgres, the function does not work.
>
> postgres=# SELECT pg_get_acl('pg_class'::regclass, 
> 'atest2'::regclass::oid);
> ERROR:  function pg_get_acl(regclass, oid) does not exist
> LINE 1: SELECT pg_get_acl('pg_class'::regclass, 'atest2'::regclass::...
>                ^
> HINT:  No function matches the given name and argument types. You might 
> need to add explicit type casts.
>
> best regards,
> Ranier Vilela

-- 
Kind regards,

Joel

---

From: Ranier Vilela <[email-masked]>
Date: Wed, 19 Jun 2024 10:28:37 -0300

> Let me know if you need more specific instructions.
Sorry, sorry but I'm on Windows -> meson.

Double checked with:
ninja clean
ninja
ninja install

best regards,
Ranier Vilela

---

From: Ranier Vilela <[email-masked]>
Date: Wed, 19 Jun 2024 10:51:32 -0300

> Sorry, sorry but I'm on Windows -> meson.
Sorry for the noise, now pg_get_acl is shown in the regress test.

Regarding the patch, could it be written in the following style?

Datum
pg_get_acl(PG_FUNCTION_ARGS)
{
Oid classId = PG_GETARG_OID(0);
Oid objectId = PG_GETARG_OID(1);
Oid catalogId;
AttrNumber Anum_oid;
AttrNumber Anum_acl;

/* for "pinned" items in pg_depend, return null */
if (!OidIsValid(classId) && !OidIsValid(objectId))
PG_RETURN_NULL();

catalogId = (classId == LargeObjectRelationId) ?
LargeObjectMetadataRelationId : classId;
Anum_oid = get_object_attnum_oid(catalogId);
Anum_acl = get_object_attnum_acl(catalogId);

if (Anum_acl != InvalidAttrNumber)
{
Relation rel;
HeapTuple tup;
Datum datum;
bool isnull;

rel = table_open(catalogId, AccessShareLock);

tup = get_catalog_object_by_oid(rel, Anum_oid, objectId);
if (!HeapTupleIsValid(tup))
elog(ERROR, "cache lookup failed for object %u of catalog \"%s\"",
objectId, RelationGetRelationName(rel));

datum = heap_getattr(tup, Anum_acl, RelationGetDescr(rel), &isnull);

table_close(rel, AccessShareLock);

if (!isnull)
PG_RETURN_DATUM(datum);
}

PG_RETURN_NULL();
}

best regards,
Ranier Vilela

---

From: "Joel Jacobson" <[email-masked]>
Date: Wed, 19 Jun 2024 16:21:43 +0200

> Regarding the patch, could it be written in the following style?

Thanks for nice improvement. New version attached.

Best,
Joel