Re: Is it useful to record whether plans are generic or custom?

On 2020-07-10 10:49, torikoshia wrote:
> On 2020-07-08 16:41, Fujii Masao wrote:
>> On 2020/07/08 10:14, torikoshia wrote:
>>> On 2020-07-06 22:16, Fujii Masao wrote:
>>>> On 2020/06/11 14:59, torikoshia wrote:
>>>>> On 2020-06-10 18:00, Kyotaro Horiguchi wrote:
>>>>> 
>>>>>> 
>>>>>> +    TupleDescInitEntry(tupdesc, (AttrNumber) 8, "last_plan",
>>>>>> 
>>>>>> This could be a problem if we showed the last plan in this view.  
>>>>>> I
>>>>>> think "last_plan_type" would be better.
>>>>>> 
>>>>>> +            if (prep_stmt->plansource->last_plan_type == 
>>>>>> PLAN_CACHE_TYPE_CUSTOM)
>>>>>> +                values[7] = CStringGetTextDatum("custom");
>>>>>> +            else if (prep_stmt->plansource->last_plan_type == 
>>>>>> PLAN_CACHE_TYPE_GENERIC)
>>>>>> +                values[7] = CStringGetTextDatum("generic");
>>>>>> +            else
>>>>>> +                nulls[7] = true;
>>>>>> 
>>>>>> Using swith-case prevents future additional type (if any) from 
>>>>>> being
>>>>>> unhandled.  I think we are recommending that as a convension.
>>>>> 
>>>>> Thanks for your reviewing!
>>>>> 
>>>>> I've attached a patch that reflects your comments.
>>>> 
>>>> Thanks for the patch! Here are the comments.
>>> 
>>> Thanks for your review!
>>> 
>>>> +        Number of times generic plan was choosen
>>>> +        Number of times custom plan was choosen
>>>> 
>>>> Typo: "choosen" should be "chosen"?
>>> 
>>> Thanks, fixed them.
>>> 
>>>> +      <entry role="catalog_table_entry"><para 
>>>> role="column_definition">
>>>> +       <structfield>last_plan_type</structfield> <type>text</type>
>>>> +      </para>
>>>> +      <para>
>>>> +        Tells the last plan type was generic or custom. If the 
>>>> prepared
>>>> +        statement has not executed yet, this field is null
>>>> +      </para></entry>
>>>> 
>>>> Could you tell me how this information is expected to be used?
>>>> I think that generic_plans and custom_plans are useful when 
>>>> investigating
>>>> the cause of performance drop by cached plan mode. But I failed to 
>>>> get
>>>> how much useful last_plan_type is.
>>> 
>>> This may be an exceptional case, but I once had a case needed
>>> to ensure whether generic or custom plan was chosen for specific
>>> queries in a development environment.
>> 
>> In your case, probably you had to ensure that the last multiple (or 
>> every)
>> executions chose generic or custom plan? If yes, I'm afraid that 
>> displaying
>> only the last plan mode is not enough for your case. No?
>> So it seems better to check generic_plans or custom_plans columns in 
>> the
>> view rather than last_plan_type even in your case. Thought?
> 
> Yeah, I now feel last_plan is not so necessary and only the numbers of
> generic/custom plan is enough.
> 
> If there are no objections, I'm going to remove this column and related 
> codes.

As mentioned, I removed last_plan column.


Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION