Re: Is it useful to record whether plans are generic or custom? - Mailing list pgsql-hackers

From Atsushi Torikoshi
Subject Re: Is it useful to record whether plans are generic or custom?
Date
Msg-id CACZ0uYEXTN6XFv9p-guBW8RrQKWazFhk9t0Onaz2BnXOPuEQiQ@mail.gmail.com
Whole thread Raw
In response to Re: Is it useful to record whether plans are generic or custom?  (Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>)
Responses Re: Is it useful to record whether plans are generic or custom?  (Atsushi Torikoshi <atorik@gmail.com>)
List pgsql-hackers
Thanks for writing a patch!

On Thu, May 21, 2020 at 5:10 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
At Thu, 21 May 2020 12:18:16 +0900, Fujii Masao <masao.fujii@oss.nttdata.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@gmail.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.yamada.tf@nttcom.co.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
Attachment

pgsql-hackers by date:

Previous
From: Justin Pryzby
Date:
Subject: Re: Failure to create GiST on ltree column
Next
From: Atsushi Torikoshi
Date:
Subject: Re: Add explanations which are influenced by track_io_timing