Re: CALL versus procedures with output-only arguments

Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
> On 02.06.21 02:04, Tom Lane wrote:
>>> It's possible that we could revert proargtypes and still accommodate
>>> the spec's definition for ALTER/DROP ROUTINE/PROCEDURE. I'm imagining
>>> some rules along the line of:
>>> 1. If arg list contains any parameter modes, then it must be PG
>>> syntax, so interpret it according to our traditional rules.
>>> 2. Otherwise, try to match the given arg types against *both*
>>> proargtypes and proallargtypes. If we get multiple matches,
>>> complain that the command is ambiguous. (In the case of DROP
>>> PROCEDURE, it's probably OK to consider only proallargtypes.)

>> Hmm, actually we could make step 2 a shade tighter: if a candidate
>> routine is a function, match against proargtypes. If it's a procedure,
>> match against coalesce(proallargtypes, proargtypes). If we find
>> multiple matches, raise ambiguity error.

> I'm ok with this proposal.

I spent some time playing with this, and ran into a problem.
Given the example we discussed upthread:

d1=# create procedure p1(int, int) language sql as 'select 1';
CREATE PROCEDURE
d1=# create procedure p1(out int, out int) language sql as 'select 1,2';
CREATE PROCEDURE

you can uniquely refer to the first p1 by writing (IN int, IN int),
and you can uniquely refer to the second p1 by writing an empty parameter
list or by writing (OUT int, OUT int). If you write just (int, int),
you get an ambiguity error as discussed.

The problem is that we have a lot of existing code that expects
p1(int, int) to work for the first p1. Notably, this scenario breaks
"pg_dump --clean", which emits commands like

DROP PROCEDURE public.p1(integer, integer);
DROP PROCEDURE public.p1(OUT integer, OUT integer);

It would likely not be very hard to fix pg_dump to include explicit
IN markers. I don't think this results in a compatibility problem
for existing dumps, since they won't be taken from databases in
which there are procedures with OUT arguments.

I'm concerned however about what other client code might get side-swiped.
We (or users) would not be likely to hit the ambiguity right away,
so that sort of issue could go unnoticed for a long time.

So I'm unsure right now whether this is going to be an acceptable
change. I feel like it's still a better situation than what we
have in HEAD, but it's not as cost-free as I'd hoped.

regards, tom lane