From: | Masahiro Ikeda <ikedamsh(at)oss(dot)nttdata(dot)com> |
---|---|
To: | Atsushi Torikoshi <atorik(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, legrand legrand <legrand_legrand(at)hotmail(dot)com>, Tatsuro Yamada <tatsuro(dot)yamada(dot)tf(at)nttcom(dot)co(dot)jp> |
Subject: | Re: Is it useful to record whether plans are generic or custom? |
Date: | 2020-06-08 11:45:15 |
Message-ID: | ee7e5455d039b00849c62e1172d8035c@oss.nttdata.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2020-06-04 17:04, Atsushi Torikoshi wrote:
> On Mon, May 25, 2020 at 10:54 AM Atsushi Torikoshi <atorik(at)gmail(dot)com>
> wrote:
>
>> On Thu, May 21, 2020 at 5:10 PM Kyotaro Horiguchi
>> <horikyota(dot)ntt(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2020-06-08 12:44:31 | TAP tests and symlinks on Windows |
Previous Message | Asif Rehman | 2020-06-08 11:39:18 | Re: proposal - plpgsql - FOR over unbound cursor |