Re: Planning counters in pg_stat_statements (using pgss_store) - Mailing list pgsql-hackers

On 2020/03/31 3:16, Julien Rouhaud wrote:
> On Mon, Mar 30, 2020 at 6:36 PM Fujii Masao <masao.fujii@oss.nttdata.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.fujii@oss.nttdata.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?

TBH, not sure if that's ok yet...

I'm now just wondering if both plan_nested_level and
exec_nested_level should be incremented in pgss_ProcessUtility().
This is just a guess, so I need more investigation about this.

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters