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

pá 2. 11. 2018 v 9:02 odesílatel Peter Eisentraut <
peter(dot)eisentraut(at)2ndquadrant(dot)com> napsal:

> On 02/11/2018 06:04, Pavel Stehule wrote:
> > čt 1. 11. 2018 v 14:31 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com
> > <mailto:pavel(dot)stehule(at)gmail(dot)com>> napsal:
> >
> > Cleaned patch with regress tests
> >
> >
> > minor cleaning
>
> Could you explain your analysis of the problem and how this patch
> proposes to fix it?
>

The original code works with original function arguments - and this compare
with argmodes fields. But when you use named args, this expectation is not
valid.

So cycle

foreach(lc, args)
{
if (armodes[i] == PROARGMODE_INOUT)
{
}
}

is just wrong

There are another issue

row->varnos[nexpr->argnumber] = param->paramid - 1

It doesn't work if not all named args are INOUT

In case mentioned in bug report this row generated a operation

row->varnos[1] = param->paramid - 1; // in this case a argnumber was 1

but varnos[0] was unitialized .. mostly there was 0 what is number of FOUND
variable - and then result and error message about wrong boolean value.

So I reorder input arguments to correct order (against a info from
argmodes), and that is all.

Maybe it can be better to work with translated arg list instead original
list (then reordering is not necessary), but I am not sure if this
information is available from plan, and reordering in this case is O(N)
operation, where N is low, so it should not to have a impact on
performance.

Regards

Pavel

> About the patch, I suspect printing out
>
> NOTICE: <unnamed portal 2>
>
> in the regression tests might lead to unstable results if there is
> concurrent activity
>

true, this test can be better based. There can be any string.

>
> --
> Peter Eisentraut http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>