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

On Fri, Feb 09, 2018 at 12:02:57PM +0100, Pavel Stehule wrote:
> 2018-02-09 7:56 GMT+01:00 Michael Paquier <michael(at)paquier(dot)xyz>:
> > The second problem involves a cache lookup failure for a type when
> > trying to use pg_get_functiondef on a procedure. Luckily, it is
> > possible to make the difference between a procedure and a function by
> > checking if prorettype is InvalidOid or not. There is room for a new
> > patch which supports pg_get_proceduredef() to generate the definition of
> > a procedure, with perhaps a dedicated psql shortcut, but that could
> > always be done later on.
>
> Blocking subqueries in CALL parameters is possible solution.

ExecuteCallStmt has visibly been written so as it is able to handle the
input set of arguments with a minimal infrastructure in place. SubLink
nodes require more advanced handling as those need to go through the
planner. There is also additional processing in the rewriter. At the
end I tend to think that any user would just turn their back on calling
a function for such cases anyway, so it seems to me that the potential
benefits are not worth the code complexity.

> But blocking
> func def for procedures without any substitution doesn't look correct for
> me.

I don't disagree with you here, there is room for such a function, but
on the other side not having it does not make the existing feature less
useful. As it is Peter's and Andrew's feature, the last word should
come from them. Here is my opinion for what it's worth:
- Procedures are not functions, the code is pretty clear about that, so
a system function to generate the definition of a procedure should not
happen with pg_get_functiondef(). They share a lot of infrastructure so
you can reuse a lot of the code present.
- A different psql shortcut should be used, and I would assume that \sp
is adapted.
- Aggregates are also in pg_proc, we generate an error on them because
they are of different type, so an error for procedures when trying to
fetch a functoin definition looks like the good answer.

If folks feel that having a way to retrieve the procedure definition
easily via ruleutils.c is a must-have, then we have material for a good
open item :)
--
Michael