On Fri, Jul 05, 2024 at 10:40:39AM +0200, Joel Jacobson wrote: > 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.
Thanks for the patch. I have been looking at it for a few hours,
eyeing a bit on the ObjectProperty parts a bit if we were to extend it
for sub-object IDs, and did not like the complexity this introduces,
so I'd be OK to live with the extra handling in pg_get_acl() itself.
+ /* ignore dropped columns */
+ if (atttup->attisdropped)
+ {
+ ReleaseSysCache(tup);
+ PG_RETURN_NULL();
+ }
Hmm. This is an important bit and did not consider it first. That
makes the use of ObjectProperty less flexible because we want to look
at the data in the pg_attribute tuple to be able to return NULL in
this case.
I've tweaked a bit what you are proposing, simplifying the code and
removing the first batch of queries in the tests as these were less
interesting. How does that look?
--
Michael
From 07b65a78fb660f6a27ef464870a3e27f788c5e28 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 8 Jul 2024 16:24:42 +0900
Subject: [PATCH v2] Extend pg_get_acl to handle sub-object IDs.
This patch modifies the pg_get_acl function to accept a third objsubid
param. This enables the retrieval of ACLs for columns within a table.
---
src/include/catalog/pg_proc.dat | 2 +-
src/backend/catalog/objectaddress.c | 60 +++++++++++++++++++-----
src/test/regress/expected/privileges.out | 32 +++++++++++--
src/test/regress/sql/privileges.sql | 12 +++--
doc/src/sgml/func.sgml | 6 +--
5 files changed, 87 insertions(+), 25 deletions(-)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 0bf413fe05..0a0f6aa198 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6364,7 +6364,7 @@
{ oid => '8730', descr => 'get ACL for SQL object',
proname => 'pg_get_acl', provolatile => 's', prorettype => '_aclitem',
- proargtypes => 'oid oid', proargnames => '{classid,objid}',
+ proargtypes => 'oid oid int4', proargnames => '{classid,objid,objsubid}',
prosrc => 'pg_get_acl' },
{ oid => '3839',
diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 2983b9180f..fac3922964 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -4364,19 +4364,19 @@ pg_identify_object_as_address(PG_FUNCTION_ARGS)
/*
* SQL-level callable function to obtain the ACL of a specified object, given
- * its catalog OID and object OID.
+ * its catalog OID, object OID and sub-object ID.
*/
Datum
pg_get_acl(PG_FUNCTION_ARGS)
{
Oid classId = PG_GETARG_OID(0);
Oid objectId = PG_GETARG_OID(1);
+ int32 objsubid = PG_GETARG_INT32(2);
Oid catalogId;
AttrNumber Anum_acl;
- Relation rel;
- HeapTuple tup;
Datum datum;
bool isnull;
+ HeapTuple tup;
/* for "pinned" items in pg_depend, return null */
if (!OidIsValid(classId) && !OidIsValid(objectId))
@@ -4391,18 +4391,52 @@ pg_get_acl(PG_FUNCTION_ARGS)
if (Anum_acl == InvalidAttrNumber)
PG_RETURN_NULL();
- rel = table_open(catalogId, AccessShareLock);
-
- tup = get_catalog_object_by_oid(rel, get_object_attnum_oid(catalogId),
- objectId);
- if (!HeapTupleIsValid(tup))
+ /*
+ * If dealing with a relation's attribute (objsubid is set), the ACL is
+ * retrieved from pg_attribute.
+ */
+ if (classId == RelationRelationId && objsubid != 0)
{
- table_close(rel, AccessShareLock);
- PG_RETURN_NULL();
- }
+ AttrNumber attnum = (AttrNumber) objsubid;
+ Form_pg_attribute atttup;
- datum = heap_getattr(tup, Anum_acl, RelationGetDescr(rel), &isnull);
- table_close(rel, AccessShareLock);
+ tup = SearchSysCache2(ATTNUM, ObjectIdGetDatum(objectId),
+ Int16GetDatum(attnum));
+
+ if (!HeapTupleIsValid(tup))
+ PG_RETURN_NULL();
+
+ atttup = (Form_pg_attribute) GETSTRUCT(tup);
+
+ /* ignore dropped columns */
+ if (atttup->attisdropped)
+ {
+ ReleaseSysCache(tup);
+ PG_RETURN_NULL();
+ }
+
+ datum = SysCacheGetAttr(ATTNUM, tup,
+ Anum_pg_attribute_attacl,
+ &isnull);
+ ReleaseSysCache(tup);
+ }
+ else
+ {
+ Relation rel;
+
+ rel = table_open(catalogId, AccessShareLock);
+
+ tup = get_catalog_object_by_oid(rel, get_object_attnum_oid(catalogId),
+ objectId);
+ if (!HeapTupleIsValid(tup))
+ {
+ table_close(rel, AccessShareLock);
+ PG_RETURN_NULL();
+ }
+
+ datum = heap_getattr(tup, Anum_acl, RelationGetDescr(rel), &isnull);
+ table_close(rel, AccessShareLock);
+ }
if (isnull)
PG_RETURN_NULL();
diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out
index 332bc584eb..fab0cc800f 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -213,7 +213,7 @@ SELECT * FROM atest1;
(0 rows)
CREATE TABLE atest2 (col1 varchar(10), col2 boolean);
-SELECT pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid);
+SELECT pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid, 0);
pg_get_acl
------------
@@ -223,7 +223,7 @@ GRANT SELECT ON atest2 TO regress_priv_user2;
GRANT UPDATE ON atest2 TO regress_priv_user3;
GRANT INSERT ON atest2 TO regress_priv_user4 GRANTED BY CURRENT_USER;
GRANT TRUNCATE ON atest2 TO regress_priv_user5 GRANTED BY CURRENT_ROLE;
-SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid));
+SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid, 0));
unnest
------------------------------------------------
regress_priv_user1=arwdDxtm/regress_priv_user1
@@ -234,13 +234,13 @@ SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid));
(5 rows)
-- Invalid inputs
-SELECT pg_get_acl('pg_class'::regclass, 0); -- null
+SELECT pg_get_acl('pg_class'::regclass, 0, 0); -- null
pg_get_acl
------------
(1 row)
-SELECT pg_get_acl(0, 0); -- null
+SELECT pg_get_acl(0, 0, 0); -- null
pg_get_acl
------------
@@ -653,6 +653,30 @@ CREATE TABLE atest5 (one int, two int unique, three int, four int unique);
CREATE TABLE atest6 (one int, two int, blue int);
GRANT SELECT (one), INSERT (two), UPDATE (three) ON atest5 TO regress_priv_user4;
GRANT ALL (one) ON atest5 TO regress_priv_user3;
+SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest5'::regclass::oid, 1));
+ unnest
+--------------------------------------------
+ regress_priv_user4=r/regress_priv_user1
+ regress_priv_user3=arwx/regress_priv_user1
+(2 rows)
+
+SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest5'::regclass::oid, 2));
+ unnest
+-----------------------------------------
+ regress_priv_user4=a/regress_priv_user1
+(1 row)
+
+SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest5'::regclass::oid, 3));
+ unnest
+-----------------------------------------
+ regress_priv_user4=w/regress_priv_user1
+(1 row)
+
+SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest5'::regclass::oid, 4));
+ unnest
+--------
+(0 rows)
+
INSERT INTO atest5 VALUES (1,2,3);
SET SESSION AUTHORIZATION regress_priv_user4;
SELECT * FROM atest5; -- fail
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql
index 980d19bde5..ae338e8cc8 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -183,16 +183,16 @@ GRANT SELECT ON atest1 TO regress_priv_user3, regress_priv_user4;
SELECT * FROM atest1;
CREATE TABLE atest2 (col1 varchar(10), col2 boolean);
-SELECT pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid);
+SELECT pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid, 0);
GRANT SELECT ON atest2 TO regress_priv_user2;
GRANT UPDATE ON atest2 TO regress_priv_user3;
GRANT INSERT ON atest2 TO regress_priv_user4 GRANTED BY CURRENT_USER;
GRANT TRUNCATE ON atest2 TO regress_priv_user5 GRANTED BY CURRENT_ROLE;
-SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid));
+SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest2'::regclass::oid, 0));
-- Invalid inputs
-SELECT pg_get_acl('pg_class'::regclass, 0); -- null
-SELECT pg_get_acl(0, 0); -- null
+SELECT pg_get_acl('pg_class'::regclass, 0, 0); -- null
+SELECT pg_get_acl(0, 0, 0); -- null
GRANT TRUNCATE ON atest2 TO regress_priv_user4 GRANTED BY regress_priv_user5; -- error
@@ -439,6 +439,10 @@ CREATE TABLE atest5 (one int, two int unique, three int, four int unique);
CREATE TABLE atest6 (one int, two int, blue int);
GRANT SELECT (one), INSERT (two), UPDATE (three) ON atest5 TO regress_priv_user4;
GRANT ALL (one) ON atest5 TO regress_priv_user3;
+SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest5'::regclass::oid, 1));
+SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest5'::regclass::oid, 2));
+SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest5'::regclass::oid, 3));
+SELECT unnest(pg_get_acl('pg_class'::regclass, 'atest5'::regclass::oid, 4));
INSERT INTO atest5 VALUES (1,2,3);
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 93ee3d4b60..aad944979f 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -26592,12 +26592,12 @@ SELECT currval(pg_get_serial_sequence('sometable', 'id'));
<indexterm>
<primary>pg_get_acl</primary>
</indexterm>
- <function>pg_get_acl</function> ( <parameter>classid</parameter> <type>oid</type>, <parameter>objid</parameter> <type>oid</type> )
+ <function>pg_get_acl</function> ( <parameter>classid</parameter> <type>oid</type>, <parameter>objid</parameter> <type>oid</type>, <parameter>objsubid</parameter> <type>integer</type> )
<returnvalue>aclitem[]</returnvalue>
</para>
<para>
Returns the <acronym>ACL</acronym> for a database object, specified
- by catalog OID and object OID. This function returns
+ by catalog OID, object OID and sub-object ID. This function returns
<literal>NULL</literal> values for undefined objects.
</para></entry>
</row>
@@ -26723,7 +26723,7 @@ SELECT currval(pg_get_serial_sequence('sometable', 'id'));
<programlisting>
postgres=# SELECT
(pg_identify_object(s.classid,s.objid,s.objsubid)).*,
- pg_catalog.pg_get_acl(s.classid,s.objid) AS acl
+ pg_catalog.pg_get_acl(s.classid,s.objid,s.objsubid) AS acl
FROM pg_catalog.pg_shdepend AS s
JOIN pg_catalog.pg_database AS d
ON d.datname = current_database() AND
--
2.45.2
signature.asc
Description: PGP signature
