From: Tom Lane <tgl@sss.pgh.pa.us>
To: Pavel Stehule <pavel.stehule@gmail.com>
cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
        PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: bugfix: BUG #15477: Procedure call with named inout refcursor
 parameter - "invalid input syntax for type boolean"
In-reply-to: 
 <CAFj8pRDUL=nC62L-AKKFDEo1en1ZT1otLfPdXkCjm8MHxif8PA@mail.gmail.com>
References: 
 <CAFj8pRA6UsujpTs9Sdwmk-R6yQykPx46wgjj+YZ7zxm4onrDyw@mail.gmail.com>
 <CAFj8pRBdNqq8+SsChS8F5s9ttkdBSTPu=cq-fLx51FF7Tk_LkA@mail.gmail.com>
 <CAFj8pRCF3pia7gScM0jh7vY9rEuqN7fbnUV+jymRTLEXZeRDNA@mail.gmail.com>
 <CAFj8pRD2=MGU8ZshvmCjv5KG+3EngRm0+8pFLsjhG5bUhUGU2Q@mail.gmail.com>
 <CAFj8pRCehpRXn2vSq8Q+dQL2sQ0XXt0rOukFb0tZ5F7RFdH+DA@mail.gmail.com>
 <1d846492-3822-616b-7246-91b429cc5dbb@2ndquadrant.com>
 <CAFj8pRB3ZPu_SUDamXGA+wXw2aMmn0oLmgvsdMT1oAyJUMgFZA@mail.gmail.com>
 <26116.1541278051@sss.pgh.pa.us> <28551.1541281624@sss.pgh.pa.us>
 <CAFj8pRDUL=nC62L-AKKFDEo1en1ZT1otLfPdXkCjm8MHxif8PA@mail.gmail.com>
Comments: In-reply-to Pavel Stehule <pavel.stehule@gmail.com>
	message dated "Sun, 04 Nov 2018 04:44:46 +0100"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <7101.1541346845.1@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit
Date: Sun, 04 Nov 2018 10:54:05 -0500
Message-ID: <7102.1541346845@sss.pgh.pa.us>
Precedence: bulk

Pavel Stehule <pavel.stehule@gmail.com> writes:
> so 3. 11. 2018 v 22:47 odesílatel Tom Lane <tgl@sss.pgh.pa.us> napsal:
>> So while looking at that ... isn't the behavior for non-writable output
>> parameters basically insane?  It certainly fails to accord with the
>> plpgsql documentation, which shows an example that would throw an error:
>> 
>> CREATE PROCEDURE triple(INOUT x int)
>> ...
>> CALL triple(5);
>> 
>> It's even weirder that you can get away with not supplying a writable
>> target value for an output argument so long as it has a default.
>> 
>> I think the behavior here ought to be "if the actual argument is a plpgsql
>> variable, assign the output back to it, otherwise do nothing".  That's
>> much closer to the behavior of OUT arguments in other old-school
>> programming languages.

> I don't agree. The constant can be used only for IN parameter. Safe
> languages like Ada does copy result to variable used as INOUT parameter.
> PL/SQL doesn't allow it too.

Well, my main point is that that ISN'T what our current code does, nor
does your patch.  The reason I'm complaining is that after I rewrote the
code to use expand_function_arguments, it started rejecting this existing
regression test case:

CREATE PROCEDURE test_proc8c(INOUT a int, INOUT b int, INOUT c int DEFAULT 11)
...

  CALL test_proc8c(_a, _b);

I do not think you can argue that that's not broken while simultaneously
claiming that this error check promotes safe coding.

I looked into SQL:2011 to see what it has to say about this.  In
10.4 <routine invocation>, syntax rule 9) g) iii) says

    For each SQL parameter Pi, 1 (one) ≤ i ≤ SRNP, that is an output SQL
    parameter or both an input SQL parameter and an output SQL parameter,
    XAi shall be a <target specification>.

The immediately preceding rules make it clear that XAi is the actual
argument corresponding to parameter Pi *after* default-insertion and
named-argument reordering.  So our existing behavior here clearly
contradicts the spec: DEFAULT is not a <target specification>.

I couldn't find any really clear statement of what PL/SQL does
in this situation, but the docs I did find say that the actual
argument for an OUT parameter has to be a variable, with no
exceptions mentioned.

In short, I think it's a bug that we allow the above.  If you
want to keep the must-be-a-variable error then it should apply in
this case too.

			regards, tom lane