At Tue, 2 Jun 2020 13:13:18 -0300, Martín Marqués <martin@2ndquadrant.com> wrote in
> > > I have no problem adding it to this ROLE, but we'd have to amend the
> > > doc for default-roles to reflect that SELECT for this view is also
> > > granted to `pg_read_all_stats`.
> >
> > I agree in general that pg_monitor shouldn't have privileges granted
> > directly to it. If this needs a new default role, that's an option, but
> > it seems like it'd make sense to be part of pg_read_all_stats to me, so
> > amending the docs looks reasonable from here.
>
> Good, that's more or less what I had in mind.
>
> Here goes v2 of the patch, now there are 4 files (I could have
> squashed the docs with the code changes, but hey, that'll be easy to
> merge if needed :-) )
>
> I did some fiddling to Michaels doc proposal, but it says basically the same.
>
> Not 100% happy with the change to user-manag.sgml, but ok enough to send.
>
> I also added an entry to the commitfest so we can track this there as well.
0001:
Looks good to me. REVOKE is performed on all functions that called
replorigin_check_prerequisites.
0002:
It is forgetting to grant pg_read_all_stats to execute
pg_show_replication_origin_status. As the result pg_read_all_stats
gets error on executing the function, not on doing select on the view.
Even if we also granted execution of the function to the specific
role, anyone who wants to grant a user for the view also needs to
grant execution of the function.
To avoid such an inconvenience, as I mentioned upthread, the view and
the function should be granted to public and the function should just
mute the output all the rows, or some columns in each row. That can be
done the same way with pg_stat_get_activity(), as Stephen said.
0003:
It seems to be a take-after of adminpack's documentation, but a
superuser is not the only one on PostgreSQL. The something like the
description in 27.2.2 Viewing Statistics looks more suitable.
> Superusers and members of the built-in role pg_read_all_stats (see
> also Section 21.5) can see all the information about all sessions.
Section 21.5 is already saying as follows.
> pg_monitor
> Read/execute various monitoring views and functions. This role is a
> member of pg_read_all_settings, pg_read_all_stats and
> pg_stat_scan_tables.
0004:
Looks fine by me.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center