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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Atsushi Torikoshi
Hi,

When we run prepared statements, as far as I know, running
EXPLAIN is the only way to know whether executed plans are
generic or custom.
There seems no way to know how many times a prepared
statement was executed as generic and custom.

I think it may be useful to record the number of generic
and custom plans mainly to ensure the prepared statements
are executed as expected plan type.
If people also feel it's useful,  I'm going to think about adding
columns such as 'generic plans' and 'custom plans' to
pg_stat_statements.

As you know, pg_stat_statements can now track not only
'calls' but 'plans', so we can presume which plan type
was executed from them.
When both 'calls' and 'plans' were incremented, plan
type would be custom. When only 'calls' was incremented,
it would be generic.
But considering the case such as only the plan phase has
succeeded and the execution phase has failed, this
presumption can be wrong.

Thoughts?


Regards,

--
Atsushi Torikoshi
Reply | Threaded
Open this post in threaded view
|

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

legrand legrand
Hello,

yes this can be usefull, under the condition of differentiating all the
counters
for a queryid using a generic plan and the one using a custom one.

For me one way to do that is adding a generic_plan column to
pg_stat_statements key, someting like:
- userid,
- dbid,
- queryid,
- generic_plan

I don't know if generic/custom plan types are available during planning
and/or
execution hooks ...

Regards
PAscal




--
Sent from: https://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html


Reply | Threaded
Open this post in threaded view
|

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

Atsushi Torikoshi
On Thu, May 14, 2020 at 2:28 AM legrand legrand <[hidden email]> wrote:
Hello,

yes this can be usefull, under the condition of differentiating all the
counters
for a queryid using a generic plan and the one using a custom one.

For me one way to do that is adding a generic_plan column to
pg_stat_statements key, someting like:
- userid,
- dbid,
- queryid,
- generic_plan

Thanks for your kind advice!
 
I don't know if generic/custom plan types are available during planning
and/or
execution hooks ...

Yeah, that's what I'm concerned about.

As far as I can see, there are no variables tracking executed
plan types so we may need to add variables in
CachedPlanSource or somewhere.

CachedPlanSource.num_custom_plans looked like what is needed,
but it is the number of PLANNING so it also increments when
the planner calculates both plans and decides to take generic
plan.


To track executed plan types, I think execution layer hooks
are appropriate.
These hooks, however, take QueryDesc as a param and it does
not include cached plan information.
Portal includes CachedPlanSource but there seem no hooks to
take Portal.

So I'm wondering it's necessary to add a hook to get Portal
or CachedPlanSource.
Are these too much change for getting plan types?


Regards,

--
Atsushi Torikoshi
Reply | Threaded
Open this post in threaded view
|

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

legrand legrand
> To track executed plan types, I think execution layer hooks
> are appropriate.
> These hooks, however, take QueryDesc as a param and it does
> not include cached plan information.

It seems that the same QueryDesc entry is reused when executing
a generic plan.
For exemple marking queryDesc->plannedstmt->queryId (outside
pg_stat_statements) with a pseudo tag during ExecutorStart
reappears in later executions with generic plans ...

Is this QueryDesc reusable by a custom plan ? If not maybe a solution
could be to add a flag in queryDesc->plannedstmt ?

(sorry, I haven't understood yet how and when this generic plan is
managed during planning)

Regards
PAscal



--
Sent from: https://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html


Reply | Threaded
Open this post in threaded view
|

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

Atsushi Torikoshi
On Sat, May 16, 2020 at 6:01 PM legrand legrand <[hidden email]> wrote:
> To track executed plan types, I think execution layer hooks
> are appropriate.
> These hooks, however, take QueryDesc as a param and it does
> not include cached plan information.

It seems that the same QueryDesc entry is reused when executing
a generic plan.
For exemple marking queryDesc->plannedstmt->queryId (outside
pg_stat_statements) with a pseudo tag during ExecutorStart
reappears in later executions with generic plans ...

Is this QueryDesc reusable by a custom plan ? If not maybe a solution
could be to add a flag in queryDesc->plannedstmt ?

Thanks for your proposal!

