Re: pg_dump dump catalog ACLs

Поиск
Список
Период
Сортировка
От Alexander Korotkov
Тема Re: pg_dump dump catalog ACLs
Дата
Msg-id CAPpHfduQdQfCe8jrVLt4R9cwVrRCQtnftQwBbVxnff-vi8TAtw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: pg_dump dump catalog ACLs  (Stephen Frost <sfrost@snowman.net>)
Ответы Re: pg_dump dump catalog ACLs  (Stephen Frost <sfrost@snowman.net>)
Список pgsql-hackers
Hi, Stephen!

I'm signed to review this patch. At first, patch wasn't applied cleanly, it had a conflict at the end of system_views.sql. But that way very easy to fix.

On Mon, Mar 14, 2016 at 7:00 PM, Stephen Frost <sfrost@snowman.net> wrote:
I've not included it in this patch, but my thinking here would be to add
a check to the GRANT functions which checks 'if (creating_extension)'
and records/updates the entries in pg_init_privs for those objects.
This is similar to how we handle dependencies for extensions currently.
That way, extensions don't have to do anything and if the user changes
the permissions on an extension's object, the permissions for that
object would then be dumped out.

This still requires more testing, documentation, and I'll see about
making it work completely for extensions also, but wanted to provide an
update and solicit for review/comments.

The patches have been broken up in what I hope is a logical way for
those who wish to review/comment:

#1 - Add the pg_init_privs catalog
#2 - Change pg_dump to use a bitmap instead of boolean for 'dump'
#3 - Split 'dump' into 'dump' and 'dump_contains'
#4 - Make pg_dump include changed ACLs in dump of 'pg_catalog'

+  "CASE WHEN pip.initprivs IS NOT DISTINCT FROM n.nspacl "
+  "THEN false ELSE true END as dumpacl "

Why don't just write " pip.initprivs IS DISTINCT FROM n.nspacl AS dumpacl"? I think there are few places whene CASE could be eliminated.

+ appendPQExpBuffer(query, "SELECT tableoid, oid, proname AS aggname, "
+  "pronamespace AS aggnamespace, "
+  "pronargs, proargtypes, "
+  "(%s proowner) AS rolname, "
+  "proacl AS aggacl "
+  "FROM pg_proc p "
+  "WHERE proisagg AND ("
+  "pronamespace != "
+  "(SELECT oid FROM pg_namespace "
+  "WHERE nspname = 'pg_catalog') OR "
+  "EXISTS (SELECT * FROM pg_init_privs pip "
+  "WHERE p.oid = pip.objoid AND pip.classoid = "
+  "(SELECT oid FROM pg_class "
+  "WHERE relname = 'pg_proc') "
+  "AND p.proacl <> pip.initprivs)",
+  username_subquery);

Why do we compare ACLs using <> funcs and using IS DISTINCT for other objects?  Is it intentional?  Assuming most of proacl values are NULLs when you change it, acl wouldn't be dumped since NULL <> value IS NULL.

In general, these checks using subqueries looks complicated for me, hard to validate and needs a lot of testing.  Could we implement function "bool need_acl_dump(oid objoid, oid classoid, int objsubid, proacl acl)" and call it?
 
#5 - Remove 'if (!superuser()) ereport()' calls and REVOKE EXECUTE
 
Other things in the patches looks good for me except they need tests and documentation.
I'm marking this waiting for author.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Michael Paquier
Дата:
Сообщение: Re: Patch to implement pg_current_logfile() function
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: Password identifiers, protocol aging and SCRAM protocol