On Tue, May 25, 2021 at 2:20 PM Tom Lane <t...@sss.pgh.pa.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