I first thought it was a good idea and tried to add a flag to QueryDesc,
but the comments on QueryDesc say it encapsulates everything that
the executor needs to execute a query.

Whether a plan is generic or custom is not what executor needs to
know for running queries, so now I hesitate to do so.

Instead, I'm now considering using a static hash for prepared queries
(static HTAB *prepared_queries).


BTW, I'd also appreciate other opinions about recording the number
of generic and custom plans on pg_stat_statemtents. 


Regards,

--
Atsushi Torikoshi
Reply | Threaded
Open this post in threaded view
|

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

Kyotaro Horiguchi-4
At Tue, 19 May 2020 22:56:17 +0900, Atsushi Torikoshi <[hidden email]> wrote in

> On Sat, May 16, 2020 at 6:01 PM legrand legrand <[hidden email]>
> wrote:
>
> > > To track executed plan types, I think execution layer hooks
> > > are appropriate.
> > > These hooks, however, take QueryDesc as a param and it does
> > > not include cached plan information.
> >
> > It seems that the same QueryDesc entry is reused when executing
> > a generic plan.
> > For exemple marking queryDesc->plannedstmt->queryId (outside
> > pg_stat_statements) with a pseudo tag during ExecutorStart
> > reappears in later executions with generic plans ...
> >
> > Is this QueryDesc reusable by a custom plan ? If not maybe a solution
> > could be to add a flag in queryDesc->plannedstmt ?
> >
>
> Thanks for your proposal!
>
> I first thought it was a good idea and tried to add a flag to QueryDesc,
> but the comments on QueryDesc say it encapsulates everything that
> the executor needs to execute a query.
>
> Whether a plan is generic or custom is not what executor needs to
> know for running queries, so now I hesitate to do so.
>
> Instead, I'm now considering using a static hash for prepared queries
> (static HTAB *prepared_queries).
>
>
> BTW, I'd also appreciate other opinions about recording the number
> of generic and custom plans on pg_stat_statemtents.

If you/we just want to know how a prepared statement is executed,
couldn't we show that information in pg_prepared_statements view?

=# select * from pg_prepared_statements;
-[ RECORD 1 ]---+----------------------------------------------------
name            | stmt1
statement       | prepare stmt1 as select * from t where b = $1;
prepare_time    | 2020-05-20 12:01:55.733469+09
parameter_types | {text}
from_sql        | t
exec_custom     | 5    <- existing num_custom_plans
exec_total    | 40   <- new member of CachedPlanSource

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center


Reply | Threaded
Open this post in threaded view
|

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

Atsushi Torikoshi

On Wed, May 20, 2020 at 1:32 PM Kyotaro Horiguchi <[hidden email]> wrote:
At Tue, 19 May 2020 22:56:17 +0900, Atsushi Torikoshi <[hidden email]> wrote in
> On Sat, May 16, 2020 at 6:01 PM legrand legrand <[hidden email]>
> wrote:
>
> BTW, I'd also appreciate other opinions about recording the number
> of generic and custom plans on pg_stat_statemtents.

If you/we just want to know how a prepared statement is executed,
couldn't we show that information in pg_prepared_statements view?

=# select * from pg_prepared_statements;
-[ RECORD 1 ]---+----------------------------------------------------
name            | stmt1
statement       | prepare stmt1 as select * from t where b = $1;
prepare_time    | 2020-05-20 12:01:55.733469+09
parameter_types | {text}
from_sql        | t
exec_custom     | 5    <- existing num_custom_plans
exec_total          | 40   <- new member of CachedPlanSource

Thanks, Horiguchi-san!

Adding counters to pg_prepared_statements seems useful when we want
to know the way prepared statements executed in the current session.

And I also feel adding counters to pg_stat_statements will be convenient
especially in production environments because it enables us to get
information about not only the current session but all sessions of a
PostgreSQL instance.

If both changes are worthwhile, considering implementation complexity,
it may be reasonable to firstly add columns to pg_prepared_statements
and then work on pg_stat_statements.
 
Regards,

--
Atsushi Torikoshi
Reply | Threaded
Open this post in threaded view
|

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

Fujii Masao-4


On 2020/05/20 21:56, Atsushi Torikoshi wrote:

