Обсуждение: pgsql: Fix failure to track role dependencies of pg_init_privs entries.
Fix failure to track role dependencies of pg_init_privs entries. If an ACL recorded in pg_init_privs mentions a non-pinned role, that reference must also be noted in pg_shdepend so that we know that the role can't go away without removing the ACL reference. Otherwise, DROP ROLE could succeed and leave dangling entries behind, which is what's causing the recent upgrade-check failures on buildfarm member copperhead. This has been wrong since pg_init_privs was introduced, but it's escaped notice because typical pg_init_privs entries would only mention the bootstrap superuser (pinned) or at worst the owner of the extension (who can't go away before the extension does). We lack even a representation of such a role reference for pg_shdepend. My first thought for a solution was entries listing pg_init_privs in classid, but that doesn't work because then there's noplace to put the granted-on object's classid. Rather than adding a new column to pg_shdepend, let's add a new deptype code SHARED_DEPENDENCY_INITACL. Much of the associated boilerplate code can be cribbed from code for SHARED_DEPENDENCY_ACL. A lot of the bulk of this patch just stems from the new need to pass the object's owner ID to recordExtensionInitPriv, so that we can consult it while updating pg_shdepend. While many callers have that at hand already, a few places now need to fetch the owner ID of an arbitrary privilege-bearing object. For that, we assume that there is a catcache on the relevant catalog's OID column, which is an assumption already made in ExecGrant_common so it seems okay here. We do need an entirely new routine RemoveRoleFromInitPriv to perform cleanup of pg_init_privs ACLs during DROP OWNED BY. It's analogous to RemoveRoleFromObjectACL, but we can't share logic because that function operates by building a command parsetree and invoking existing GRANT/REVOKE infrastructure. There is of course no SQL command that would update pg_init_privs entries when we're not in process of creating their extension, so we need a routine that can do the updates directly. catversion bump because this changes the expected contents of pg_shdepend. For the same reason, there's no hope of back-patching this, even though it fixes a longstanding bug. Fortunately, the case where it's a problem seems to be near nonexistent in the field. If it weren't for the buildfarm breakage, I'd have been content to leave this for v18. Patch by me; thanks to Daniel Gustafsson for review and discussion. Discussion: https://postgr.es/m/1745535.1712358659@sss.pgh.pa.us Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/534287403914cc9760db98f7320ac4e92f5d416e Modified Files -------------- doc/src/sgml/catalogs.sgml | 14 ++ src/backend/catalog/aclchk.c | 262 +++++++++++++++++++-- src/backend/catalog/pg_shdepend.c | 56 ++++- src/include/catalog/catversion.h | 2 +- src/include/catalog/dependency.h | 16 +- src/include/utils/acl.h | 2 + .../modules/test_pg_dump/expected/test_pg_dump.out | 183 ++++++++++++++ src/test/modules/test_pg_dump/sql/test_pg_dump.sql | 37 +++ 8 files changed, 549 insertions(+), 23 deletions(-)
Re: pgsql: Fix failure to track role dependencies of pg_init_privs entries.
От
Alexander Korotkov
Дата:
On Tue, Apr 30, 2024 at 2:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Fix failure to track role dependencies of pg_init_privs entries. I just noticed that test_pg_dump checks don't pass for me. And for most of the buildfarm members too. You must be already aware of this, just in case. ------ Regards, Alexander Korotkov
Alexander Korotkov <aekorotkov@gmail.com> writes: > I just noticed that test_pg_dump checks don't pass for me. And for > most of the buildfarm members too. > You must be already aware of this, just in case. D'oh. Should have actually checked that test with a non-'postgres' superuser. Will fix in a few. regards, tom lane