Re: [PATCH v2] use has_privs_for_role for predefined roles

On 3/3/22 11:26, Joshua Brindle wrote:
> On Thu, Feb 10, 2022 at 2:37 PM Joe Conway <mail(at)joeconway(dot)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.

On that last note, I did not find basebackup_to_shell.required_role
documented at all, and did not attempt to fix that.

When this thread petered out, it seemed that Robert was at least neutral
on the patch, and everyone else was +1 on applying it to master for pg15.

As such, if there are any other issues, complaints, etc., please speak
real soon now...

Thanks,

Joe