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
