Konstantin Knizhnik <k.knizh...@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