agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
From: Yeb Havinga <yebhavinga@gmail.com>
To: Royce Ausburn <royce.ml@inomial.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: [REVIEW] Patch for cursor calling with named parameters
Date: Fri, 07 Oct 2011 12:21:27 +0200
Message-ID: <4E8ED2A7.2040800@gmail.com> (raw)
In-Reply-To: <FCEADDEF-8563-4965-8D4A-E37F2328C42B@inomial.com>
References: <FCEADDEF-8563-4965-8D4A-E37F2328C42B@inomial.com>

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




view thread (38+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: yebhavinga@gmail.com, royce.ml@inomial.com
  Subject: Re: [REVIEW] Patch for cursor calling with named parameters
  In-Reply-To: <4E8ED2A7.2040800@gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox