Re: [PATCH v2] use has_privs_for_role for predefined roles

Поиск
Список
Период
Сортировка
От Stephen Frost
Тема Re: [PATCH v2] use has_privs_for_role for predefined roles
Дата
Msg-id CAOuzzgqzTSwtnUwZXFL2Hx6M2TTTmK_SZyjp+AdTcQMhL_Eskg@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [PATCH v2] use has_privs_for_role for predefined roles  (Joshua Brindle <joshua.brindle@crunchydata.com>)
Ответы Re: [PATCH v2] use has_privs_for_role for predefined roles  (Joe Conway <mail@joeconway.com>)
Список pgsql-hackers
Greetings,

On Sun, Mar 20, 2022 at 18:31 Joshua Brindle <joshua.brindle@crunchydata.com> wrote:
On Sun, Mar 20, 2022 at 12:27 PM Joe Conway <mail@joeconway.com> wrote:
>
> On 3/3/22 11:26, Joshua Brindle wrote:
> > On Thu, Feb 10, 2022 at 2:37 PM Joe Conway <mail@joeconway.com> wrote:
> >>
> >> On 2/10/22 14:28, Nathan Bossart wrote:
> >> > On Wed, Feb 09, 2022 at 04:39:11PM -0500, Joe Conway wrote:
> >> >> On 2/9/22 13:13, Nathan Bossart wrote:
> >> >>> I do wonder if users find the differences between predefined roles and role
> >> >>> attributes confusing.  INHERIT doesn't govern role attributes, but it will
> >> >>> govern predefined roles when this patch is applied.  Maybe the role
> >> >>> attribute system should eventually be deprecated in favor of using
> >> >>> predefined roles for everything.  Or perhaps the predefined roles should be
> >> >>> converted to role attributes.
> >> >>
> >> >> Yep, I was suggesting that the latter would have been preferable to me while
> >> >> Robert seemed to prefer the former. Honestly I could be happy with either of
> >> >> those solutions, but as I alluded to that is probably a discussion for the
> >> >> next development cycle since I don't see us doing that big a change in this
> >> >> one.
> >> >
> >> > I agree.  I still think Joshua's proposed patch is a worthwhile improvement
> >> > for v15.
> >>
> >> +1
> >>
> >> I am planning to get into it in detail this weekend. So far I have
> >> really just ensured it merges cleanly and passes make world.
> >
> > Rebased patch to apply to master attached.
>
> Well longer than I planned, but finally took a closer look.
>
> I made one minor editorial fix to Joshua's patch, rebased to current
> master, and added two missing call sites that presumably are related to
> recent commits for pg_basebackup.

FWIW I pinged Stephen when I saw the basebackup changes included
is_member_of and he didn't think they necessarily needed to be changed
since they aren't accessible to human and you can't SET ROLE on a
replication connection in order to access the role where inheritance
was blocked.

Yeah, though that should really be very clearly explained in comments around that code as anything that uses is_member should really be viewed as an exception.  I also wouldn’t be against using has_privs there anyway and saying that you shouldn’t be trying to connect as one role on a replication connection and using the privs of another when you don’t automatically inherit those rights, but on the assumption that the committer intended is_member there because SET ROLE isn’t something one does on replication connections, I’m alright with it being as is.

Kind Regards,

Stephen P Frost

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

Предыдущее
От: Joshua Brindle
Дата:
Сообщение: Re: [PATCH v2] use has_privs_for_role for predefined roles
Следующее
От: Andres Freund
Дата:
Сообщение: Re: shared-memory based stats collector - v66