Re: CALL versus procedures with output-only arguments

On Tue, May 25, 2021 at 2:20 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Just to enlarge on that point a bit:
>
> regression=# create function foo(int, out int) language sql
> regression-# as 'select $1';
> CREATE FUNCTION
> regression=# create procedure foo(int, out int) language sql
> regression-# as 'select $1';
> CREATE PROCEDURE
>
> IMO this should have failed, but since it doesn't:
>
> regression=# drop routine foo(int, out int);
> DROP ROUTINE
>
> Which object was dropped, and what is the argument for that one
> being the right one?
>
> Experinentation shows that in HEAD, what is dropped is the procedure,
> and indeed the DROP will fail if you try to use it on the function.
> That is a compatibility break, because in previous versions this
> worked:
>
> regression=# create function foo(int, out int) language sql
> as 'select $1';
> CREATE FUNCTION
> regression=# drop routine foo(int, out int);
> DROP ROUTINE
>
> The fact that you now have to be aware of these details to use
> ALTER/DROP ROUTINE seems like a pretty serious loss of user
> friendliness, as well as compatibility.

I'm also concerned about the behavior here. I noticed it when this
commit went in, and it seemed concerning to me then, and it still
does. Nevertheless, I'm not convinced that your proposal is an
improvement. Suppose we have foo(int, out int) and also foo(int).
Then, if I understand correctly, under your proposal, foo(4) will call
the former within plpgsql code, because in that context the OUT
parameters must be included, and the latter from SQL code, because in
that context they must be emitted. I suspect in practice what will
happen is that you'll end up with both interpretations even within the
body of a plpgsql function, because plpgsql functions tend to include
SQL queries where, I presume, the SQL interpretation must apply. It
seems that it will be very difficult for users to know which set of
rules apply in which contexts.

Now, that being said, the status quo is also pretty bad, because we
have one set of rules for functions and another for procedures. I
believe that users will expect those to behave in similar ways, and
will be sad and surprised when they don't.

But on the third hand, Peter is also correct when he says that there's
not much use in implementing standard features with non-standard
semantics. The fact that we've chosen to make OUT parameters do some
random thing that is not what other systems do is, indeed, not great
for migrations. So doubling down on that questionable choice is also
not great. In a green field I think we ought to go the other way and
make OUT parameters as consistent with the standard as we can, and
have that handling be the same for procedures and for functions, but
it seems impossible to imagine making such a large compatibility break
with our own previous releases, however much the spec may dictate it.

I don't see any really great choice here, but in some sense your
proposal seems like the worst of all the options. It does not reverse
the patch's choice to treat functions and procedures differently, so
users will still have to deal with that inconsistency. But in addition
the handling of procedures will itself be inconsistent based on
context.

--
Robert Haas
EDB: http://www.enterprisedb.com