>
> On Wed, May 20, 2020 at 1:32 PM Kyotaro Horiguchi <[hidden email] <mailto:[hidden email]>> wrote:
>
>     At Tue, 19 May 2020 22:56:17 +0900, Atsushi Torikoshi <[hidden email] <mailto:[hidden email]>> wrote in
>      > On Sat, May 16, 2020 at 6:01 PM legrand legrand <[hidden email] <mailto:[hidden email]>>
>      > wrote:
>      >
>      > BTW, I'd also appreciate other opinions about recording the number
>      > of generic and custom plans on pg_stat_statemtents.
>
>     If you/we just want to know how a prepared statement is executed,
>     couldn't we show that information in pg_prepared_statements view?
>
>     =# select * from pg_prepared_statements;
>     -[ RECORD 1 ]---+----------------------------------------------------
>     name            | stmt1
>     statement       | prepare stmt1 as select * from t where b = $1;
>     prepare_time    | 2020-05-20 12:01:55.733469+09
>     parameter_types | {text}
>     from_sql        | t
>     exec_custom     | 5    <- existing num_custom_plans
>     exec_total          | 40   <- new member of CachedPlanSource
>
>
> Thanks, Horiguchi-san!
>
> Adding counters to pg_prepared_statements seems useful when we want
> to know the way prepared statements executed in the current session.

I like the idea exposing more CachedPlanSource fields in
pg_prepared_statements. I agree it's useful, e.g., for the debug purpose.
This is why I implemented the similar feature in my extension.
Please see [1] for details.

> And I also feel adding counters to pg_stat_statements will be convenient
> especially in production environments because it enables us to get
> information about not only the current session but all sessions of a
> PostgreSQL instance.

+1

Regards,

[1]
https://github.com/MasaoFujii/pg_cheat_funcs#record-pg_cached_plan_sourcestmt-text


--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION


Reply | Threaded
Open this post in threaded view
|

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

Kyotaro Horiguchi-4
At Thu, 21 May 2020 12:18:16 +0900, Fujii Masao <[hidden email]> wrote in

>
>
> On 2020/05/20 21:56, Atsushi Torikoshi wrote:
> > On Wed, May 20, 2020 at 1:32 PM Kyotaro Horiguchi
> > <[hidden email] <mailto:[hidden email]>> wrote:
> >     At Tue, 19 May 2020 22:56:17 +0900, Atsushi Torikoshi
> >     <[hidden email] <mailto:[hidden email]>> wrote in
> >      > On Sat, May 16, 2020 at 6:01 PM legrand legrand
> >      > <[hidden email] <mailto:[hidden email]>>
> >      > wrote:
> >      >
> >      > BTW, I'd also appreciate other opinions about recording the number
> >      > of generic and custom plans on pg_stat_statemtents.
> >     If you/we just want to know how a prepared statement is executed,
> >     couldn't we show that information in pg_prepared_statements view?
> >     =# select * from pg_prepared_statements;
> >     -[ RECORD 1 ]---+----------------------------------------------------
> >     name            | stmt1
> >     statement       | prepare stmt1 as select * from t where b = $1;
> >     prepare_time    | 2020-05-20 12:01:55.733469+09
> >     parameter_types | {text}
> >     from_sql        | t
> >     exec_custom     | 5    <- existing num_custom_plans
> >     exec_total          | 40   <- new member of CachedPlanSource
> > Thanks, Horiguchi-san!
> > Adding counters to pg_prepared_statements seems useful when we want
> > to know the way prepared statements executed in the current session.
>
> I like the idea exposing more CachedPlanSource fields in
> pg_prepared_statements. I agree it's useful, e.g., for the debug
> purpose.
> This is why I implemented the similar feature in my extension.
> Please see [1] for details.
Thanks. I'm not sure plan_cache_mode should be a part of the view.
Cost numbers would look better if it is cooked a bit.  Is it worth
being in core?

=# select * from pg_prepared_statements;
-[ RECORD 1 ]---+--------------------------------------------
name            | p1
statement       | prepare p1 as select a from t where a = $1;
prepare_time    | 2020-05-21 15:41:50.419578+09
parameter_types | {integer}
from_sql        | t
calls           | 7
custom_calls    | 5
plan_generation | 6
generic_cost    | 4.3100000000000005
custom_cost     | 9.31

