Re: [REVIEW] Patch for cursor calling with named parameters

From: Yeb Havinga <yebhavinga(at)gmail(dot)com>
To: Royce Ausburn <royce(dot)ml(at)inomial(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [REVIEW] Patch for cursor calling with named parameters
Date: 2011-10-07 10:21:27
Message-ID: 4E8ED2A7.2040800@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2011-10-07 10:31:30 Re: Why does WAL_DEBUG macro need to be defined by default?
Previous Message Fujii Masao 2011-10-07 09:19:37 Why does WAL_DEBUG macro need to be defined by default?