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
Show quoted text
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 statement cannot return multiple rows")));
I didn't think the HINT was adding anything useful ...
regards, tom lane