On 3/20/22 12:38, Stephen Frost wrote:
Greetings,
On Sun, Mar 20, 2022 at 18:31 Joshua Brindle
<joshua.brin...@crunchydata.com <mailto:joshua.brin...@crunchydata.com>>
wrote:
On Sun, Mar 20, 2022 at 12:27 PM Joe Conway <m...@joeconway.com
<mailto:m...@joeconway.com>> wrote:
>
> On 3/3/22 11:26, Joshua Brindle wrote:
> > On Thu, Feb 10, 2022 at 2:37 PM Joe Conway <m...@joeconway.com
<mailto:m...@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.
Robert -- any opinion on this? If I am not mistaken it is code that you
are actively working on.
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
