Re: Planning counters in pg_stat_statements (using pgss_store)

On Thu, Apr 9, 2020 at 5:59 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>
>
>
> On 2020/04/08 18:31, Julien Rouhaud wrote:
> > On Wed, Apr 08, 2020 at 05:37:27PM +0900, Fujii Masao wrote:
> >>
> >>
> >> On 2020/04/03 16:26, Julien Rouhaud wrote:
> >>> On Thu, Apr 02, 2020 at 01:04:28PM -0700, legrand legrand wrote:
> >>>> Fujii Masao-4 wrote
> >>>>> On 2020/04/01 18:19, Fujii Masao wrote:
> >>>>>
> >>>>> Finally I pushed the patch!
> >>>>> Many thanks for all involved in this patch!
> >>>>>
> >>>>> As a remaining TODO item, I'm thinking that the document would need to
> >>>>> be improved. For example, previously the query was not stored in pgss
> >>>>> when it failed. But, in v13, if pgss_planning is enabled, such a query is
> >>>>> stored because the planning succeeds. Without the explanation about
> >>>>> that behavior in the document, I'm afraid that users will get confused.
> >>>>> Thought?
> >>>>
> >>>> Thank you all for this work and especially to Julian for its major
> >>>> contribution !
> >>>
> >>>
> >>> Thanks a lot to everyone! This was quite a long journey.
> >>>
> >>>
> >>>> Regarding the TODO point: Yes I agree that it can be improved.
> >>>> My proposal:
> >>>>
> >>>> "Note that planning and execution statistics are updated only at their
> >>>> respective end phase, and only for successfull operations.
> >>>> For exemple executions counters of a long running SELECT query,
> >>>> will be updated at the execution end, without showing any progress
> >>>> report in the interval.
> >>>> Other exemple, if the statement is successfully planned but fails in
> >>>> the execution phase, only its planning statistics are stored.
> >>>> This may give uncorrelated plans vs calls informations."
> >>
> >> Thanks for the proposal!
> >>
> >>> There are numerous reasons for lack of correlation between number of planning
> >>> and number of execution, so I'm afraid that this will give users the false
> >>> impression that only failed execution can lead to that.
> >>>
> >>> Here's some enhancement on your proposal:
> >>>
> >>> "Note that planning and execution statistics are updated only at their
> >>> respective end phase, and only for successful operations.
> >>> For example the execution counters of a long running query
> >>> will only be updated at the execution end, without showing any progress
> >>> report before that.
> >>
> >> Probably since this is not the example for explaining the relationship of
> >> planning and execution stats, it's better to explain this separately or just
> >> drop it?
> >>
> >> What about the attached patch based on your proposals?
> >>
> >
> > Thanks Fuji-san, it looks perfect to me!
>
> Thanks for the check! Pushed!

Thanks a lot Fuji-san!

For the record, the commit is available, but I didn't receive the
usual mail, and it's also not present in the archives apparently:
https://www.postgresql.org/list/pgsql-committers/since/202004090000/
(although Amit's latest commit was delivered as expected).

Given your previous discussion with Magnus, I'm assuming that your
address is now allowed to post for a year. I'm not sure what went
wrong here, so I'm adding Magnus in Cc.