On Mon, Jul 9, 2018 at 11:28 AM Haribabu Kommi <kommi.haribabu@gmail.com> wrote: > > On Mon, Jul 9, 2018 at 3:39 PM Haribabu Kommi <kommi.haribabu@gmail.com> wrote: >> >> >> On Mon, Jul 9, 2018 at 12:24 PM Michael Paquier <michael@paquier.xyz> wrote: >>> >>> On Fri, Jul 06, 2018 at 05:10:18PM -0400, Alvaro Herrera wrote: >>> > Ugh, it's true :-( >>> > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=25fff40798fc4ac11a241bfd9ab0c45c085e2212#patch8 >>> > >>> > Dave, Simon, any comments? >>> >>> The offending line: >>> contrib/pg_stat_statements/pg_stat_statements--1.4--1.5.sql: >>> GRANT EXECUTE ON FUNCTION pg_stat_statements_reset() TO pg_read_all_stats; >>> >>> This will need a new version bump down to REL_10_STABLE... >> >> >> Hearing no objections, attached patch removes all permissions from PUBLIC >> as before this change went in. Or do we need to add command for revoke only >> from pg_read_all_stats? > > > Revoke all doesn't work, so patch updated with revoke from pg_read_all_stats. >
Thanks for the review.
The other questionable part of that commit (25fff40798) is that it changes permissions for function pg_stat_statements_reset at SQL level and for C function it changes the permission check for pg_stat_statements, refer below change.
The below changes are to support the read access of particular columns
of the pg_stat_statements view to pg_read_all_stats role. These changes
should exist and only the permissions of pg_stat_statements_reset() function
+ /* Superusers or members of pg_read_all_stats members are allowed */ + is_allowed_role = is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS);
Am I confused here? In any case, I think it is better to start a separate thread to discuss this issue. It might help us in getting more attention on this issue and we can focus on your proposed patch in this thread.
Started a new thread to discuss about the revoke the execute permissions
of the pg_stat_statements_reset() from pg_read_all_stats role at [1]
Attached new rebased version of the patch that enhances the pg_stat_statements_reset()
function. This needs to be applied on top of the patch that is posted in [1].