From: Joel Jacobson <joel@compiler.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pgsql: Add pg_get_acl() to get the ACL for a database object
Date: Mon, 08 Jul 2024 11:55:28 +0200
Message-ID: <aa46ce00-17ad-4a87-938f-c66196800129@app.fastmail.com> (raw)
In-Reply-To: <ZoukijRBef4m-VMO@paquier.xyz>
References: <E1sPHWu-000GPc-0z@gemulon.postgresql.org>
<3408967.1720107891@sss.pgh.pa.us>
<f2688fb7-add6-43da-b1df-97312a451893@app.fastmail.com>
<ZoctyZzE3UlUSer_@paquier.xyz>
<f2539bff-64be-47f0-9f0b-df85d3cc0432@app.fastmail.com>
<ZoukijRBef4m-VMO@paquier.xyz>
On Mon, Jul 8, 2024, at 10:34, Michael Paquier wrote:
> 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?
Thanks, nice simplifications.
I agree the tests you removed are not that interesting.
Looks good to me.
Patch didn't apply to HEAD nor on top of any of the previous commits either,
but I couldn't figure out why based on the .rej files, strange.
I copy/pasted the parts from the patch to test it. Let me know if you need a
rebased version of it, in case you will need to do the same, to save some work.
Also noted the below in your last commit:
a6417078c414 has introduced as project policy that new features
committed during the development cycle should use new OIDs in the
[8000,9999] range.
4564f1cebd43 did not respect that rule, so let's renumber pg_get_acl()
to use an OID in the correct range.
Thanks for the info!
Will make sure to use the `src/include/catalog/renumber_oids.pl` tool
for future patches.
Regards,
Joel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: joel@compiler.org, michael@paquier.xyz, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Add pg_get_acl() to get the ACL for a database object
In-Reply-To: <aa46ce00-17ad-4a87-938f-c66196800129@app.fastmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox