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-DIAGNOSTICSRETURN 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
Show quoted text
--
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
Show quoted text
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>:
Show quoted text
"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
Attachments:
returnquery.difftext/x-patch; name=returnquery.diffDownload
*** ./doc/src/sgml/plpgsql.sgml.orig 2008-11-13 11:44:57.000000000 +0100
--- ./doc/src/sgml/plpgsql.sgml 2008-11-13 11:48:39.000000000 +0100
***************
*** 1356,1361 ****
--- 1356,1368 ----
execution of other statements within the loop body.
</para>
</listitem>
+ <listitem>
+ <para>
+ A <command>RETURN QUERY</command> and <command>RETURN QUERY
+ EXECUTE</command> statements sets <literal>FOUND</literal>
+ true if query returns least one row.
+ </para>
+ </listitem>
</itemizedlist>
<literal>FOUND</literal> is a local variable within each
*** ./src/pl/plpgsql/src/pl_exec.c.orig 2008-11-13 10:53:37.000000000 +0100
--- ./src/pl/plpgsql/src/pl_exec.c 2008-11-13 11:29:24.000000000 +0100
***************
*** 2285,2290 ****
--- 2285,2291 ----
PLpgSQL_stmt_return_query *stmt)
{
Portal portal;
+ uint32 processed = 0;
if (!estate->retisset)
ereport(ERROR,
***************
*** 2326,2331 ****
--- 2327,2333 ----
HeapTuple tuple = SPI_tuptable->vals[i];
tuplestore_puttuple(estate->tuple_store, tuple);
+ processed++;
}
MemoryContextSwitchTo(old_cxt);
***************
*** 2335,2340 ****
--- 2337,2345 ----
SPI_freetuptable(SPI_tuptable);
SPI_cursor_close(portal);
+ estate->eval_processed = processed;
+ exec_set_found(estate, processed != 0);
+
return PLPGSQL_RC_OK;
}
*** ./src/test/regress/expected/plpgsql.out.orig 2008-11-13 11:44:34.000000000 +0100
--- ./src/test/regress/expected/plpgsql.out 2008-11-13 11:42:56.000000000 +0100
***************
*** 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();
*** ./src/test/regress/sql/plpgsql.sql.orig 2008-11-13 11:32:17.000000000 +0100
--- ./src/test/regress/sql/plpgsql.sql 2008-11-13 11:41:20.000000000 +0100
***************
*** 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();
+
Uh, is this ready to be applied?
---------------------------------------------------------------------------
Pavel Stehule wrote:
I am sending patch, that adds FOUND and GET DIAGNOSTICS support for
RETURN QUERY statementRegards
Pavel Stehule2008/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
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
Show quoted text
--
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 Stehule2008/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. +
Attachments:
/rtmp/difftext/x-diffDownload
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();
+