2009/9/28 John Naylor <[email protected]>:
> Pavel,
>
> It looks good. My last email didn't go to -hackers, since I wasn't
> subscribed. I had to resend to -hackers so there will be a link for
> the commitfest page. I think you might have to resend your latest
> patch to the list. Sorry!
nothing, patch attached
Pavel
>
> In any case, I will say it's ready for commiter.
>
> Thanks,
> John
>
> On Mon, Sep 28, 2009 at 2:07 AM, Pavel Stehule <[email protected]>
> wrote:
>> Hello
>>
>> I am sending actualised patch as per John comment.
>>
>> regards
>> Pavel Stehule
>>
>> 2009/9/26 John Naylor <[email protected]>:
>>> Hi,
>>>
>>> Sorry, I didn't notice the attachment on Pavel's email, otherwise I
>>> would have done this sooner! :)
>>>
>>> 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
>>>>
>>>
>>
>
*** ./doc/src/sgml/plpgsql.sgml.orig 2009-09-28 09:38:55.711469112 +0200
--- ./doc/src/sgml/plpgsql.sgml 2009-09-28 09:39:24.613468923 +0200
***************
*** 2656,2670 ****
<para>
The options for the <replaceable>direction</replaceable> clause are
! the same as for <command>FETCH</>, namely
<literal>NEXT</>,
<literal>PRIOR</>,
<literal>FIRST</>,
<literal>LAST</>,
<literal>ABSOLUTE</> <replaceable>count</replaceable>,
<literal>RELATIVE</> <replaceable>count</replaceable>,
! <literal>FORWARD</>, or
! <literal>BACKWARD</>.
Omitting <replaceable>direction</replaceable> is the same
as specifying <literal>NEXT</>.
<replaceable>direction</replaceable> values that require moving
--- 2656,2670 ----
<para>
The options for the <replaceable>direction</replaceable> clause are
! similar to <command>FETCH</>, namely
<literal>NEXT</>,
<literal>PRIOR</>,
<literal>FIRST</>,
<literal>LAST</>,
<literal>ABSOLUTE</> <replaceable>count</replaceable>,
<literal>RELATIVE</> <replaceable>count</replaceable>,
! <literal>FORWARD</> <optional> <replaceable>count</replaceable> | <literal>ALL</> </optional>, or
! <literal>BACKWARD</> <optional> <replaceable>count</replaceable> | <literal>ALL</> </optional>.
Omitting <replaceable>direction</replaceable> is the same
as specifying <literal>NEXT</>.
<replaceable>direction</replaceable> values that require moving
***************
*** 2678,2683 ****
--- 2678,2684 ----
MOVE curs1;
MOVE LAST FROM curs3;
MOVE RELATIVE -2 FROM curs4;
+ MOVE FORWARD 2 FROM curs4;
</programlisting>
</para>
</sect3>
*** ./src/pl/plpgsql/src/gram.y.orig 2009-09-28 09:38:55.713470217 +0200
--- ./src/pl/plpgsql/src/gram.y 2009-09-28 11:00:53.811467762 +0200
***************
*** 72,77 ****
--- 72,79 ----
int until, const char *expected);
static List *read_raise_options(void);
+ static PLpgSQL_stmt_fetch *complete_direction(PLpgSQL_stmt_fetch *fetch, bool *check_FROM);
+
%}
%expect 0
***************
*** 178,183 ****
--- 180,186 ----
* Keyword tokens
*/
%token K_ALIAS
+ %token K_ALL
%token K_ASSIGN
%token K_BEGIN
%token K_BY
***************
*** 1621,1626 ****
--- 1624,1635 ----
if (yylex() != ';')
yyerror("syntax error");
+
+ if (fetch->returns_multiple_rows)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("statement FETCH returns multiple rows"),
+ errhint("Multirow FETCH is not allowed in PL/pgSQL.")));
fetch->lineno = $2;
fetch->rec = rec;
***************
*** 2252,2257 ****
--- 2261,2271 ----
}
+ /*
+ * Read FETCH or MOVE statement direction. By default,
+ * cursor will only move one row. To MOVE more than one row
+ * at a time see complete_direction().
+ */
static PLpgSQL_stmt_fetch *
read_fetch_direction(void)
{
***************
*** 2269,2274 ****
--- 2283,2289 ----
fetch->direction = FETCH_FORWARD;
fetch->how_many = 1;
fetch->expr = NULL;
+ fetch->returns_multiple_rows = false;
/*
* Most of the direction keywords are not plpgsql keywords, so we
***************
*** 2313,2323 ****
}
else if (pg_strcasecmp(yytext, "forward") == 0)
{
! /* use defaults */
}
else if (pg_strcasecmp(yytext, "backward") == 0)
{
fetch->direction = FETCH_BACKWARD;
}
else if (tok != T_SCALAR)
{
--- 2328,2339 ----
}
else if (pg_strcasecmp(yytext, "forward") == 0)
{
! fetch = complete_direction(fetch, &check_FROM);
}
else if (pg_strcasecmp(yytext, "backward") == 0)
{
fetch->direction = FETCH_BACKWARD;
+ fetch = complete_direction(fetch, &check_FROM);
}
else if (tok != T_SCALAR)
{
***************
*** 2346,2351 ****
--- 2362,2415 ----
}
+ /*
+ * Allows directions:
+ * FORWARD expr, FORWARD ALL, FORWARD
+ * BACKWARD expr, BACKWARD ALL, BACKWARD
+ *
+ * so plpgsql should fully support PostgreSQL's MOVE statement.
+ */
+ static PLpgSQL_stmt_fetch *
+ complete_direction(PLpgSQL_stmt_fetch *fetch, bool *check_FROM)
+ {
+ int tok;
+
+ tok = yylex();
+ if (tok == K_FROM || tok == K_IN)
+ {
+ *check_FROM = false;
+
+ return fetch;
+ }
+
+ if (tok == K_ALL)
+ {
+ fetch->how_many = fetch->direction == FETCH_FORWARD ? -1 : 0;
+ fetch->direction = FETCH_ABSOLUTE;
+ fetch->returns_multiple_rows = true;
+ *check_FROM = true;
+
+ return fetch;
+ }
+
+ plpgsql_push_back_token(tok);
+ fetch->expr = read_sql_expression2(K_FROM, K_IN,
+ "FROM or IN",
+ NULL);
+
+ /*
+ * We don't allow multiple rows in PL/pgSQL's FETCH statement. This
+ * limit is enforced by syntax. Only for sure one row directions
+ * are allowed. Because result of expr should be greather than
+ * one, we expect possible multiple rows and disable this direction
+ * when current statement is FETCH statement.
+ */
+ fetch->returns_multiple_rows = true;
+ *check_FROM = false;
+
+ return fetch;
+ }
+
static PLpgSQL_stmt *
make_return_stmt(int lineno)
{
*** ./src/pl/plpgsql/src/plpgsql.h.orig 2009-09-28 09:38:55.715469366 +0200
--- ./src/pl/plpgsql/src/plpgsql.h 2009-09-28 09:39:24.653469785 +0200
***************
*** 520,525 ****
--- 520,526 ----
int how_many; /* count, if constant (expr is NULL) */
PLpgSQL_expr *expr; /* count, if expression */
bool is_move; /* is this a fetch or move? */
+ bool returns_multiple_rows; /* returns more than one rows? */
} PLpgSQL_stmt_fetch;
*** ./src/test/regress/expected/plpgsql.out.orig 2009-09-28 09:38:55.719468503 +0200
--- ./src/test/regress/expected/plpgsql.out 2009-09-28 09:39:24.684468331 +0200
***************
*** 3027,3032 ****
--- 3027,3054 ----
create or replace function sc_test() returns setof integer as $$
declare
+ c refcursor;
+ x integer;
+ begin
+ open c scroll for execute 'select f1 from int4_tbl';
+ fetch last from c into x;
+ while found loop
+ return next x;
+ move backward 2 from c;
+ fetch relative -1 from c into x;
+ end loop;
+ close c;
+ end;
+ $$ language plpgsql;
+ select * from sc_test();
+ sc_test
+ -------------
+ -2147483647
+ 123456
+ (2 rows)
+
+ create or replace function sc_test() returns setof integer as $$
+ declare
c cursor for select * from generate_series(1, 10);
x integer;
begin
***************
*** 3052,3057 ****
--- 3074,3149 ----
9
(3 rows)
+ create or replace function sc_test() returns setof integer as $$
+ declare
+ c cursor for select * from generate_series(1, 10);
+ x integer;
+ begin
+ open c;
+ loop
+ move forward 2 in c;
+ if not found then
+ exit;
+ end if;
+ fetch next from c into x;
+ if found then
+ return next x;
+ end if;
+ end loop;
+ close c;
+ end;
+ $$ language plpgsql;
+ select * from sc_test();
+ sc_test
+ ---------
+ 3
+ 6
+ 9
+ (3 rows)
+
+ drop function sc_test();
+ create or replace function sc_test() returns integer
+ as $$
+ declare
+ c cursor for select * from generate_series(1, 10);
+ x integer;
+ begin
+ open c;
+ move forward in c; --should be at '5'
+ fetch relative 0 from c into x; --fetch current position
+ if found then
+ return x;
+ end if;
+ close c;
+ end;
+ $$ language plpgsql;
+ select sc_test();
+ sc_test
+ ---------
+ 1
+ (1 row)
+
+ create or replace function sc_test() returns integer
+ as $$
+ declare
+ c cursor for select * from generate_series(1, 10);
+ x integer;
+ begin
+ open c;
+ move forward 5 in c; --should be at '5'
+ fetch relative 0 from c into x; --fetch current position
+ if found then
+ return x;
+ end if;
+ close c;
+ end;
+ $$ language plpgsql;
+ select sc_test();
+ sc_test
+ ---------
+ 5
+ (1 row)
+
drop function sc_test();
-- test qualified variable names
create function pl_qual_names (param1 int) returns void as $$
*** ./src/test/regress/sql/plpgsql.sql.orig 2009-09-28 09:38:55.721469049 +0200
--- ./src/test/regress/sql/plpgsql.sql 2009-09-28 10:58:39.991468013 +0200
***************
*** 2513,2518 ****
--- 2513,2536 ----
create or replace function sc_test() returns setof integer as $$
declare
+ c refcursor;
+ x integer;
+ begin
+ open c scroll for execute 'select f1 from int4_tbl';
+ fetch last from c into x;
+ while found loop
+ return next x;
+ move backward 2 from c;
+ fetch relative -1 from c into x;
+ end loop;
+ close c;
+ end;
+ $$ language plpgsql;
+
+ select * from sc_test();
+
+ create or replace function sc_test() returns setof integer as $$
+ declare
c cursor for select * from generate_series(1, 10);
x integer;
begin
***************
*** 2533,2540 ****
--- 2551,2619 ----
select * from sc_test();
+ create or replace function sc_test() returns setof integer as $$
+ declare
+ c cursor for select * from generate_series(1, 10);
+ x integer;
+ begin
+ open c;
+ loop
+ move forward 2 in c;
+ if not found then
+ exit;
+ end if;
+ fetch next from c into x;
+ if found then
+ return next x;
+ end if;
+ end loop;
+ close c;
+ end;
+ $$ language plpgsql;
+
+ select * from sc_test();
+
+ drop function sc_test();
+
+ create or replace function sc_test() returns integer
+ as $$
+ declare
+ c cursor for select * from generate_series(1, 10);
+ x integer;
+ begin
+ open c;
+ move forward in c; --should be at '1'
+ fetch relative 0 from c into x; --fetch current position
+ if found then
+ return x;
+ end if;
+ close c;
+ end;
+ $$ language plpgsql;
+
+ select sc_test();
+
+ create or replace function sc_test() returns integer
+ as $$
+ declare
+ c cursor for select * from generate_series(1, 10);
+ x integer;
+ begin
+ open c;
+ move forward 5 in c; --should be at '5'
+ fetch relative 0 from c into x; --fetch current position
+ if found then
+ return x;
+ end if;
+ close c;
+ end;
+ $$ language plpgsql;
+
+ select sc_test();
+
drop function sc_test();
+
-- test qualified variable names
create function pl_qual_names (param1 int) returns void as $$
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
