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

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

> 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

At Tue, 19 May 2020 22:56:17 +0900, Atsushi Torikoshi <[hidden email]> wrote in
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

On 2020/05/20 21:56, Atsushi Torikoshi wrote:
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

At Thu, 21 May 2020 12:18:16 +0900, Fujii Masao <[hidden email]> wrote in 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

Hi Torikoshi-san!

On 2020/05/21 17:10, Kyotaro Horiguchi wrote: 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

On 2020-06-04 17:04, Atsushi Torikoshi wrote:
As a user, I think this feature is useful for performance analysis.
I hope that this feature is merged.

BTW, I found that the dependency between function's comments and
the modified code is broken at latest patch. Before this is
committed, please fix it.

```
diff --git a/src/backend/commands/prepare.c
b/src/backend/commands/prepare.c
index 990782e77f..b63d3214df 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, last_plan).
   */
  Datum
  pg_prepared_statement(PG_FUNCTION_ARGS)
```

Regards,

--
Masahiro Ikeda

On 2020-06-08 20:45, Masahiro Ikeda wrote:
Thanks for reviewing!

I've fixed it.



Regards,

--
Atsushi Torikoshi

At Wed, 10 Jun 2020 10:50:58 +0900, torikoshia <[hidden email]> wrote in
+ 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.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On 2020-06-10 18:00, Kyotaro Horiguchi wrote:
Thanks for your reviewing!

I've attached a patch that reflects your comments.


Regards,

--
Atsushi Torikoshi

On 2020/06/11 14:59, torikoshia wrote:
Thanks for the patch! Here are the comments.


+        Number of times generic plan was choosen
+        Number of times custom plan was choosen

Typo: "choosen" should be "chosen"?


+      <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.


Regards,

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

On 2020-07-06 22:16, Fujii Masao wrote: 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.
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.

Of course, we can know it from adding EXPLAIN and ensuring whether $n
is contained in the plan, but I feel using the view is easier to use
and understand.


Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION

On 2020/07/08 10:14, torikoshia wrote:
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?

Regards,

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

On 2020-07-08 16:41, Fujii Masao wrote:
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.


Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION