Re: superuser() shortcuts

Поиск
Список
Период
Сортировка
От Brightwell, Adam
Тема Re: superuser() shortcuts
Дата
Msg-id CAKRt6CTaxSNKi=W=-NF4wOHJOMqiyKh4dEvjbLoQ+Ve_xbMvDg@mail.gmail.com
обсуждение исходный текст
Ответ на superuser() shortcuts  (Stephen Frost <sfrost@snowman.net>)
Ответы Re: superuser() shortcuts
Список pgsql-hackers
Stephen,

  We, as a community, have gotten flak from time-to-time about the
  superuser role.  We also tend to wish to avoid unnecessary
  optimization as it complicates the code base and makes folks reviewing
  the code wonder at the exceptions.

I have attached a patch for consideration/discussion that addresses these items.  I have based it off of current master (0c013e0).  I have done some cursory testing including check-world with success.
 
      My 2c about this function is that it should be completely removed
      and the place where it's checked replaced  with just the
      'has_rolreplication' call and error.  It's only called in one
      place and it'd be a simple one-liner anyway.  As for
      has_rolreplication, I don't understand why it's in miscinit.c when
      the rest of the has_* set is in acl.c.

With all the other pg_has_* functions being found in acl.c I agree that it would seem odd that this (or any other related function) would be found elsewhere.  Since aclchk.c also has a couple of functions that follow the pattern of "has_<priv>_privilege", I moved this function there, for now, as well as modified it to include superuser checks as they were not previously there.  The only related function I found in acl.c was "has_rolinherit" which is a static function. :-/  There is also a static function "has_rolcatupdate" in aclchk.c.  I would agree that acl.c (or aclchk.c?) would be a more appropriate location for these functions, though in some of the above cases, it might require exposing them (I'm not sure that I see any harm in doing so).
 
Perhaps refactoring to consolidate all of these in either acl.c or aclchk.c with the following pattern might be a step in the right direction?

* has_createrole_privilege
* has_bypassrls_privilege
* has_inherit_privilege
* has_catupdate_privilege
* has_replication_privilege
* has_???_privilege

In either case, I think following a "convention" on the naming of these functions (unless there is semantic reason not to) would also help to reduce any confusion or head scratching.

  Removing these design patterns may also help to avoid ending up with
  more of them in the future as folks copy and/or crib off of what we've
  already done to implement their features...
 
I agree.

-Adam

--
Вложения

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

Предыдущее
От: Kohei KaiGai
Дата:
Сообщение: Re: [v9.5] Custom Plan API
Следующее
От: Ants Aasma
Дата:
Сообщение: Re: "Value locking" Wiki page