From: | Martín Marqués <martin(at)2ndquadrant(dot)com> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Cc: | Stephen Frost <sfrost(at)snowman(dot)net>, Michael Paquier <michael(at)paquier(dot)xyz>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Read access for pg_monitor to pg_replication_origin_status view |
Date: | 2020-06-03 16:32:28 |
Message-ID: | CAPdiE1zUeSycQ-VHrZQMDR4vkPthYTSxeQPXM-ZfH8QHDBCE=Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Kyotaro-san,
Thank you for taking the time to review my patches. Would you like to
set yourself as a reviewer in the commit entry here?
https://commitfest.postgresql.org/28/2577/
> 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.
Seems I was testing on a cluster where I had already been digging, so
pg_real_all_stats had execute privileges on
pg_show_replication_origin_status (I had manually granted that) and
didn't notice because I forgot to drop the cluster and initialize
again.
Thanks for the pointer here!
> 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.
I'm not sure if I got this right, but I added some more text to point
out that the pg_read_all_stats role can also access one specific
function. I personally think it's a bit too detailed, and if we wanted
to add details it should be formatted differently, which would require
a more invasive patch (would prefer leaving that out, as it might even
mean moving parts which are not part of this patch).
In any case, I hope the change fits what you've kindly pointed out.
> 0004:
>
> Looks fine by me.
Here goes v3 of the patch
--
Martín Marqués http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachment | Content-Type | Size |
---|---|---|
v3-0001-Access-to-pg_replication_origin_status-view-has-r.patch | text/x-patch | 3.0 KB |
v3-0004-Apply-changes-to-the-documentation-to-reflect-tha.patch | text/x-patch | 1.1 KB |
v3-0002-We-want-the-monitoring-role-pg_read_all_stats-to-.patch | text/x-patch | 1.2 KB |
v3-0003-Change-replication-origin-function-documenation-t.patch | text/x-patch | 1.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Mark Dilger | 2020-06-03 17:05:26 | Towards easier AMs: Cleaning up inappropriate use of name "relkind" |
Previous Message | Robert Haas | 2020-06-03 16:16:08 | Re: Expand the use of check_canonical_path() for more GUCs |