From: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
---|---|
To: | Yeb Havinga <yebhavinga(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [REVIEW] Patch for cursor calling with named parameters |
Date: | 2011-12-14 13:58:39 |
Message-ID: | 4EE8AB8F.8090800@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 14.12.2011 12:31, Yeb Havinga wrote:
> On 2011-12-13 18:34, Tom Lane wrote:
>> Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2011-12-14 13:59:28 | Re: [PATCH] PostgreSQL fails to build with 32bit MinGW-w64 |
Previous Message | Pavan Deolasee | 2011-12-14 13:11:30 | Re: Race condition in HEAD, possibly due to PGPROC splitup |