Thread: enable pg_stat_statements to track rows processed by REFRESH MATERIALIZED VIEW
enable pg_stat_statements to track rows processed by REFRESH MATERIALIZED VIEW
From
Katsuragi Yuta
Date:
Hi, pg_stat_statements tracks the number of rows processed by some utility commands. But, currently, it does not track the number of rows processed by REFRESH MATERIALIZED VIEW. Attached patch enables pg_stat_statements to track processed rows by REFRESH MATERIALIZED VIEW. Regards, Katsuragi Yuta
Attachment
Re: enable pg_stat_statements to track rows processed by REFRESH MATERIALIZED VIEW
From
legrand legrand
Date:
Hi, isn't this already fixed in pg14 https://www.postgresql.org/message-id/E1k0mzG-0002Vn-2W@gemulon.postgresql.org ? Regards PAscal -- Sent from: https://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
Re: enable pg_stat_statements to track rows processed by REFRESH MATERIALIZED VIEW
From
Fujii Masao
Date:
On 2020/09/25 19:04, legrand legrand wrote: > Hi, > > isn't this already fixed in pg14 > https://www.postgresql.org/message-id/E1k0mzG-0002Vn-2W@gemulon.postgresql.org > ? IIUC that commit handled CREATE TABLE AS, SELECT INTO, CREATE MATERIALIZED VIEW and FETCH commands, but not REFRESH MATERIALIZED VIEW. Katsuragi-san's patch is for REFRESH MATERIALIZED VIEW. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Re: enable pg_stat_statements to track rows processed by REFRESH MATERIALIZED VIEW
From
legrand legrand
Date:
oups, sorry so +1 for this fix Regards PAscal -- Sent from: https://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
Re: enable pg_stat_statements to track rows processed by REFRESH MATERIALIZED VIEW
From
Yuki Seino
Date:
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 +1. I checked the patch and there were no problems. I hope this fix will be reflected. The new status of this patch is: Ready for Committer
Re: enable pg_stat_statements to track rows processed by REFRESH MATERIALIZED VIEW
From
Fujii Masao
Date:
On 2020/11/02 14:02, 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 > > +1. > I checked the patch and there were no problems. + PG_END_TRY(); + SetQueryCompletion(qc, CMDTAG_REFRESH_MATERIALIZED_VIEW, processed); Isn't it better to call SetQueryCompletion() in ExecRefreshMatView() instead of ProcessUtilitySlow() (e.g., ExecCreateTableAs() does)? Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Re: enable pg_stat_statements to track rows processed by REFRESH MATERIALIZED VIEW
From
Seino Yuki
Date:
2020-11-02 20:01 に Fujii Masao さんは書きました: > On 2020/11/02 14:02, 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 >> >> +1. >> I checked the patch and there were no problems. > > + PG_END_TRY(); > + SetQueryCompletion(qc, CMDTAG_REFRESH_MATERIALIZED_VIEW, processed); > > Isn't it better to call SetQueryCompletion() in ExecRefreshMatView() > instead of ProcessUtilitySlow() (e.g., ExecCreateTableAs() does)? > > Regards, Sorry. I missed it. I've incorporated your point into this patch. So the changes to "matview.h" and "utility.c" have been canceld. We also confirmed that the new patch passed the regression test. Regards,
Attachment
Re: enable pg_stat_statements to track rows processed by REFRESH MATERIALIZED VIEW
From
Fujii Masao
Date:
On 2020/11/05 23:54, Seino Yuki wrote: > 2020-11-02 20:01 に Fujii Masao さんは書きました: >> On 2020/11/02 14:02, 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 >>> >>> +1. >>> I checked the patch and there were no problems. >> >> + PG_END_TRY(); >> + SetQueryCompletion(qc, CMDTAG_REFRESH_MATERIALIZED_VIEW, processed); >> >> Isn't it better to call SetQueryCompletion() in ExecRefreshMatView() >> instead of ProcessUtilitySlow() (e.g., ExecCreateTableAs() does)? >> >> Regards, > > > Sorry. I missed it. > I've incorporated your point into this patch. > So the changes to "matview.h" and "utility.c" have been canceld. > > We also confirmed that the new patch passed the regression test. Thanks for updating the patch! + /* save the rowcount if we're given a qc to fill */ + SetQueryCompletion(qc, CMDTAG_REFRESH_MATERIALIZED_VIEW, processed); I added the check "if (qc)" into the above. I also added the following comments about that we don't display the rowcount in the command completion tag output though we save it in qc. There is the discussion related to this topic, at [1]. Thought? + * Save the rowcount so that pg_stat_statements can track the total number + * of rows processed by REFRESH MATERIALIZED VIEW command. Note that we + * still don't display the rowcount in the command completion tag output, + * i.e., the display_rowcount flag of CMDTAG_REFRESH_MATERIALIZED_VIEW + * command tag is left false in cmdtaglist.h. Otherwise, the change of + * completion tag output might break applications using it. Attached is the updated version of the patch. Barring no objection, I will commit that. Regards, [1] https://postgr.es/m/aadbfba9-e4bb-9531-6b3a-d13c31c8f4fe@oss.nttdata.com -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Attachment
Re: enable pg_stat_statements to track rows processed by REFRESH MATERIALIZED VIEW
From
Fujii Masao
Date:
On 2020/11/10 17:29, Fujii Masao wrote: > > > On 2020/11/05 23:54, Seino Yuki wrote: >> 2020-11-02 20:01 に Fujii Masao さんは書きました: >>> On 2020/11/02 14:02, 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 >>>> >>>> +1. >>>> I checked the patch and there were no problems. >>> >>> + PG_END_TRY(); >>> + SetQueryCompletion(qc, CMDTAG_REFRESH_MATERIALIZED_VIEW, processed); >>> >>> Isn't it better to call SetQueryCompletion() in ExecRefreshMatView() >>> instead of ProcessUtilitySlow() (e.g., ExecCreateTableAs() does)? >>> >>> Regards, >> >> >> Sorry. I missed it. >> I've incorporated your point into this patch. >> So the changes to "matview.h" and "utility.c" have been canceld. >> >> We also confirmed that the new patch passed the regression test. > > Thanks for updating the patch! > > + /* save the rowcount if we're given a qc to fill */ > + SetQueryCompletion(qc, CMDTAG_REFRESH_MATERIALIZED_VIEW, processed); > > I added the check "if (qc)" into the above. I also added the following > comments about that we don't display the rowcount in the command > completion tag output though we save it in qc. There is the discussion > related to this topic, at [1]. Thought? > > + * Save the rowcount so that pg_stat_statements can track the total number > + * of rows processed by REFRESH MATERIALIZED VIEW command. Note that we > + * still don't display the rowcount in the command completion tag output, > + * i.e., the display_rowcount flag of CMDTAG_REFRESH_MATERIALIZED_VIEW > + * command tag is left false in cmdtaglist.h. Otherwise, the change of > + * completion tag output might break applications using it. > > Attached is the updated version of the patch. > Barring no objection, I will commit that. Pushed. Thanks! Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION