On 3/20/22 12:31, Joshua Brindle wrote:
On Sun, Mar 20, 2022 at 12:27 PM Joe Conway <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> 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.
Maybe worth a discussion, but it seems strange to me to treat them
differently when the whole purpose of this patch is to make things
consistent ¯\_(ツ)_/¯
Joe