Perhaps plan_generation is not needed there.

> > And I also feel adding counters to pg_stat_statements will be
> > convenient
> > especially in production environments because it enables us to get
> > information about not only the current session but all sessions of a
> > PostgreSQL instance.
>
> +1

Agreed. It is global and persistent.

At Tue, 19 May 2020 22:56:17 +0900, Atsushi Torikoshi <[hidden email]> wrote in
> Instead, I'm now considering using a static hash for prepared queries
> (static HTAB *prepared_queries).

That might be complex and fragile considering nested query and SPI
calls.  I'm not sure, but could we use ActivePortal?
ActivePortal->cplan is a CachedPlan, which can hold the generic/custom
information.

Instead,


> [1]
> https://github.com/MasaoFujii/pg_cheat_funcs#record-pg_cached_plan_sourcestmt-text

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

From 1c6a3dd41a59504244134ee44ddd4516191656da Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <[hidden email]>
Date: Thu, 21 May 2020 15:32:38 +0900
Subject: [PATCH] Expose counters of plancache to pg_prepared_statements

We didn't have an easy way to find how many times generic or custom
plans of a prepared statement have been executed.  This patch exposes
such numbers and costs of both plans in pg_prepared_statements.
---
 doc/src/sgml/catalogs.sgml            | 45 +++++++++++++++++++++++++++
 src/backend/commands/prepare.c        | 30 ++++++++++++++++--
 src/backend/utils/cache/plancache.c   | 13 ++++----
 src/include/catalog/pg_proc.dat       |  6 ++--
 src/include/utils/plancache.h         |  5 +--
 src/test/regress/expected/prepare.out | 43 +++++++++++++++++++++++++
 src/test/regress/expected/rules.out   |  9 ++++--
 src/test/regress/sql/prepare.sql      |  9 ++++++
 8 files changed, 144 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index b1b077c97f..950ed30694 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -10826,6 +10826,51 @@ 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>calls</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of times the prepared statement was executed
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>custom_calls</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of times generic plan is executed for the prepared statement
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>plan_geneation</structfield> <type>bigint</type>
+      </para>
+      <para>
+       Number of times execution plan was generated for the prepared statement
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>generic_cost</structfield> <type>double precision</type>
+      </para>
+      <para>
+       Estimated cost of generic plan. NULL if not yet planned.
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>custom_cost</structfield> <type>double precision</type>
+      </para>
+      <para>
+       Estimated cost of custom plans. NULL if not yet planned. This is mean of the estimated costs for the past calls including planning cost.
+      </para></entry>
+     </row>
     </tbody>
    </tgroup>
   </table>
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index 80d6df8ac1..e6755df543 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -723,7 +723,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(10);
  TupleDescInitEntry(tupdesc, (AttrNumber) 1, "name",
    TEXTOID, -1, 0);
  TupleDescInitEntry(tupdesc, (AttrNumber) 2, "statement",
@@ -734,6 +734,16 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
    REGTYPEARRAYOID, -1, 0);
  TupleDescInitEntry(tupdesc, (AttrNumber) 5, "from_sql",
    BOOLOID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 6, "calls",
+   INT8OID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 7, "custom_calls",
+   INT8OID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 8, "plan_generation",
+   INT8OID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 9, "generic_cost",
+   FLOAT8OID, -1, 0);
+ TupleDescInitEntry(tupdesc, (AttrNumber) 10, "custom_cost",
+   FLOAT8OID, -1, 0);
 
  /*
  * We put all the tuples into a tuplestore in one scan of the hashtable.
@@ -755,8 +765,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[10];
+ bool nulls[10];
 
  MemSet(nulls, 0, sizeof(nulls));
 
@@ -766,6 +776,20 @@ 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_total_calls);
+ values[6] = Int64GetDatumFast(prep_stmt->plansource->num_custom_plans);
+ values[7] = Int64GetDatumFast(prep_stmt->plansource->generation);
+ if (prep_stmt->plansource->generic_cost >= 0.0)
+ values[8] = Float8GetDatum(prep_stmt->plansource->generic_cost);
+ else
+ nulls[8] = true;
+
+ if (prep_stmt->plansource->num_custom_plans > 0)
+ values[9] =
+ Float8GetDatum(prep_stmt->plansource->total_custom_cost /
+   prep_stmt->plansource->num_custom_plans);
+ else
+ nulls[9] = true;
 
  tuplestore_putvalues(tupstore, tupdesc, values, nulls);
  }
diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 75b475c179..d91444f60f 100644
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -286,6 +286,7 @@ CreateOneShotCachedPlan(RawStmt *raw_parse_tree,
  plansource->generic_cost = -1;
  plansource->total_custom_cost = 0;
  plansource->num_custom_plans = 0;
+ plansource->num_total_calls = 0;
 
  return plansource;
 }
@@ -1213,14 +1214,13 @@ 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++;
  }
 
+ plansource->num_total_calls++;
+
  Assert(plan != NULL);
 
  /* Flag the plan as in use by caller */
