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

Thanks for writing a patch!

On Thu, May 21, 2020 at 5:10 PM Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
wrote:

> At Thu, 21 May 2020 12:18:16 +0900, Fujii Masao <
> masao(dot)fujii(at)oss(dot)nttdata(dot)com> 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 <atorik(at)gmail(dot)com>
> 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 <
tatsuro(dot)yamada(dot)tf(at)nttcom(dot)co(dot)jp> 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