On 2020-06-04 17:04, Atsushi Torikoshi wrote:
> On Mon, May 25, 2020 at 10:54 AM Atsushi Torikoshi <atorik@gmail.com>
> wrote:
>
>> On Thu, May 21, 2020 at 5:10 PM Kyotaro Horiguchi
>> <horikyota.ntt@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