Konstantin Knizhnik <k.knizhnik@postgrespro.ru> writes:
> [ autoprepare-extended-4.patch ]
The cfbot is showing that this doesn't apply anymore; there's
some doubtless-trivial conflict in prepare.c.
However ... TBH I've been skeptical of this whole proposal from the
beginning, on the grounds that it would probably hurt more use-cases
than it helps. The latest approach doesn't really do anything to
assuage that fear, because now that you've restricted it to extended
query mode, the feature amounts to nothing more than deliberately
overriding the application's choice to use unnamed rather than named
(prepared) statements. How often is that going to be a good idea?
And when it is, wouldn't it be better to fix the app? The client is
likely to have a far better idea of which statements would benefit
from this treatment than the server will; and in this context,
the client-side changes needed would really be trivial. The original
proposal, scary as it was, at least supported the argument that you
could use it to improve applications that were too dumb/hoary to
parameterize commands for themselves.
I'm also unimpressed by benchmark testing that relies entirely on
pgbench's default scenario, because that scenario consists entirely
of queries that are perfectly adapted to plan-only-once treatment.
In the real world, we constantly see complaints about cases where the
plancache mistakenly decides that a generic plan is acceptable. I think
that extending that behavior to more cases is going to be a net loss,
until we find some way to make it smarter and more reliable. At the
very least, you need to show some worst-case numbers alongside these
best-case numbers --- what happens with queries where we conclude we
must replan every time, so that the plancache becomes pure overhead?
The pgbench test case is laughably unrealistic in another way, in that
there are so few distinct queries it issues, so that there's no
stress at all on the cache-management aspect of this problem.
In short, I really think we ought to reject this patch and move on.
Maybe it could be resurrected sometime in the future when we have a
better handle on when to cache plans and when not.
If you want to press forward with it anyway, certainly the lack of
any tests in this patch is another big objection. Perhaps you
could create a pgbench TAP script that exercises the logic.
regards, tom lane
