On Wed, Jul 4, 2018 at 7:12 PM, Haribabu Kommi <kommi.harib...@gmail.com> wrote: > > > On Mon, Jul 2, 2018 at 6:42 PM Sergei Kornilov <s...@zsrv.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