From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> |
Cc: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: bugfix: BUG #15477: Procedure call with named inout refcursor parameter - "invalid input syntax for type boolean" |
Date: | 2018-11-04 19:26:51 |
Message-ID: | 13789.1541359611@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I wrote:
> BTW, it looks to me like ExecuteCallStmt trashes the passed-in CallStmt
> in cases where expand_function_arguments is not a no-op. Is that
> really safe? Seems to me it'd cause problems if, for example, dealing
> with a CallStmt that's part of a prepared stmt or cached plan.
I dug into why this wasn't falling over like mad, and the answer is that
the only reason it doesn't fall over is that plancache.c *always* decides
to build a new custom plan for any cached plan consisting strictly of
utility statements. That's because cached_plan_cost() ignores utility
statements altogether, causing both the generic and custom plan costs
to be exactly zero, and choose_custom_plan() figures it might as well
break a tie in favor of generating a custom plan. Therefore, even when
dealing with a cached CALL statement, ExecuteCallStmt always sees a
virgin new parse tree, and the fact that it corrupts it isn't a problem.
If I adjust the plancache logic so that it's capable of deciding to
use a generic cached plan, repeated execution of a CALL statement with
named parameters crashes at the seventh iteration, which is the first
one that would actually try to re-use a cached plan tree.
Nonetheless, make check-world passes, indicating that there is no place
in our regression tests that stresses this type of situation. (On the
other hand, if I set plan_cache_mode = force_generic_plan, kaboom.)
This leaves me worried that there are probably other places besides
CallStmt that think they can scribble on a utility statement's parse
tree at execution time. We're clearly not testing that situation
enough.
For the moment, I'm going to go fix ExecuteCallStmt so it doesn't do
this, but I'm wondering how we could detect such problems more
generically.
Also, it's just silly that the plancache won't use a generic plan for
utility statements, so I think we should change that (in HEAD only).
The easiest mod is to adjust cached_plan_cost() so that it charges
some nonzero amount for "planning" of utility statements.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | David Fetter | 2018-11-04 20:35:22 | Re: date_trunc() in a specific time zone |
Previous Message | Peter Geoghegan | 2018-11-04 18:58:19 | Re: Making all nbtree entries unique by having heap TIDs participate in comparisons |