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. > 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