On 2020-07-20 11:57, Fujii Masao wrote:
On 2020/07/17 16:25, Fujii Masao wrote:
On 2020/07/16 11:50, torikoshia wrote:
On 2020-07-15 11:44, Fujii Masao wrote:
On 2020/07/14 21:24, torikoshia wrote:
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.
Thanks for updating the patch! It basically looks good to me.
I have one comment; you added the regression tests for generic and
custom plans into prepare.sql. But the similar tests already exist
in
plancache.sql. So isn't it better to add the tests for generic_plans
and
custom_plans columns, into plancache.sql?
Thanks for your comments!
Agreed.
I removed tests on prepare.sql and added them to plancache.sql.
Thoughts?
Thanks for updating the patch!
I also applied the following minor changes to the patch.
- Number of times generic plan was chosen
+ Number of times generic plan was chosen
- Number of times custom plan was chosen
+ Number of times custom plan was chosen
I got rid of one space character before those descriptions because
they should start from the position of 7th character.
-- but we can force a custom plan
set plan_cache_mode to force_custom_plan;
explain (costs off) execute test_mode_pp(2);
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
In the regression test, I added the execution of
pg_prepared_statements
after the last execution of test query, to confirm that custom plan is
used
when force_custom_plan is set, by checking from
pg_prepared_statements.
I changed the status of this patch to "Ready for Committer" in CF.
Barring any objection, I will commit this patch.
Committed. Thanks!
Thanks!
As I proposed earlier in this thread, I'm now trying to add information
about generic/cudstom plan to pg_stat_statements.
I'll share the idea and the poc patch soon.
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
