Re: bugfix: BUG #15477: Procedure call with named inout refcursor parameter - "invalid input syntax for type boolean"

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: bugfix: BUG #15477: Procedure call with named inout refcursor parameter - "invalid input syntax for type boolean"
Date: 2018-11-04 16:01:54
Message-ID: CAFj8pRAcdrRogAC2DU9in=gT1h3y1AgpJnAnDmzho46NS7fJ-w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

ne 4. 11. 2018 v 16:54 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> napsal:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > so 3. 11. 2018 v 22:47 odesílatel Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.
>

Hard to say what is correct in this case. When we use CALL statement
directly from top level, then it is clean. But the semantic from PLpgSQL is
difficult.

Maybe we can disallow this case when procedure is called from PL/pgSQL.

>
> 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.
>

I agree. This should be prohibited from PLpgSQL.

Regards

Pavel

>
> regards, tom lane
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-11-04 16:14:52 Re: bugfix: BUG #15477: Procedure call with named inout refcursor parameter - "invalid input syntax for type boolean"
Previous Message Tom Lane 2018-11-04 15:54:05 Re: bugfix: BUG #15477: Procedure call with named inout refcursor parameter - "invalid input syntax for type boolean"