Обсуждение: BUG #4516: FOUND variable does not work after RETURN QUERY
The following bug has been logged online:
Bug reference: 4516
Logged by: Michal szymanski
Email address: szymanskim@datera.pl
PostgreSQL version: 8.3
Operating system: Windows
Description: FOUND variable does not work after RETURN QUERY
Details:
This short program display two rows instead one. If I use RETURN NEXT it
works.
CREATE TABLE test_table (
value VARCHAR
);
INSERT INTO test_table VALUES ('a');
INSERT INTO test_table VALUES ('b');
CREATE OR REPLACE FUNCTION test()
RETURNS SETOF test_table AS
$BODY$
DECLARE
BEGIN
RETURN QUERY
SELECT * FROM test_table WHERE value='a';
IF NOT FOUND THEN
RETURN QUERY
SELECT * FROM test_table WHERE value='b';
END IF;
RETURN;
END;
$BODY$
LANGUAGE 'plpgsql' VOLATILE;
select * from test()
"Michal szymanski" <szymanskim@datera.pl> writes: > Description: FOUND variable does not work after RETURN QUERY The list of statements that set FOUND is here: http://www.postgresql.org/docs/8.3/static/plpgsql-statements.html#PLPGSQL-STATEMENTS-DIAGNOSTICS RETURN isn't one of them. regards, tom lane
Hello 2008/11/6 Tom Lane <tgl@sss.pgh.pa.us>: > "Michal szymanski" <szymanskim@datera.pl> writes: >> Description: FOUND variable does not work after RETURN QUERY > > The list of statements that set FOUND is here: > http://www.postgresql.org/docs/8.3/static/plpgsql-statements.html#PLPGSQL-STATEMENTS-DIAGNOSTICS > > RETURN isn't one of them. > > regards, tom lane > It should be enhanced - my initial proposal of return query expected so return query is last statement, that isn't now. So we could add this check there. regards Pavel Stehule > -- > Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-bugs >
"Pavel Stehule" <pavel.stehule@gmail.com> writes:
> 2008/11/6 Tom Lane <tgl@sss.pgh.pa.us>:
>> RETURN isn't one of them.
> It should be enhanced - my initial proposal of return query expected
> so return query is last statement, that isn't now. So we could add
> this check there.
Well, changing the semantics of an already-released statement carries a
risk of breaking existing apps that aren't expecting it to change FOUND.
So I'd want to see a pretty strong case why this is important --- not
just that it didn't meet someone's didn't-read-the-manual expectation.
regards, tom lane
2008/11/6 Tom Lane <tgl@sss.pgh.pa.us>: > "Pavel Stehule" <pavel.stehule@gmail.com> writes: >> 2008/11/6 Tom Lane <tgl@sss.pgh.pa.us>: >>> RETURN isn't one of them. > >> It should be enhanced - my initial proposal of return query expected >> so return query is last statement, that isn't now. So we could add >> this check there. > > Well, changing the semantics of an already-released statement carries a > risk of breaking existing apps that aren't expecting it to change FOUND. > So I'd want to see a pretty strong case why this is important --- not > just that it didn't meet someone's didn't-read-the-manual expectation. > It's should do some problems, but I belive much less than change of casting or tsearch2 integration. And actually it's not ortogonal. Every not dynamic statement change FOUND variable. regards Pavel Stehule > regards, tom lane >
>>>>> "Pavel" == "Pavel Stehule" <pavel.stehule@gmail.com> writes:
>> Well, changing the semantics of an already-released statement
>> carries a risk of breaking existing apps that aren't expecting it
>> to change FOUND. So I'd want to see a pretty strong case why this
>> is important --- not just that it didn't meet someone's
>> didn't-read-the-manual expectation.
Pavel> It's should do some problems, but I belive much less than
Pavel> change of casting or tsearch2 integration. And actually it's
Pavel> not ortogonal. Every not dynamic statement change FOUND
Pavel> variable.
Regardless of what you think of FOUND, a more serious problem is this:
postgres=# create function test(n integer) returns setof integer language plpgsql
as $f$
declare
rc bigint;
begin
return query (select i from generate_series(1,n) i);
get diagnostics rc = row_count;
raise notice 'rc = %',rc;
end;
$f$;
CREATE FUNCTION
postgres=# select test(3);
NOTICE: rc = 0
test
------
1
2
3
(3 rows)
Since GET DIAGNOSTICS is documented as working for every SQL query
executed in the function, rather than for a specific list of
constructs, this is clearly a bug.
--
Andrew (irc:RhodiumToad)
I am sending patch, that adds FOUND and GET DIAGNOSTICS support for RETURN QUERY statement Regards Pavel Stehule 2008/11/10 Andrew Gierth <andrew@tao11.riddles.org.uk>: >>>>>> "Pavel" == "Pavel Stehule" <pavel.stehule@gmail.com> writes: > > >> Well, changing the semantics of an already-released statement > >> carries a risk of breaking existing apps that aren't expecting it > >> to change FOUND. So I'd want to see a pretty strong case why this > >> is important --- not just that it didn't meet someone's > >> didn't-read-the-manual expectation. > > Pavel> It's should do some problems, but I belive much less than > Pavel> change of casting or tsearch2 integration. And actually it's > Pavel> not ortogonal. Every not dynamic statement change FOUND > Pavel> variable. > > Regardless of what you think of FOUND, a more serious problem is this: > > postgres=# create function test(n integer) returns setof integer language plpgsql > as $f$ > declare > rc bigint; > begin > return query (select i from generate_series(1,n) i); > get diagnostics rc = row_count; > raise notice 'rc = %',rc; > end; > $f$; > CREATE FUNCTION > postgres=# select test(3); > NOTICE: rc = 0 > test > ------ > 1 > 2 > 3 > (3 rows) > > Since GET DIAGNOSTICS is documented as working for every SQL query > executed in the function, rather than for a specific list of > constructs, this is clearly a bug. > > -- > Andrew (irc:RhodiumToad) > > -- > Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-bugs >
Вложения
Uh, is this ready to be applied? --------------------------------------------------------------------------- Pavel Stehule wrote: > I am sending patch, that adds FOUND and GET DIAGNOSTICS support for > RETURN QUERY statement > > Regards > Pavel Stehule > > > > 2008/11/10 Andrew Gierth <andrew@tao11.riddles.org.uk>: > >>>>>> "Pavel" == "Pavel Stehule" <pavel.stehule@gmail.com> writes: > > > > >> Well, changing the semantics of an already-released statement > > >> carries a risk of breaking existing apps that aren't expecting it > > >> to change FOUND. So I'd want to see a pretty strong case why this > > >> is important --- not just that it didn't meet someone's > > >> didn't-read-the-manual expectation. > > > > Pavel> It's should do some problems, but I belive much less than > > Pavel> change of casting or tsearch2 integration. And actually it's > > Pavel> not ortogonal. Every not dynamic statement change FOUND > > Pavel> variable. > > > > Regardless of what you think of FOUND, a more serious problem is this: > > > > postgres=# create function test(n integer) returns setof integer language plpgsql > > as $f$ > > declare > > rc bigint; > > begin > > return query (select i from generate_series(1,n) i); > > get diagnostics rc = row_count; > > raise notice 'rc = %',rc; > > end; > > $f$; > > CREATE FUNCTION > > postgres=# select test(3); > > NOTICE: rc = 0 > > test > > ------ > > 1 > > 2 > > 3 > > (3 rows) > > > > Since GET DIAGNOSTICS is documented as working for every SQL query > > executed in the function, rather than for a specific list of > > constructs, this is clearly a bug. > > > > -- > > Andrew (irc:RhodiumToad) > > > > -- > > Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) > > To make changes to your subscription: > > http://www.postgresql.org/mailpref/pgsql-bugs > > [ Attachment, skipping... ] > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes:
> Uh, is this ready to be applied?
I don't think any consensus has been reached on changing this behavior.
regards, tom lane
Hello 2009/1/10 Tom Lane <tgl@sss.pgh.pa.us>: > Bruce Momjian <bruce@momjian.us> writes: >> Uh, is this ready to be applied? > > I don't think any consensus has been reached on changing this behavior. > > regards, tom lane > I thing, so this is bug - RETURN QUERY has to supply FOR SELECT LOOP RETURN NEXT pattern. My first patch expected so RETURN QUERY is final statement, so I don't solve FOUND variable, but Heikki changed this behave. Without correct FOUND behave we can't to use RETURN QUERY for following pattern RETURN QUERY some; IF FOUND THEN RETURN; END IF; RETURN QUERY some_other; RETURN; regards Pavel Stehule
> 2009/1/10 Tom Lane <tgl@sss.pgh.pa.us>: >> Bruce Momjian <bruce@momjian.us> writes: >>> Uh, is this ready to be applied? >> >> I don't think any consensus has been reached on changing this behavior. > > I thing, so this is bug - RETURN QUERY has to supply FOR SELECT LOOP > RETURN NEXT pattern. > > My first patch expected so RETURN QUERY is final statement, so I don't > solve FOUND variable, but Heikki changed this behave. > > Without correct FOUND behave we can't to use RETURN QUERY for following pattern > > RETURN QUERY some; > IF FOUND THEN RETURN; END IF; > RETURN QUERY some_other; > RETURN; +1. I can't imagine it's good for this to be randomly inconsistent. ...Robert
Re: [HACKERS] BUG #4516: FOUND variable does not work after RETURN QUERY
От
Heikki Linnakangas
Дата:
Pavel Stehule wrote: > My first patch expected so RETURN QUERY is final statement, so I don't > solve FOUND variable, but Heikki changed this behave. Me? I don't recall doing anything related to this. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
2009/1/10 Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>: > Pavel Stehule wrote: >> >> My first patch expected so RETURN QUERY is final statement, so I don't >> solve FOUND variable, but Heikki changed this behave. > > Me? I don't recall doing anything related to this. > I have to look to archive, maybe Peter. Pavel > -- > Heikki Linnakangas > EnterpriseDB http://www.enterprisedb.com >
Robert Haas wrote: > > 2009/1/10 Tom Lane <tgl@sss.pgh.pa.us>: > >> Bruce Momjian <bruce@momjian.us> writes: > >>> Uh, is this ready to be applied? > >> > >> I don't think any consensus has been reached on changing this behavior. > > > > I thing, so this is bug - RETURN QUERY has to supply FOR SELECT LOOP > > RETURN NEXT pattern. > > > > My first patch expected so RETURN QUERY is final statement, so I don't > > solve FOUND variable, but Heikki changed this behave. > > > > Without correct FOUND behave we can't to use RETURN QUERY for following pattern > > > > RETURN QUERY some; > > IF FOUND THEN RETURN; END IF; > > RETURN QUERY some_other; > > RETURN; > > +1. I can't imagine it's good for this to be randomly inconsistent. So should this be applied or just kept for 8.5? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Wed, Feb 4, 2009 at 1:32 PM, Bruce Momjian <bruce@momjian.us> wrote: > Robert Haas wrote: >> > 2009/1/10 Tom Lane <tgl@sss.pgh.pa.us>: >> >> Bruce Momjian <bruce@momjian.us> writes: >> >>> Uh, is this ready to be applied? >> >> >> >> I don't think any consensus has been reached on changing this behavior. >> > >> > I thing, so this is bug - RETURN QUERY has to supply FOR SELECT LOOP >> > RETURN NEXT pattern. >> > >> > My first patch expected so RETURN QUERY is final statement, so I don't >> > solve FOUND variable, but Heikki changed this behave. >> > >> > Without correct FOUND behave we can't to use RETURN QUERY for following pattern >> > >> > RETURN QUERY some; >> > IF FOUND THEN RETURN; END IF; >> > RETURN QUERY some_other; >> > RETURN; >> >> +1. I can't imagine it's good for this to be randomly inconsistent. > > So should this be applied or just kept for 8.5? Well, *I* think it should be applied. But YMMV. ...Robert
Pavel Stehule wrote: > I am sending patch, that adds FOUND and GET DIAGNOSTICS support for > RETURN QUERY statement Updated patch attached and applied. Thanks. --------------------------------------------------------------------------- > > Regards > Pavel Stehule > > > > 2008/11/10 Andrew Gierth <andrew@tao11.riddles.org.uk>: > >>>>>> "Pavel" == "Pavel Stehule" <pavel.stehule@gmail.com> writes: > > > > >> Well, changing the semantics of an already-released statement > > >> carries a risk of breaking existing apps that aren't expecting it > > >> to change FOUND. So I'd want to see a pretty strong case why this > > >> is important --- not just that it didn't meet someone's > > >> didn't-read-the-manual expectation. > > > > Pavel> It's should do some problems, but I belive much less than > > Pavel> change of casting or tsearch2 integration. And actually it's > > Pavel> not ortogonal. Every not dynamic statement change FOUND > > Pavel> variable. > > > > Regardless of what you think of FOUND, a more serious problem is this: > > > > postgres=# create function test(n integer) returns setof integer language plpgsql > > as $f$ > > declare > > rc bigint; > > begin > > return query (select i from generate_series(1,n) i); > > get diagnostics rc = row_count; > > raise notice 'rc = %',rc; > > end; > > $f$; > > CREATE FUNCTION > > postgres=# select test(3); > > NOTICE: rc = 0 > > test > > ------ > > 1 > > 2 > > 3 > > (3 rows) > > > > Since GET DIAGNOSTICS is documented as working for every SQL query > > executed in the function, rather than for a specific list of > > constructs, this is clearly a bug. > > > > -- > > Andrew (irc:RhodiumToad) > > > > -- > > Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) > > To make changes to your subscription: > > http://www.postgresql.org/mailpref/pgsql-bugs > > [ Attachment, skipping... ] > > -- > Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-bugs -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/plpgsql.sgml =================================================================== RCS file: /cvsroot/pgsql/doc/src/sgml/plpgsql.sgml,v retrieving revision 1.137 diff -c -c -r1.137 plpgsql.sgml *** doc/src/sgml/plpgsql.sgml 4 Feb 2009 21:30:41 -0000 1.137 --- doc/src/sgml/plpgsql.sgml 5 Feb 2009 15:15:03 -0000 *************** *** 1356,1361 **** --- 1356,1369 ---- execution of other statements within the loop body. </para> </listitem> + <listitem> + <para> + A <command>RETURN QUERY</command> and <command>RETURN QUERY + EXECUTE</command> statements set <literal>FOUND</literal> + true if the query returns at least one row, false if no row + is returned. + </para> + </listitem> </itemizedlist> <literal>FOUND</literal> is a local variable within each Index: src/pl/plpgsql/src/pl_exec.c =================================================================== RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v retrieving revision 1.231 diff -c -c -r1.231 pl_exec.c *** src/pl/plpgsql/src/pl_exec.c 21 Jan 2009 11:13:14 -0000 1.231 --- src/pl/plpgsql/src/pl_exec.c 5 Feb 2009 15:15:03 -0000 *************** *** 2286,2291 **** --- 2286,2292 ---- PLpgSQL_stmt_return_query *stmt) { Portal portal; + uint32 processed = 0; if (!estate->retisset) ereport(ERROR, *************** *** 2327,2332 **** --- 2328,2334 ---- HeapTuple tuple = SPI_tuptable->vals[i]; tuplestore_puttuple(estate->tuple_store, tuple); + processed++; } MemoryContextSwitchTo(old_cxt); *************** *** 2336,2341 **** --- 2338,2346 ---- SPI_freetuptable(SPI_tuptable); SPI_cursor_close(portal); + estate->eval_processed = processed; + exec_set_found(estate, processed != 0); + return PLPGSQL_RC_OK; } Index: src/test/regress/expected/plpgsql.out =================================================================== RCS file: /cvsroot/pgsql/src/test/regress/expected/plpgsql.out,v retrieving revision 1.66 diff -c -c -r1.66 plpgsql.out *** src/test/regress/expected/plpgsql.out 18 Jul 2008 03:32:53 -0000 1.66 --- src/test/regress/expected/plpgsql.out 5 Feb 2009 15:15:03 -0000 *************** *** 3666,3668 **** --- 3666,3700 ---- (2 rows) drop function tftest(int); + create or replace function rttest() + returns setof int as $$ + declare rc int; + begin + return query values(10),(20); + get diagnostics rc = row_count; + raise notice '% %', found, rc; + return query select * from (values(10),(20)) f(a) where false; + get diagnostics rc = row_count; + raise notice '% %', found, rc; + return query execute 'values(10),(20)'; + get diagnostics rc = row_count; + raise notice '% %', found, rc; + return query execute 'select * from (values(10),(20)) f(a) where false'; + get diagnostics rc = row_count; + raise notice '% %', found, rc; + end; + $$ language plpgsql; + select * from rttest(); + NOTICE: t 2 + NOTICE: f 0 + NOTICE: t 2 + NOTICE: f 0 + rttest + -------- + 10 + 20 + 10 + 20 + (4 rows) + + drop function rttest(); Index: src/test/regress/sql/plpgsql.sql =================================================================== RCS file: /cvsroot/pgsql/src/test/regress/sql/plpgsql.sql,v retrieving revision 1.56 diff -c -c -r1.56 plpgsql.sql *** src/test/regress/sql/plpgsql.sql 18 Jul 2008 03:32:53 -0000 1.56 --- src/test/regress/sql/plpgsql.sql 5 Feb 2009 15:15:03 -0000 *************** *** 2948,2950 **** --- 2948,2974 ---- select * from tftest(10); drop function tftest(int); + + create or replace function rttest() + returns setof int as $$ + declare rc int; + begin + return query values(10),(20); + get diagnostics rc = row_count; + raise notice '% %', found, rc; + return query select * from (values(10),(20)) f(a) where false; + get diagnostics rc = row_count; + raise notice '% %', found, rc; + return query execute 'values(10),(20)'; + get diagnostics rc = row_count; + raise notice '% %', found, rc; + return query execute 'select * from (values(10),(20)) f(a) where false'; + get diagnostics rc = row_count; + raise notice '% %', found, rc; + end; + $$ language plpgsql; + + select * from rttest(); + + drop function rttest(); +