Re: Cached plans and statement generalization

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