@@ -1575,6 +1575,7 @@ CopyCachedPlan(CachedPlanSource *plansource)
  newsource->generic_cost = plansource->generic_cost;
  newsource->total_custom_cost = plansource->total_custom_cost;
  newsource->num_custom_plans = plansource->num_custom_plans;
+ newsource->num_total_calls = plansource->num_total_calls;
 
  MemoryContextSwitchTo(oldcxt);
 
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 61f2c2f5b4..48d0d88d97 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -7743,9 +7743,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,int8,float8,float8}',
+  proargmodes => '{o,o,o,o,o,o,o,o,o,o}',
+  proargnames => '{name,statement,prepare_time,parameter_types,from_sql,calls,custom_calls,plan_generation,generic_cost,custom_cost}',
   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..146eb15d34 100644
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -124,13 +124,14 @@ typedef struct CachedPlanSource
  bool is_complete; /* has CompleteCachedPlan been done? */
  bool is_saved; /* has CachedPlanSource been "saved"? */
  bool is_valid; /* is the query_list currently valid? */
- int generation; /* increments each time we create a plan */
+ int64 generation; /* increments each time we create a plan */
  /* If CachedPlanSource has been saved, it is a member of a global list */
  dlist_node node; /* list link, if is_saved */
  /* 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_total_calls;/* total number of execution */
 } CachedPlanSource;
 
 /*
diff --git a/src/test/regress/expected/prepare.out b/src/test/regress/expected/prepare.out
index 3306c696b1..b41e75c275 100644
--- a/src/test/regress/expected/prepare.out
+++ b/src/test/regress/expected/prepare.out
@@ -64,6 +64,49 @@ EXECUTE q2('postgres');
  postgres | f             | t
 (1 row)
 
+EXECUTE q2('postgres');
+ datname  | datistemplate | datallowconn
+----------+---------------+--------------
+ postgres | f             | t
+(1 row)
+
+EXECUTE q2('postgres');
+ datname  | datistemplate | datallowconn
+----------+---------------+--------------
+ postgres | f             | t
+(1 row)
+
+EXECUTE q2('postgres');
+ datname  | datistemplate | datallowconn
+----------+---------------+--------------
+ postgres | f             | t
+(1 row)
+
+EXECUTE q2('postgres');
+ datname  | datistemplate | datallowconn
+----------+---------------+--------------
+ postgres | f             | t
+(1 row)
+
+EXECUTE q2('postgres');
+ datname  | datistemplate | datallowconn
+----------+---------------+--------------
+ postgres | f             | t
+(1 row)
+
+EXECUTE q2('postgres');
+ datname  | datistemplate | datallowconn
+----------+---------------+--------------
+ postgres | f             | t
+(1 row)
+
+-- the view should return the correct counts
+SELECT name, calls, custom_calls, plan_generation FROM pg_prepared_statements;
+ name | calls | custom_calls | plan_generation
+------+-------+--------------+-----------------
+ q2   |     7 |            5 |               6
+(1 row)
+
 PREPARE q3(text, int, float, boolean, smallint) AS
  SELECT * FROM tenk1 WHERE string4 = $1 AND (four = $2 OR
  ten = $3::bigint OR true = $4 OR odd = $5::int)
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index b813e32215..ff9ce88ce1 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1428,8 +1428,13 @@ 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.calls,
+    p.custom_calls,
+    p.plan_generation,
+    p.generic_cost,
+    p.custom_cost
+   FROM pg_prepared_statement() p(name, statement, prepare_time, parameter_types, from_sql, calls, custom_calls, plan_generation, generic_cost, custom_cost);
 pg_prepared_xacts| SELECT p.transaction,
     p.gid,
     p.prepared,
diff --git a/src/test/regress/sql/prepare.sql b/src/test/regress/sql/prepare.sql
index 985d0f05c9..9d73c2fea3 100644
--- a/src/test/regress/sql/prepare.sql
+++ b/src/test/regress/sql/prepare.sql
@@ -35,6 +35,15 @@ PREPARE q2(text) AS
  FROM pg_database WHERE datname = $1;
 
 EXECUTE q2('postgres');
+EXECUTE q2('postgres');
+EXECUTE q2('postgres');
+EXECUTE q2('postgres');
+EXECUTE q2('postgres');
+EXECUTE q2('postgres');
+EXECUTE q2('postgres');
+
+-- the view should return the correct counts
+SELECT name, calls, custom_calls, plan_generation FROM pg_prepared_statements;
 
 PREPARE q3(text, int, float, boolean, smallint) AS
  SELECT * FROM tenk1 WHERE string4 = $1 AND (four = $2 OR
--
2.18.2

Reply | Threaded
Open this post in threaded view
|

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

Tatsuro Yamada-2
Hi Torikoshi-san!

On 2020/05/21 17:10, Kyotaro Horiguchi wrote:

> At Thu, 21 May 2020 12:18:16 +0900, Fujii Masao <[hidden email]> wrote in
>>
>>
>> On 2020/05/20 21:56, Atsushi Torikoshi wrote:
>>> On Wed, May 20, 2020 at 1:32 PM Kyotaro Horiguchi
>>> <[hidden email] <mailto:[hidden email]>> wrote:
>>>      At Tue, 19 May 2020 22:56:17 +0900, Atsushi Torikoshi
>>>      <[hidden email] <mailto:[hidden email]>> wrote in
>>>       > On Sat, May 16, 2020 at 6:01 PM legrand legrand
>>>       > <[hidden email] <mailto:[hidden email]>>
>>>       > wrote:
>>>       >
>>>       > BTW, I'd also appreciate other opinions about recording the number
>>>       > of generic and custom plans on pg_stat_statemtents.
>>>      If you/we just want to know how a prepared statement is executed,
>>>      couldn't we show that information in pg_prepared_statements view?
>>>      =# select * from pg_prepared_statements;
>>>      -[ RECORD 1 ]---+----------------------------------------------------
>>>      name            | stmt1
>>>      statement       | prepare stmt1 as select * from t where b = $1;
>>>      prepare_time    | 2020-05-20 12:01:55.733469+09
>>>      parameter_types | {text}
>>>      from_sql        | t
>>>      exec_custom     | 5    <- existing num_custom_plans
>>>      exec_total          | 40   <- new member of CachedPlanSource
>>> Thanks, Horiguchi-san!
>>> Adding counters to pg_prepared_statements seems useful when we want
>>> to know the way prepared statements executed in the current session.
>>
>> I like the idea exposing more CachedPlanSource fields in
>> pg_prepared_statements. I agree it's useful, e.g., for the debug
>> purpose.
>> This is why I implemented the similar feature in my extension.
>> Please see [1] for details.
>
> Thanks. I'm not sure plan_cache_mode should be a part of the view.
> Cost numbers would look better if it is cooked a bit.  Is it worth
> being in core?
>
> =# select * from pg_prepared_statements;
> -[ RECORD 1 ]---+--------------------------------------------
> name            | p1
> statement       | prepare p1 as select a from t where a = $1;
> prepare_time    | 2020-05-21 15:41:50.419578+09
> parameter_types | {integer}
> from_sql        | t
> calls           | 7
> custom_calls    | 5
> plan_generation | 6
> generic_cost    | 4.3100000000000005
> custom_cost     | 9.31
>
> Perhaps plan_generation is not needed there.
I tried to creating PoC patch too, so I share it.
Please find attached file.

# Test case
prepare count as select count(*) from pg_class where oid >$1;
execute count(1); select * from pg_prepared_statements;

-[ RECORD 1 ]---+--------------------------------------------------------------
name            | count
statement       | prepare count as select count(*) from pg_class where oid >$1;
prepare_time    | 2020-05-21 17:41:16.134362+09
parameter_types | {oid}
from_sql        | t
is_generic_plan | f     <= False

You can see the following result, when you execute it 6 times.

-[ RECORD 1 ]---+--------------------------------------------------------------
name            | count
statement       | prepare count as select count(*) from pg_class where oid >$1;
prepare_time    | 2020-05-21 17:41:16.134362+09
parameter_types | {oid}
from_sql        | t
is_generic_plan | t     <= True


Thanks,
Tatsuro Yamada

poc_pg_prepared_statements.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Atsushi Torikoshi
Thanks for writing a patch!

On Thu, May 21, 2020 at 5:10 PM Kyotaro Horiguchi <[hidden email]> wrote:
At Thu, 21 May 2020 12:18:16 +0900, Fujii Masao <[hidden email]> wrote in
> I like the idea exposing more CachedPlanSource fields in
> pg_prepared_statements. I agree it's useful, e.g., for the debug
> purpose.
> This is why I implemented the similar feature in my extension.
> Please see [1] for details.

Thanks. I'm not sure plan_cache_mode should be a part of the view.
Cost numbers would look better if it is cooked a bit.  Is it worth
being in core?
 
I didn't come up with ideas about how to use them.
IMHO they might not so necessary..
  
=# select * from pg_prepared_statements;
-[ RECORD 1 ]---+--------------------------------------------
name            | p1
statement       | prepare p1 as select a from t where a = $1;
prepare_time    | 2020-05-21 15:41:50.419578+09
parameter_types | {integer}
from_sql        | t
calls           | 7
custom_calls    | 5
plan_generation | 6
generic_cost    | 4.3100000000000005
custom_cost     | 9.31

Perhaps plan_generation is not needed there.

+1.
Now 'calls' is sufficient and 'plan_generation' may confuse users.

BTW, considering 'calls' in pg_stat_statements is the number of times
statements were EXECUTED and 'plans' is the number of times
statements were PLANNED,  how about substituting 'calls' for 'plans'?


At Tue, 19 May 2020 22:56:17 +0900, Atsushi Torikoshi <[hidden email]> wrote in
> Instead, I'm now considering using a static hash for prepared queries
> (static HTAB *prepared_queries).

That might be complex and fragile considering nested query and SPI
calls.  I'm not sure, but could we use ActivePortal?
ActivePortal->cplan is a CachedPlan, which can hold the generic/custom
information.

Yes, I once looked for hooks which can get Portal, I couldn't find them.
And it also seems difficult getting keys for HTAB *prepared_queries
in existing executor hooks.
There may be oversights, but I'm now feeling returning to the idea
hook additions. 

| Portal includes CachedPlanSource but there seem no hooks to
| take Portal.
| So I'm wondering it's necessary to add a hook to get Portal
| or CachedPlanSource.
| Are these too much change for getting plan types?


On Thu, May 21, 2020 at 5:43 PM Tatsuro Yamada <[hidden email]> wrote:
I tried to creating PoC patch too, so I share it.
Please find attached file.

Thanks!

I agree with your idea showing the latest plan is generic or custom.

This patch judges whether the lastest plan was generic based on
plansource->gplan existence,  but plansource->gplan can exist even
when the planner chooses custom. 
For example, a prepared statement was executed first 6 times and
a generic plan was generated for comparison but the custom plan
won.

Attached another patch showing latest plan based on
'0001-Expose-counters-of-plancache-to-pg_prepared_statemen.patch'.

As I wrote above, I suppose some of the columns might not necessary
and it'd better change some column and variable names, but  I left them
for other opinions.

Regards,

--
Atsushi Torikoshi

0002-Expose-counters-of-plancache-to-pg_prepared_statement.patch (14K) Download Attachment