Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 5 Jul 2024 08:18:33 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Joel Jacobson <joel@compiler.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
	Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pgsql: Add pg_get_acl() to get the ACL for a database object
Message-ID: <ZoctyZzE3UlUSer_@paquier.xyz>
References: <E1sPHWu-000GPc-0z@gemulon.postgresql.org>
 <3408967.1720107891@sss.pgh.pa.us>
 <f2688fb7-add6-43da-b1df-97312a451893@app.fastmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="6UQK2KDtdANUlsli"
Content-Disposition: inline
In-Reply-To: <f2688fb7-add6-43da-b1df-97312a451893@app.fastmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ZoctyZzE3UlUSer_%40paquier.xyz>
Precedence: bulk


--6UQK2KDtdANUlsli
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

(Moving to -hackers)

On Thu, Jul 04, 2024 at 10:53:49PM +0200, Joel Jacobson wrote:
> On Thu, Jul 4, 2024, at 17:44, Tom Lane wrote:
>> Uh, why is it defined like that rather than allowing a subobject?
>> This definition is unable to fetch column-specific ACLs.

Yes, I was wondering about that as well yesterday when looking at the
patch, and saw that this was already a pretty good cut as it covers
most of the objects types and does 90% of the job, so just moved on.

> I wonder if it would be motivated to provide overloads for this function,
> and perhaps even for pg_get_object_address and pg_identify_object_as_address?
>
> That is, two param versions (class OID and object OID),
> and three param versions that in addition also take subobject ID.

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.
--
Michael

--6UQK2KDtdANUlsli
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmaHLckACgkQnvQgOdby
QH29XA//f7QhyjJRfc3VWqZltS9of6pJPpfaihPsAttlkZq570h7SsYdLvNo/PB5
73Bfip1kg3M6Z8yl7/BDRe1huBOgzFWCjT2xCeMVVYy4x5i9myN1Sa82W3qC3yrg
5jEuGX7d46XamerizzRCqEw59vkg42qO7gvlqtD15CJ/89MFzllISIIGUDu0yaiR
c0yl0tbiEfk4XTCB3isfE5wBGeoIHduN7Bqdh1LJU4fNDF3jA89yvCmxQCcnYKL8
4c9+N+bSrQwo1XYqmLP5sUOOl8oSWsFNTu14JUCAKNV6wf2SB2V+AbCOkQwxQrUc
nC5TzHZvsw4uMQ91pLcPA+rgG456LfA40PX6Qcva7BR8+MPjwc/AuK8wNojaKHVz
FQTCm17ikPzkQN82QlyubXSjI+Ed8JBqYapm4l0qi3lItidQ5GVwnvNRFfCSVGu4
V/jFm5XtUpMDbiTBFAh2LZFE7Vn9lWZ4TdgyT75wg3UOKhgNwnvrxbu81svv+R2w
6n365BKSQiDGhDaAzApBSB5y5RlBBNQomHELg+ptizHgKgSBhJs0+zZCV3OpFr8h
MFfiGLhKVwmg414VZmTgshF+OITpVVxS5on9yK6JVnvp3NmnvuzMH2krMKdgDKHH
vaHMkxeMpFX8fYII1hY8mJX5UWFV7Z2siLbokC3iXhvyz53Rt/8=
=cuSK
-----END PGP SIGNATURE-----

--6UQK2KDtdANUlsli--