Re: CALL stmt, ERROR: unrecognized node type: 113 bug

Michael Paquier <michael(at)paquier(dot)xyz> writes:
> On Fri, Feb 09, 2018 at 08:30:49AM -0800, Andres Freund wrote:
>> On 2018-02-09 09:42:41 -0500, Tom Lane wrote:
>>> ... But otherwise it's an OK restriction that stems from
>>> exactly the same cause: we do not want to invoke the full planner in
>>> this context (and even if we did, we don't want to use the full
>>> executor to execute the result).

So I idly looked at ExecuteCallStmt to see how, in fact, it is executing
these things, and I was right to guess that it couldn't possibly support
a sub-select, since it's just using ExecEvalExprSwitchContext. So we
need to go teach transformSubLink that EXPR_KIND_CALL is *not* okay,
which is simple enough.

However, I also wondered how ExecuteCallStmt works at all for pass-by-
reference datatypes, since it immediately destroys the execution context
for each expression. And the answer is that it doesn't, as proven here:

regression=# create procedure myp(f1 text) as $$begin
regression$# raise notice 'f1 = %', f1;
regression$# end$$ language plpgsql;
CREATE PROCEDURE
regression=# call myp('xyzzy');
NOTICE: f1 = xyzzy
CALL
regression=# call myp('xyzzy' || 'x');
NOTICE: f1 =
CALL

The call with a literal constant accidentally seems to work, because the
returned Datum is actually pointing into the original expression tree.
But if you have the expression do any actual work, then not so much.

I think this could be fixed by evaluating all the arguments in a single
execution context that is not destroyed till after the call finishes
(using a separate one for each argument seems pretty silly anyway).
However, the code could do with more than zero commentary about how come
this is safe at all --- even if we keep the execution context, it's still
a child of whatever random memory context ExecuteCallStmt was called in,
and it's not very clear that that context will survive a transaction
commit.

This does not leave me with a warm feeling about either the amount
of testing the procedure feature has gotten, or the state of its
internal documentation.

regards, tom lane