pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
To: Yeb Havinga <yebhavinga@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
Cc: Kevin Grittner <Kevin.Grittner@wicourts.gov>
Cc: pgsql-hackers@postgresql.org
Subject: Re: [REVIEW] Patch for cursor calling with named parameters
Date: Wed, 14 Dec 2011 15:58:39 +0200
Message-ID: <4EE8AB8F.8090800@enterprisedb.com> (raw)
In-Reply-To: <4EE87B1B.7070609@gmail.com>
References: <4EE607B90200002500043C0F@gw.wicourts.gov>
	<4EE77BB5.5040906@enterprisedb.com>
	<627.1323797659@sss.pgh.pa.us>
	<4EE87B1B.7070609@gmail.com>

On 14.12.2011 12:31, Yeb Havinga wrote:
> On 2011-12-13 18:34, Tom Lane wrote:
>> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com> writes:
>>> Attached is a patch with those changes. I also I removed a few of the
>>> syntax error regression tests, that seemed excessive, plus some general
>>> naming and comment fiddling. I'll apply this tomorrow, if it still looks
>>> good to me after sleeping on it.
>> However, I'm still concerned about whether this approach gives
>> reasonable error messages in cases where the error would be
>> detected during parse analysis of the rearranged statement.
>> The regression test examples don't cover such cases, and I'm
>> too busy right now to apply the patch and check for myself.
>> What happens for example if a named parameter's value contains
>> a misspelled variable reference, or a type conflict?
>
> I tested this and seems to be ok:
>
> regression=# select namedparmcursor_test1(20000, 20000) as "Should be
> false",
> namedparmcursor_test1(20, 20) as "Should be true";
> ERROR: column "yy" does not exist
> LINE 1: SELECT x AS param1, yy AS param12;

For the record, the caret pointing to the position is there, too. As in:

regression=# do $$
declare
   c1 cursor (param1 int, param2 int) for select 123;
begin
   open c1(param2 := xxx, param1 := 123);
end;
$$;
ERROR:  column "xxx" does not exist
LINE 1: SELECT 123 AS param1, xxx AS param2;
                               ^
QUERY:  SELECT 123 AS param1, xxx AS param2;
CONTEXT:  PL/pgSQL function "inline_code_block" line 5 at OPEN

I think that's good enough. It would be even better if we could print 
the original OPEN statement as the context, as in:

ERROR:  column "xxx" does not exist
LINE 4: OPEN c1(param2 := xxx, param1 := 123);
                           ^

but it didn't look quite like that before the patch either, and isn't 
specific to this patch but more of a general usability issue in PL/pgSQL.

Committed.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com



view thread (37+ messages)

Message-ID: <4EE8AB8F.8090800@enterprisedb.com>
Permalink:  ../4EE8AB8F.8090800@enterprisedb.com/
Also on:    postgresql.org/message-id/4EE8AB8F.8090800@enterprisedb.com

 · 

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: heikki.linnakangas@enterprisedb.com, yebhavinga@gmail.com, tgl@sss.pgh.pa.us, Kevin.Grittner@wicourts.gov
  Subject: Re: [REVIEW] Patch for cursor calling with named parameters
  In-Reply-To: <4EE8AB8F.8090800@enterprisedb.com>

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

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