Thread: Re: Review handling of MOVE and FETCH (ToDo)

(resent to -hackers)

I just applied and tested the new patch. Everything works great.

The only thing I would change now is some of the comments.

1). On line 289, one of the regression test comments got copied:

+   move forward in c;                --should be at '5'

change to:

+   move forward in c;                --should be at '1'

2). Lines 79/80:

+ errmsg("statement FETCH returns more rows."),
+ errhint("Multirows fetch are not allowed in PL/pgSQL.")));

This might sound better as "statement FETCH returns multiple rows.",
and "Multirow FETCH is not allowed in PL/pgSQL."

Everything else looks good to me.
John


> Hi Selena and John,
>
> Pavel's latest patch seems to address all the issues you raised in
> your initial review.  Do you have any comments on this new revision?
> If you're happy that your issues have been resolved, please mark the
> patch as Ready for Committer.
>
> Cheers,
> BJ
>

On Mon, 2009-09-28 at 10:44 -0700, John Naylor wrote:
> +
>   errmsg("statement FETCH returns more rows."),
> +
>   errhint("Multirows fetch are not allowed in PL/pgSQL.")));
> 
> This might sound better as "statement FETCH returns multiple rows.",

errmsg should be without period.

> and "Multirow FETCH is not allowed in PL/pgSQL."

That might better be errdetail.

Peter Eisentraut <peter_e@gmx.net> writes:
> On Mon, 2009-09-28 at 10:44 -0700, John Naylor wrote:
>> + errmsg("statement FETCH returns more rows."),
>> + errhint("Multirows fetch are not allowed in PL/pgSQL.")));
>> 
>> This might sound better as "statement FETCH returns multiple rows.",

> errmsg should be without period.

>> and "Multirow FETCH is not allowed in PL/pgSQL."

> That might better be errdetail.

It got committed like this:
           ereport(ERROR,                   (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),                    errmsg("FETCH
statementcannot return multiple rows")));
 

I didn't think the HINT was adding anything useful ...
        regards, tom lane