Re: Planning counters in pg_stat_statements (using pgss_store)

On Mon, Mar 30, 2020 at 6:36 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>
> On 2020/03/30 17:03, Julien Rouhaud wrote:
> > On Mon, Mar 30, 2020 at 01:56:43PM +0900, Fujii Masao wrote:
> >>
> >>
> >> On 2020/03/29 15:15, Julien Rouhaud wrote:
> >>> On Fri, Mar 27, 2020 at 03:42:50PM +0100, Julien Rouhaud wrote:
> >>>> On Fri, Mar 27, 2020 at 2:01 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> >>>>>
> >>>>
> >>>>> So what I'd like to say is that the information that users are interested
> >>>>> in would vary on each situation and case. At least for me it seems
> >>>>> enough for pgss to report only the basic information. Then users
> >>>>> can calculate to get the numbers (like total_time) they're interested in,
> >>>>> from those basic information.
> >>>>>
> >>>>> But of course, I'd like to hear more opinions about this...
> >>>>
> >>>> +1
> >>>>
> >>>> Unless someone chime in by tomorrow, I'll just drop the sum as it
> >>>> seems less controversial and not a blocker in userland if users are
> >>>> interested.
> >>>
> >>> Done in attached v11, with also the s/querytext/query_text/ discrepancy noted
> >>> previously.
> >>
> >> Thanks for updating the patch! But I still think query_string is better
> >> name because it's used in other several places, for the sake of consistency.
> >
> > You're absolutely right. That's what I actually wanted to do given your
> > previous comment, but somehow managed to miss it, sorry about that and thanks
> > for fixing.
> >
> >> So I changed the argument name that way and commit the 0001 patch.
> >> If you think query_text is better, let's keep discussing this topic!
> >>
> >> Anyway many thanks for your great job!
> >
> > Thanks a lot!
> >
> >>
> >>>>>> I also exported BufferUsageAccumDiff as mentioned previously, as it seems
> >>>>>> clearner and will avoid future useless code churn, and run pgindent.
> >>>>>
> >>>>> Many thanks!! I'm thinking to commit this part separately.
> >>>>> So I made that patch based on your patch. Attached.
> >>>>
> >>>> Thanks! It looks good to me.
> >>>
> >>> I also kept that part in a distinct commit for convenience.
> >>
> >> I also pushed 0002 patch. Thanks!
> >>
> >> I will review 0003 patch again.
> >
> > And thanks for that too :)
>
> While testing the patched pgss, I found that the patched version
> may track the statements that the original version doesn't.
> Please imagine the case where the following queries are executed,
> with pgss.track = top.
>
> PREPARE hoge AS SELECT * FROM t;
> EXPLAIN EXECUTE hoge;
>
> The pgss view returned "PREPARE hoge AS SELECT * FROM t"
> in the patched version, but not in the orignal version.
>
> Is this problematic?

Oh indeed. That's a side effect of having different the executed query
and the planned query being different.

I guess the question is to chose if the top level executed query of a
utilty statement containing an optimisable query, should the top level
planner call of that optimisable statement be considered at top level
or not. I tend to think that's the correct behavior here, as this is
also what would happen if a regular DML was provided. What do you
think?