Re: [REVIEW] Patch for cursor calling with named parameters - Mailing list pgsql-hackers

From Yeb Havinga
Subject Re: [REVIEW] Patch for cursor calling with named parameters
Date
Msg-id 4E8ED2A7.2040800@gmail.com
Whole thread Raw
In response to [REVIEW] Patch for cursor calling with named parameters  (Royce Ausburn <royce.ml@inomial.com>)
Responses Re: [REVIEW] Patch for cursor calling with named parameters
List pgsql-hackers
On 2011-10-06 16:04, Royce Ausburn wrote:
> Initial Review for patch:
>
> http://archives.postgresql.org/pgsql-hackers/2011-09/msg00744.php

Hello Royce,

Thank you for your review.

>
> I don't think so.  The new feature accepts opening a cursor with some 
> parameter names not specified:
>
>       open cur1(param3 := 4, 1, param1 := 5);
>
> It seems that if a parameter is not named, its position is used to 
> bind to a variable.  For example, the following fail:
>
> psql:plsqltest.sql:26: ERROR:  cursor "cur1" argument 2 "param2" 
> provided multiple times
> LINE 10:       open cur1(param3 := 4, 1, param2 := 5);
>
> and
>
> psql:plsqltest.sql:26: ERROR:  cursor "cur1" argument 2 "param2" 
> provided multiple times
> LINE 10:       open cur1(param2 := 4, 2, param1 := 5);
>
>
> I think that postgres ought to enforce some consistency here.  Use one 
> way or the other, never both.

This was meant as a feature, but I can remove it.
>
>
> I can also produce some unhelpful errors when I give bad syntax.  For 
> example:
>
> psql:plsqltest.sql:28: ERROR:  cursor "cur1" argument 1 "param1" 
> provided multiple times
> LINE 11:       open cur1( param3 : = 4, 2, param1 := 5);
> (notice the space between the : and =)

Yes, the whole of the expression before the first comma, 'param3 : = 4' 
is not recognized as <parametername> <:= symbol> <expression>, so that 
is taken as the value of the first parameter. This value is parsed after 
all named arguments are read, and hence no meaningful error is given. If 
there was no param1 parameter name at the end, the 'multiple times' 
error would not have caused the processing to stop, and a syntax error 
at the correct : would have been given.

The same reasoning also explains the other 'multiple times' errors you 
could get, by putting a syntax error in some value.
>
> --
>
>       open cur1( param3 := param3 , param2 = 3, param1 := 1 );
>
> psql:plsqltest.sql:29: ERROR:  column "param2" does not exist
> LINE 2: ,param2 = 3
>          ^
> QUERY:  SELECT 1
> ,param2 = 3
> ,param3;
> CONTEXT:  PL/pgSQL function "named_para_test" line 7 at OPEN

This is a valid error, since the parser / SQL will try to evaluate the 
boolean expression param2 = 3, while param2 is not a defined variabele.

Again, thank you very much for your thorough review. I'll update the 
patch so mixing positional and named parameters are removed, add 
documentation, and give syntax errors before an error message indicating 
that positional and named parameters were mixed.

-- 
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data



pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Why does WAL_DEBUG macro need to be defined by default?
Next
From: Heikki Linnakangas
Date:
Subject: Re: Why does WAL_DEBUG macro need to be defined by default?