Re: DROP OWNED BY fails to clean out pg_init_privs grants - Mailing list pgsql-hackers
From | Hannu Krosing |
---|---|
Subject | Re: DROP OWNED BY fails to clean out pg_init_privs grants |
Date | |
Msg-id | CAMT0RQTBiJTmDT0Gf+yGa+YWkhrQu1v=z3Uiq4+=BoSSDJe-YA@mail.gmail.com Whole thread Raw |
In response to | Re: DROP OWNED BY fails to clean out pg_init_privs grants (Daniel Gustafsson <daniel@yesql.se>) |
Responses |
Re: DROP OWNED BY fails to clean out pg_init_privs grants
|
List | pgsql-hackers |
Hi Daniel, pg_upgrade is just one important user of pg_dump which is the one that generates REVOKE for a non-existent role. We should definitely also fix pg_dump, likely just checking that the role exists when generating REVOKE commands (may be a good practice for other cases too so instead of casting to ::regrole do the actual join) ## here is the fix for pg_dump While flying to Vancouver I looked around in pg_dump code, and it looks like the easiest way to mitigate the dangling pg_init_priv entries is to replace the query in pg_dump with one that filters out invalid entries The current query is at line 9336: /* Fetch initial-privileges data */ if (fout->remoteVersion >= 90600) { printfPQExpBuffer(query, "SELECT objoid, classoid, objsubid, privtype, initprivs " "FROM pg_init_privs"); res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); And we need the same but filtering out invalid aclitems from initprivs something like this WITH q AS ( SELECT objoid, classoid, objsubid, privtype, unnest(initprivs) AS initpriv FROM saved_init_privs ) SELECT objoid, classoid, objsubid, privtype, array_agg(initpriv) as initprivs FROM q WHERE is_valid_value_for_type(initpriv::text, 'aclitem') GROUP BY 1,2,3,4; ### The proposed re[placement query: Unfortunately we do not have an existing is_this_a_valid_value_for_type(value text, type text, OUT res boolean) function, so for a read-only workaround the following seems to work: Here I first collect the initprivs array elements which fail the conversion to text and back into an array and store it in GUC pg_dump.bad_aclitems Then I use this stored list to filter out the bad ones in the actual query. DO $$ DECLARE aclitem_text text; bad_aclitems text[] = '{}'; BEGIN FOR aclitem_text IN SELECT DISTINCT unnest(initprivs)::text FROM pg_init_privs LOOP BEGIN /* try to convert back to aclitem */ PERFORM aclitem_text::aclitem; EXCEPTION WHEN OTHERS THEN /* collect bad aclitems */ bad_aclitems := bad_aclitems || ARRAY[aclitem_text]; END; END LOOP; IF bad_aclitems != '{}' THEN RAISE WARNING 'Ignoring bad aclitems "%" in pg_init_privs', bad_aclitems; END IF; PERFORM set_config('pg_dump.bad_aclitems', bad_aclitems::text, false); -- true for trx-local END; $$; WITH q AS ( SELECT objoid, classoid, objsubid, privtype, unnest(initprivs) AS initpriv FROM pg_init_privs ) SELECT objoid, classoid, objsubid, privtype, array_agg(initpriv) AS initprivs FROM q WHERE NOT initpriv::text = ANY (current_setting('pg_dump.bad_aclitems')::text[]) GROUP BY 1,2,3,4; -- Hannu On Sun, May 26, 2024 at 11:27 PM Daniel Gustafsson <daniel@yesql.se> wrote: > > > On 26 May 2024, at 23:25, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > > Hannu Krosing <hannuk@google.com> writes: > >> Attached is a minimal patch to allow missing roles in REVOKE command > > > > FTR, I think this is a very bad idea. > > Agreed, this is papering over a bug. If we are worried about pg_upgrade it > would be better to add a check to pg_upgrade which detects this case and > advices the user how to deal with it. > > -- > Daniel Gustafsson >
pgsql-hackers by date: