Re: DROP OWNED BY fails to clean out pg_init_privs grants
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Hannu Krosing <hannuk@google.com>,
"David G. Johnston" <david.g.johnston@gmail.com>,
PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
Stephen Frost <sfrost@snowman.net>,
Andres Freund <andres@anarazel.de>,
Noah Misch <nmisch@google.com>
Date: 2024-05-24T14:20:00Z
Lists: pgsql-hackers
Daniel Gustafsson <daniel@yesql.se> writes: > I had a look, but I didn't beat you to a fix since it's not immediately clear > to me how this should work for REASSING OWNED (DROP OWNED seems a simpler > case). Should REASSIGN OWNED alter the rows in pg_shdepend matching init privs > from SHARED_DEPENDENCY_OWNER to SHARED_DEPENDENCY_INITACL, so that these can be > mopped up with a later DROP OWNED? Trying this in a POC patch it fails with > RemoveRoleFromInitPriv not removing the rows, shortcircuiting that for a test > seems to make it work but is it the right approach? I've tentatively concluded that I shouldn't have modeled SHARED_DEPENDENCY_INITACL so closely on SHARED_DEPENDENCY_ACL, in particular the decision that we don't need such an entry if there's also SHARED_DEPENDENCY_OWNER. I think one reason we can get away with omitting a SHARED_DEPENDENCY_ACL entry for the owner is that the object's normal ACL is part of its primary catalog row, so it goes away automatically if the object is dropped. But obviously that's not true for a pg_init_privs entry. I can see two routes to a solution: 1. Create SHARED_DEPENDENCY_INITACL, if applicable, whether the role is the object's owner or not. Then, clearing out the pg_shdepend entry cues us to go delete the pg_init_privs entry. 2. Just always search pg_init_privs for relevant entries when dropping an object. I don't especially like #2 on performance grounds, but it has a lot fewer moving parts than #1. In particular, there's some handwaving in changeDependencyOnOwner() about why we should drop SHARED_DEPENDENCY_ACL when changing owner, and I've not wrapped my head around how those concerns map to INITACL if we treat it in this different way. Another point: shdepReassignOwned explicitly does not touch grants or default ACLs. It feels like the same should be true of pg_init_privs entries, or at least if not, why not? In that case there's nothing to be done in shdepReassignOwned (although maybe its comments should be adjusted to mention this explicitly). The bug is just that DROP OWNED isn't getting rid of the entries because there's no INITACL entry to cue it to do so. Another thing I'm wondering about right now is privileges on global objects (roles, databases, tablespaces). The fine manual says "Although an extension script is not prohibited from creating such objects, if it does so they will not be tracked as part of the extension". Presumably, that also means that no pg_init_privs entries are made; but do we do that correctly? Anyway, -ENOCAFFEINE for the moment. I'll look more later. regards, tom lane
Commits
-
Allow meson builds to run test_pg_dump test in installcheck mode.
- f663f4daf075 16.5 landed
- b0c5b215dace 17.0 landed
-
Remove recordExtensionInitPriv[Worker]'s ownerId argument.
- ba26d156636c 17.0 landed
-
Improve tracking of role dependencies of pg_init_privs entries.
- 35dd40d34cbd 17.0 landed
-
Fix failure to track role dependencies of pg_init_privs entries.
- 534287403914 17.0 landed
-
Drop global objects after completed test
- 936e3fa3787a 17.0 cited