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?
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
From e3517479cea76e88f3ab8626d14452b36288dcb5 Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikos...@oss.nttdata.com>
Date: Tue, 14 Jul 2020 10:27:32 +0900
Subject: [PATCH] We didn't have an easy way to find how many times generic and
ustom plans of a prepared statement have been executed. This patch exposes
the numbers of both plans in pg_prepared_statements.
---
doc/src/sgml/catalogs.sgml | 18 ++++++++++++
src/backend/commands/prepare.c | 15 +++++++---
src/backend/utils/cache/plancache.c | 17 ++++++++----
src/include/catalog/pg_proc.dat | 6 ++--
src/include/utils/plancache.h | 3 +-
src/test/regress/expected/plancache.out | 37 ++++++++++++++++++++++++-
src/test/regress/expected/rules.out | 6 ++--
src/test/regress/sql/plancache.sql | 12 +++++++-
8 files changed, 96 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index e9cdff4864..8bc6d685f4 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -10830,6 +10830,24 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx
frontend/backend protocol
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>generic_plans</structfield> <type>int8</type>
+ </para>
+ <para>
+ Number of times generic plan was chosen
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>custom_plans</structfield> <type>int8</type>
+ </para>
+ <para>
+ Number of times custom plan was chosen
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 80d6df8ac1..4b18be5b27 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -694,7 +694,8 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es,
/*
* This set returning function reads all the prepared statements and
- * returns a set of (name, statement, prepare_time, param_types, from_sql).
+ * returns a set of (name, statement, prepare_time, param_types, from_sql,
+ * generic_plans, custom_plans).
*/
Datum
pg_prepared_statement(PG_FUNCTION_ARGS)
@@ -723,7 +724,7 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
* build tupdesc for result tuples. This must match the definition of the
* pg_prepared_statements view in system_views.sql
*/
- tupdesc = CreateTemplateTupleDesc(5);
+ tupdesc = CreateTemplateTupleDesc(7);
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name",
TEXTOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 2, "statement",
@@ -734,6 +735,10 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
REGTYPEARRAYOID, -1, 0);
TupleDescInitEntry(tupdesc, (AttrNumber) 5, "from_sql",
BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 6, "generic_plans",
+ INT8OID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7, "custom_plans",
+ INT8OID, -1, 0);
/*
* We put all the tuples into a tuplestore in one scan of the hashtable.
@@ -755,8 +760,8 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
hash_seq_init(&hash_seq, prepared_queries);
while ((prep_stmt = hash_seq_search(&hash_seq)) != NULL)
{
- Datum values[5];
- bool nulls[5];
+ Datum values[7];
+ bool nulls[7];
MemSet(nulls, 0, sizeof(nulls));
@@ -766,6 +771,8 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
values[3] = build_regtype_array(prep_stmt->plansource->param_types,
prep_stmt->plansource->num_params);
values[4] = BoolGetDatum(prep_stmt->from_sql);
+ values[5] = Int64GetDatumFast(prep_stmt->plansource->num_generic_plans);
+ values[6] = Int64GetDatumFast(prep_stmt->plansource->num_custom_plans);
tuplestore_putvalues(tupstore, tupdesc, values, nulls);
}
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 75b475c179..50d6ad28b4 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -218,6 +218,7 @@ CreateCachedPlan(RawStmt *raw_parse_tree,
plansource->generation = 0;
plansource->generic_cost = -1;
plansource->total_custom_cost = 0;
+ plansource->num_generic_plans = 0;
plansource->num_custom_plans = 0;
MemoryContextSwitchTo(oldcxt);
@@ -285,6 +286,7 @@ CreateOneShotCachedPlan(RawStmt *raw_parse_tree,
plansource->generation = 0;
plansource->generic_cost = -1;
plansource->total_custom_cost = 0;
+ plansource->num_generic_plans = 0;
plansource->num_custom_plans = 0;
return plansource;
@@ -1213,12 +1215,14 @@ GetCachedPlan(CachedPlanSource *plansource, ParamListInfo boundParams,
{
/* Build a custom plan */
plan = BuildCachedPlan(plansource, qlist, boundParams, queryEnv);
- /* Accumulate total costs of custom plans, but 'ware overflow */
- if (plansource->num_custom_plans < INT_MAX)
- {
- plansource->total_custom_cost += cached_plan_cost(plan, true);
- plansource->num_custom_plans++;
- }
+ /* Accumulate total costs of custom plans */
+ plansource->total_custom_cost += cached_plan_cost(plan, true);
+
+ plansource->num_custom_plans++;
+ }
+ else
+ {
+ plansource->num_generic_plans++;
}
Assert(plan != NULL);
@@ -1574,6 +1578,7 @@ CopyCachedPlan(CachedPlanSource *plansource)
/* We may as well copy any acquired cost knowledge */
newsource->generic_cost = plansource->generic_cost;
newsource->total_custom_cost = plansource->total_custom_cost;
+ newsource->num_generic_plans = plansource->num_generic_plans;
newsource->num_custom_plans = plansource->num_custom_plans;
MemoryContextSwitchTo(oldcxt);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 95604e988a..4b5af32440 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -7755,9 +7755,9 @@
{ oid => '2510', descr => 'get the prepared statements for this session',
proname => 'pg_prepared_statement', prorows => '1000', proretset => 't',
provolatile => 's', proparallel => 'r', prorettype => 'record',
- proargtypes => '', proallargtypes => '{text,text,timestamptz,_regtype,bool}',
- proargmodes => '{o,o,o,o,o}',
- proargnames => '{name,statement,prepare_time,parameter_types,from_sql}',
+ proargtypes => '', proallargtypes => '{text,text,timestamptz,_regtype,bool,int8,int8}',
+ proargmodes => '{o,o,o,o,o,o,o}',
+ proargnames => '{name,statement,prepare_time,parameter_types,from_sql,generic_plans,custom_plans}',
prosrc => 'pg_prepared_statement' },
{ oid => '2511', descr => 'get the open cursors for this session',
proname => 'pg_cursor', prorows => '1000', proretset => 't',
diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 522020d763..4901568553 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -130,7 +130,8 @@ typedef struct CachedPlanSource
/* State kept to help decide whether to use custom or generic plans: */
double generic_cost; /* cost of generic plan, or -1 if not known */
double total_custom_cost; /* total cost of custom plans so far */
- int num_custom_plans; /* number of plans included in total */
+ int64 num_custom_plans; /* # of custom plans included in total */
+ int64 num_generic_plans; /* # of generic plans */
} CachedPlanSource;
/*
diff --git a/src/test/regress/expected/plancache.out b/src/test/regress/expected/plancache.out
index 7d289b8c5e..c2513306fe 100644
--- a/src/test/regress/expected/plancache.out
+++ b/src/test/regress/expected/plancache.out
@@ -278,12 +278,19 @@ drop table pc_list_part_1;
execute pstmt_def_insert(1);
drop table pc_list_parted, pc_list_part_null;
deallocate pstmt_def_insert;
--- Test plan_cache_mode
+-- Test plan_cache_mode and the numbers of custom and generic plan
create table test_mode (a int);
insert into test_mode select 1 from generate_series(1,1000) union all select 2;
create index on test_mode (a);
analyze test_mode;
prepare test_mode_pp (int) as select count(*) from test_mode where a = $1;
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
+ name | generic_plans | custom_plans
+--------------+---------------+--------------
+ test_mode_pp | 0 | 0
+(1 row)
+
-- up to 5 executions, custom plan is used
explain (costs off) execute test_mode_pp(2);
QUERY PLAN
@@ -293,6 +300,13 @@ explain (costs off) execute test_mode_pp(2);
Index Cond: (a = 2)
(3 rows)
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
+ name | generic_plans | custom_plans
+--------------+---------------+--------------
+ test_mode_pp | 0 | 1
+(1 row)
+
-- force generic plan
set plan_cache_mode to force_generic_plan;
explain (costs off) execute test_mode_pp(2);
@@ -303,6 +317,13 @@ explain (costs off) execute test_mode_pp(2);
Filter: (a = $1)
(3 rows)
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
+ name | generic_plans | custom_plans
+--------------+---------------+--------------
+ test_mode_pp | 1 | 1
+(1 row)
+
-- get to generic plan by 5 executions
set plan_cache_mode to auto;
execute test_mode_pp(1); -- 1x
@@ -329,12 +350,26 @@ execute test_mode_pp(1); -- 4x
1000
(1 row)
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
+ name | generic_plans | custom_plans
+--------------+---------------+--------------
+ test_mode_pp | 1 | 5
+(1 row)
+
execute test_mode_pp(1); -- 5x
count
-------
1000
(1 row)
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
+ name | generic_plans | custom_plans
+--------------+---------------+--------------
+ test_mode_pp | 2 | 5
+(1 row)
+
-- we should now get a really bad plan
explain (costs off) execute test_mode_pp(2);
QUERY PLAN
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index fa436f2caa..601734a6f1 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1428,8 +1428,10 @@ pg_prepared_statements| SELECT p.name,
p.statement,
p.prepare_time,
p.parameter_types,
- p.from_sql
- FROM pg_prepared_statement() p(name, statement, prepare_time, parameter_types, from_sql);
+ p.from_sql,
+ p.generic_plans,
+ p.custom_plans
+ FROM pg_prepared_statement() p(name, statement, prepare_time, parameter_types, from_sql, generic_plans, custom_plans);
pg_prepared_xacts| SELECT p.transaction,
p.gid,
p.prepared,
diff --git a/src/test/regress/sql/plancache.sql b/src/test/regress/sql/plancache.sql
index fa218c8d21..d176cb66bd 100644
--- a/src/test/regress/sql/plancache.sql
+++ b/src/test/regress/sql/plancache.sql
@@ -178,7 +178,7 @@ execute pstmt_def_insert(1);
drop table pc_list_parted, pc_list_part_null;
deallocate pstmt_def_insert;
--- Test plan_cache_mode
+-- Test plan_cache_mode and the numbers of custom and generic plan
create table test_mode (a int);
insert into test_mode select 1 from generate_series(1,1000) union all select 2;
@@ -186,13 +186,19 @@ create index on test_mode (a);
analyze test_mode;
prepare test_mode_pp (int) as select count(*) from test_mode where a = $1;
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
-- up to 5 executions, custom plan is used
explain (costs off) execute test_mode_pp(2);
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
-- force generic plan
set plan_cache_mode to force_generic_plan;
explain (costs off) execute test_mode_pp(2);
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
-- get to generic plan by 5 executions
set plan_cache_mode to auto;
@@ -200,7 +206,11 @@ execute test_mode_pp(1); -- 1x
execute test_mode_pp(1); -- 2x
execute test_mode_pp(1); -- 3x
execute test_mode_pp(1); -- 4x
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
execute test_mode_pp(1); -- 5x
+select name, generic_plans, custom_plans from pg_prepared_statements
+ where name = 'test_mode_pp';
-- we should now get a really bad plan
explain (costs off) execute test_mode_pp(2);
--
2.18.1
