From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
---|---|
To: | Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> |
Cc: | sk(at)zsrv(dot)org, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Euler Taveira <euler(at)timbira(dot)com(dot)br>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: New function pg_stat_statements_reset_query() to reset statistics of a specific query |
Date: | 2018-07-05 17:22:46 |
Message-ID: | CAHGQGwH0Z2mCy4TGBmtHfs4bm=of+yDtofSfd-AGEH1aJ35dqg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jul 4, 2018 at 7:12 PM, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> wrote:
>
>
> On Mon, Jul 2, 2018 at 6:42 PM Sergei Kornilov <sk(at)zsrv(dot)org> wrote:
>>
>> Hello
>
>
> Thanks for the review.
>
>>
>> I found SELECT pg_stat_statements_reset(NULL,NULL,s.queryid) in tests and
>> it pass tests, but i wonder how it works. Should not we check the NULL
>> through PG_ARGISNULL macro before any PG_GETARG_*? According
>> src/include/fmgr.h
>> > * If function is not marked "proisstrict" in pg_proc, it must check for
>> > * null arguments using this macro. Do not try to GETARG a null
>> > argument!
>
>
> Thanks for checking, Added.
>
>>
>> > pg_stat_statements_reset(userid Oid, dbid Oid, queryid bigint) returns
>> > void
>> And you forgot to change return type in docs (and description of return
>> value)
>
>
> Corrected and also added details of the returns value.
>
> Update patch attached.
+ if (userid != 0 && dbid != 0 && queryid != 0)
UINT64CONST() should be used for the constant for queryid?
It's rare case, but 0 can be assigned as valid queryid. Right?
If yes, it's problematic to use 0 as an invalid queryid here.
+ By default, this function can only be executed by superusers and members
+ of the <literal>pg_read_all_stats</literal> role.
Currently pg_stat_statements_reset() and other stats reset functions like
pg_stat_reset() can be executed only by superusers.
But why did you allow pg_read_all_stats role to execute that function,
by default? That change looks strange to me.
Regards,
--
Fujii Masao
From | Date | Subject | |
---|---|---|---|
Next Message | Andrey Borodin | 2018-07-05 17:27:29 | Re: Covering GiST indexes |
Previous Message | Justin Pryzby | 2018-07-05 16:58:27 | Re: Make deparsing of column defaults faster |