On 2020-06-04 17:04, Atsushi Torikoshi wrote:
On Mon, May 25, 2020 at 10:54 AM Atsushi Torikoshi <ato...@gmail.com>
wrote:

On Thu, May 21, 2020 at 5:10 PM Kyotaro Horiguchi
<horikyota....@gmail.com> wrote:

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

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'?

I've modified the above points and also exposed the numbers of each
 generic plans and custom plans.

I'm now a little bit worried about the below change which removed
the overflow checking for num_custom_plans, which was introduced
in 0001-Expose-counters-of-plancache-to-pg_prepared_statement.patch,
but I've left it because the maximum of int64 seems enough large
for counters.
Also referencing other counters in pg_stat_user_tables, they don't
seem to care about it.

```
-               /* 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++;
-               }

```

Regards,

Atsushi Torikoshi



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


Reply via email to