Re: DROP OWNED BY fails to clean out pg_init_privs grants
Hannu Krosing <hannuk@google.com>
From: Hannu Krosing <hannuk@google.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>, Daniel Gustafsson <daniel@yesql.se>, "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-25T14:09:44Z
Lists: pgsql-hackers
On Fri, May 24, 2024 at 10:00 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Robert Haas <robertmhaas@gmail.com> writes: > > On Fri, May 24, 2024 at 2:57 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> Doesn't seem right to me. That will give pg_dump the wrong idea > >> of what the initial privileges actually were, and I don't see how > >> it can construct correct delta GRANT/REVOKE on the basis of false > >> information. During the dump reload, the extension will be > >> recreated with the original owner (I think), causing its objects' > >> privileges to go back to the original pg_init_privs values. > > > Oh! That does seem like it would make what I said wrong, but how would > > it even know who the original owner was? Shouldn't we be recreating > > the object with the owner it had at dump time? > > Keep in mind that the whole point here is for the pg_dump script to > just say "CREATE EXTENSION foo", not to mess with the individual > objects therein. So the objects are (probably) going to be owned by > the user that issued CREATE EXTENSION. > > In the original conception, that was the end of it: what you got for > the member objects was whatever state CREATE EXTENSION left behind. > The idea of pg_init_privs is to support dump/reload of subsequent > manual alterations of privileges for extension-created objects. > I'm not, at this point, 100% certain that that's a fully realizable > goal. The issue became visible because pg_dump issued a bogus REVOKE ALL ON TABLE public.pg_stat_statements FROM "16390"; Maybe the right place for a fix is in pg_dump and the fix would be to *not* issue REVOKE ALL ON <any object> FROM <non-existing users> ? Or alternatively change REVOKE to treat non-existing users as a no-op ? Also, the pg_init_privs entry should either go away or at least be changed at the point when the user referenced in init-privs is dropped. Having an pg_init_privs entry referencing a non-existing user is certainly of no practical use. Or maybe we should change the user at that point to NULL or some special non-existing-user-id ? > But I definitely think it's insane to expect that to work > without also tracking changes in the ownership of said objects. > > Maybe forbidding ALTER OWNER on extension-owned objects isn't > such a bad idea?
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