On Mon, Sep 24, 2018 at 11:13 AM Haribabu Kommi <kommi.harib...@gmail.com> wrote:
> On Sat, Sep 22, 2018 at 11:23 PM Amit Kapila <amit.kapil...@gmail.com> > wrote: > >> On Mon, Jul 9, 2018 at 11:28 AM Haribabu Kommi <kommi.harib...@gmail.com> >> wrote: >> > >> > On Mon, Jul 9, 2018 at 3:39 PM Haribabu Kommi <kommi.harib...@gmail.com> >> wrote: >> >> >> >> >> >> On Mon, Jul 9, 2018 at 12:24 PM Michael Paquier <mich...@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 > needs to be removed. > > >> @@ -1391,7 +1392,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, >> MemoryContext per_query_ctx; >> MemoryContext oldcontext; >> Oid userid = GetUserId(); >> - bool is_superuser = superuser(); >> + bool is_allowed_role = false; >> char *qbuffer = NULL; >> Size qbuffer_size = 0; >> Size extent = 0; >> @@ -1399,6 +1400,9 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo, >> HASH_SEQ_STATUS hash_seq; >> pgssEntry *entry; >> >> + /* 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]. [1] - https://www.postgresql.org/message-id/CAJrrPGf5fCnKqXObpwGN9nMyD--tzOf-7LFCJiz59Z1wJ5qj9A%40mail.gmail.com Regards, Haribabu Kommi Fujitsu Australia
0001-pg_stat_statements_reset-to-reset-specific-query-use_v5.patch
Description: Binary data
