Re: Additional role attributes && superuser review

Michael,

* Michael Paquier (michael.paquier@gmail.com) wrote:
> On Sat, Nov 21, 2015 at 2:29 AM, Stephen Frost <sfrost@snowman.net> wrote:
> > * Michael Paquier (michael.paquier@gmail.com) wrote:
> > Even so, in the interest of having more fine-grained permission
> > controls, I've gone ahead and added a pg_switch_xlog default role.
> > Note that this means that pg_switch_xlog() can be called by both
> > pg_switch_xlog roles and pg_backup roles.  I'd be very much against
> > removing the ability to call pg_switch_xlog from the pg_backup role as
> > that really is a capability which is needed by users running backups and
> > it'd just add unnecessary complexity to require users setting up backup
> > tools to grant two different roles to get the backup to work.
>
> There is going to be many opinions regarding the granularity of this
> control, each one of us having a different opinion at the end. I don't
> think this should be a stopper for this patch, hence I am fine with the
> judgement you think is good. We could still more finely tune those default
> roles later in the dev cycle of 9.6 (10.0?).

Agreed.

> Thanks, this looks good to me.

Great.

> I guess that's better than nothing.

Agreed.

> I don't think you mean to refer to the switch of segments files here. Same
> comment for pg_current_xlog_insert_location, pg_last_xlog_receive_location
> and pg_last_xlog_replay_location.

Urgh.  Got a bit ahead of myself there, apologies.  I've updated all of
these and a number of other minor typos and incorrect comments along the
way.

> +               ereport(ERROR,
> +                               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> +                                errmsg("must be superuser or member of
> pg_file_settings to see all config file settings")));
> Should avoid abbreviations => "all configuration file settings".

Fixed.

>        <varlistentry>
> -        <term><literal>\dg[+] [ <link
> linkend="APP-PSQL-patterns"><replaceable
> class="parameter">pattern</replaceable></link> ]</literal></term>
> +        <term><literal>\dgS[+] [ <link
> linkend="APP-PSQL-patterns"><replaceable
> class="parameter">pattern</replaceable></link> ]</literal></term>
>          <listitem>
> I'm picky here, but that should be "\dg[S+]". Same for \du[S+].

Fixed

Updated patch attached.

Thanks!

Stephen