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(at)gmx(dot)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 statement cannot return multiple rows")));

I didn't think the HINT was adding anything useful ...

regards, tom lane