Re: [HACKERS] Cached plans and statement generalization - Mailing list pgsql-hackers

On 02.05.2017 21:26, Robert Haas wrote:
>
> I am sympathetic to the fact that this is a hard problem to solve.
> I'm just telling you that the way you've got it is not acceptable and
> nobody's going to commit it like that (or if they do, they will end up
> having to revert it).  If you want to have a technical discussion
> about what might be a way to change the patch to be more acceptable,
> cool, but I don't want to get into a long debate about whether what
> you have is acceptable or not; I've already said what I think about
> that and I believe that opinion will be widely shared.  I am not
> trying to beat you up here, just trying to be clear.
>
>

Based on the Robert's feedback and Tom's proposal I have implemented two 
new versions of autoprepare patch.

First version is just refactoring of my original implementation: I have 
extracted common code into prepare_cached_plan and exec_prepared_plan
function to avoid code duplication. Also I rewrote assignment of values 
to parameters. Now types of parameters are inferred from types of 
literals, so there may be several
prepared plans which are different only by types of parameters. Due to 
the problem with type coercion for parameters, I have to catch errors in 
parse_analyze_varparams.

Robert, can you please explain why using TRY/CATCH is not safe here:
> This is definitely not a safe way of using TRY/CATCH.

Second version is based on Tom's suggestion:
> Personally I'd think about
> replacing the entire literal-with-cast construct with a Param having
> already-known type.
So here I first patch raw parse tree, replacing A_Const with ParamRef. 
Such plan is needed to perform cache lookup.
Then I restore original raw parse tree and call pg_analyze_and_rewrite. 
Then I mutate analyzed tree, replacing Const with Param nodes.
In this case type coercion is already performed and I know precise types 
which should be used for parameters.
It seems to be more sophisticated approach. And I can not extract common 
code in prepare_cached_plan,
because preparing of plan is currently mix of steps done in 
exec_simple_query and exec_parse_message.
But there is no need to catch analyze errors.

Finally performance of both approaches is the same: at pgbench it is 
180k TPS on read-only queries, comparing with 80k TPS for not prepared 
queries.
In both cases 7 out of  177 regression tests  are not passed (mostly 
because session environment is changed between subsequent query execution).

I am going to continue work on this patch I will be glad to receive any 
feedback and suggestions for its improvement.
In most cases, applications are not accessing Postgres directly, but 
using some connection pooling layer and so them are not able to use 
prepared statements.
But at simple OLTP Postgres spent more time on building query plan than 
on execution itself. And it is possible to speedup Postgres about two 
times at such workload!
Another alternative is true shared plan cache.  May be it is even more 
perspective approach, but definitely much more invasive and harder to 
implement.

-- 
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers