On 02/11/2018 06:04, Pavel Stehule wrote:
> čt 1. 11. 2018 v 14:31 odesílatel Pavel Stehule <pavel.stehule@gmail.com
> <mailto:pavel.stehule@gmail.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