On 2020/10/12 21:18, Yuki Seino wrote:
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation: tested, passed The patch applies cleanly and looks fine to me. It's a small detail, However wouldn't it be better if the following changes were made. 1.There are unnecessary difference lines 707 and 863 in "pg_stat_statements.c". 2.There is no comment on "slock_t mutex" in "struct pgssCtlCounter" in "pg_stat_statements.c". 3."update_ctl" and "reset_ctl" are generic and illegible name.You might want to rename it something like "pgss_ctl_update". 4.To improve the readability of the source, why not change the function declaration option in "pg_stat_statements_ctl" from "AS 'MODULE_PATHNAME'" to "AS 'MODULE_PATHNAME', 'pg_stat_statements_ctl'"? in "pg_stat_statements--1.8--1.9.sql". The new status of this patch is: Waiting on Author
Here are other comments from me. -DATA = pg_stat_statements--1.4.sql \ +DATA = pg_stat_statements--1.4.sql pg_stat_statements--1.8--1.9.sql\ One space character needs to be added just before the character "\". +--- Define pg_stat_statements_ctl ISTM that this is not good name. What about pg_stat_statements_info, _stats, _activity or something? + OUT dealloc integer, The type of "dealloc" should be bigint, instead? + OUT last_dealloc TIMESTAMP WITH TIME ZONE Is this information really useful? If there is no valid use case for this, I'd like to drop it. Thought? +LANGUAGE C STRICT VOLATILE PARALLEL SAFE; There are two space characters just after "LANGUAGE". One space character should be removed from there. +CREATE VIEW pg_stat_statements_ctl AS + SELECT * FROM pg_stat_statements_ctl(); If we rename the function, this view name also should be changed. +GRANT SELECT ON pg_stat_statements TO PUBLIC; "pg_stat_statements" should be "pg_stat_statement_xxx"? Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
