pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17199: Calling stored procedure with stable function as argument results in wrong result
3+ messages / 2 participants
[nested] [flat]
* BUG #17199: Calling stored procedure with stable function as argument results in wrong result
@ 2021-09-21 12:52 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 3+ messages in thread
From: PG Bug reporting form @ 2021-09-21 12:52 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: notegihu@confused.at
The following bug has been logged on the website:
Bug reference: 17199
Logged by: Alexander Nawratil
Email address: notegihu@confused.at
PostgreSQL version: 13.4
Operating system: Linux
Description:
Hello,
we are facing some inconsistencies in one of our unit tests since the last
upgrade of PostgreSQL to 11.13 and 13.4.
When a function that returns just a row count of a table is marked as STABLE
and is called from a stored procedure as argument, the result of the
function is different than when the function is called beforehand and stored
to a local variable.
It's working fine when the function is marked as VOLATILE, or on PostgreSQL
11.12 / 13.3.
Steps to reproduce:
1. Create an empty table
2. Create a function that is marked as STABLE and returns the row count of
the table (RETURN SELECT COUNT(*) FROM table)
3. Create a procedure that takes an integer parameter and simply outputs it,
e.g. RAISE NOTICE
4. In a single DO block: INSERT a row into the table, then CALL
proc(func()); /* --> output: 0, instead of 1 */
5. Storing the result of func() to a variable x, then CALL proc(x); /* -->
output: 1, as expected */
Example SQL:
CREATE TABLE tbl (col INT4);
CREATE OR REPLACE FUNCTION stable_func()
RETURNS INT4
STABLE AS
$code$
BEGIN
RETURN (SELECT COUNT(1) FROM tbl);
END
$code$ LANGUAGE plpgsql;
CREATE OR REPLACE FUNCTION volatile_func()
RETURNS INT4
VOLATILE AS
$code$
BEGIN
RETURN (SELECT COUNT(1) FROM tbl);
END
$code$ LANGUAGE plpgsql;
CREATE OR REPLACE PROCEDURE call_proc(t TEXT, cnt INT4)
AS
$code$
BEGIN
RAISE NOTICE '%', (t || ' -- count: ' || cnt::TEXT);
END
$code$ LANGUAGE plpgsql;
DO
$$
DECLARE
x INT4;
BEGIN
RAISE NOTICE '%', (VERSION());
INSERT INTO tbl(col) VALUES (1);
x := volatile_func();
CALL call_proc('from_var_volatile_func', x);
CALL call_proc('inline_volatile_func', volatile_func());
x := stable_func();
CALL call_proc('from_var_stable_func', x);
CALL call_proc('inline_stable_func', stable_func());
END
$$;
I would expect that all calls output the same row count of 1. However, the
actual value differs for the last call since the upgrade.
Output on Postgres 13.3:
PostgreSQL 13.3 (Debian 13.3-1.pgdg100+1) on x86_64-pc-linux-gnu, compiled
by gcc (Debian 8.3.0-6) 8.3.0, 64-bit
from_var_volatile_func -- count: 1
inline_volatile_func -- count: 1
from_var_stable_func -- count: 1
inline_stable_func -- count: 1
Output on Postgres 13.4:
PostgreSQL 13.4 (Debian 13.4-1.pgdg100+1) on x86_64-pc-linux-gnu, compiled
by gcc (Debian 8.3.0-6) 8.3.0, 64-bit
from_var_volatile_func -- count: 1
inline_volatile_func -- count: 1
from_var_stable_func -- count: 1
inline_stable_func -- count: 0
The same change in behavior seems to be introduced in 12.8, not being
present in 12.7. I ran the code above using official Docker Hub images.
Best regards,
Alexander Nawratil
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: BUG #17199: Calling stored procedure with stable function as argument results in wrong result
@ 2021-09-21 15:01 Tom Lane <tgl@sss.pgh.pa.us>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 3+ messages in thread
From: Tom Lane @ 2021-09-21 15:01 UTC (permalink / raw)
To: notegihu@confused.at; +Cc: pgsql-bugs@lists.postgresql.org
PG Bug reporting form <noreply@postgresql.org> writes:
> When a function that returns just a row count of a table is marked as STABLE
> and is called from a stored procedure as argument, the result of the
> function is different than when the function is called beforehand and stored
> to a local variable.
Ugh. Looks like I broke this in 84f5c2908, by not thinking about the
possibility that a CALL's argument expressions would need an up-to-date
snapshot.
Will fix, thanks for the report!
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: BUG #17199: Calling stored procedure with stable function as argument results in wrong result
@ 2021-09-21 15:49 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 3+ messages in thread
From: Tom Lane @ 2021-09-21 15:49 UTC (permalink / raw)
To: notegihu@confused.at; +Cc: pgsql-bugs@lists.postgresql.org
I wrote:
> Ugh. Looks like I broke this in 84f5c2908, by not thinking about the
> possibility that a CALL's argument expressions would need an up-to-date
> snapshot.
Concretely, the attached seems to take care of it. I'll refrain
from pushing this till later, though, since v14 is frozen for rc1
for a few hours more.
regards, tom lane
Attachments:
eval-CALL-arguments-with-fresh-snapshot.patch (text/x-diff, 3.7K) download
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 79d875ab10..38e78f7102 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -73,6 +73,7 @@
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/rel.h"
+#include "utils/snapmgr.h"
#include "utils/syscache.h"
#include "utils/typcache.h"
@@ -2250,6 +2251,16 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver
estate->es_param_list_info = params;
econtext = CreateExprContext(estate);
+ /*
+ * If we're called in non-atomic context, we also have to ensure that the
+ * argument expressions run with an up-to-date snapshot. Our caller will
+ * have provided a current snapshot in atomic contexts, but not in
+ * non-atomic contexts, because the possibility of a COMMIT/ROLLBACK
+ * destroying the snapshot makes higher-level management too complicated.
+ */
+ if (!atomic)
+ PushActiveSnapshot(GetTransactionSnapshot());
+
i = 0;
foreach(lc, fexpr->args)
{
@@ -2267,20 +2278,23 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver
i++;
}
+ /* Get rid of temporary snapshot for arguments, if we made one */
+ if (!atomic)
+ PopActiveSnapshot();
+
+ /* Here we actually call the procedure */
pgstat_init_function_usage(fcinfo, &fcusage);
retval = FunctionCallInvoke(fcinfo);
pgstat_end_function_usage(&fcusage, true);
+ /* Handle the procedure's outputs */
if (fexpr->funcresulttype == VOIDOID)
{
/* do nothing */
}
else if (fexpr->funcresulttype == RECORDOID)
{
- /*
- * send tuple to client
- */
-
+ /* send tuple to client */
HeapTupleHeader td;
Oid tupType;
int32 tupTypmod;
diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
index 57ab0bc0e7..f79f847321 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
@@ -600,6 +600,27 @@ SELECT * FROM test2;
42
(1 row)
+-- another snapshot handling case: argument expressions of a CALL need
+-- to be evaluated with an up-to-date snapshot
+CREATE FUNCTION report_count() RETURNS int
+STABLE LANGUAGE sql
+AS $$ SELECT COUNT(*) FROM test2 $$;
+CREATE PROCEDURE transaction_test9b(cnt int)
+LANGUAGE plpgsql
+AS $$
+BEGIN
+ RAISE NOTICE 'count = %', cnt;
+END
+$$;
+DO $$
+BEGIN
+ CALL transaction_test9b(report_count());
+ INSERT INTO test2 VALUES(43);
+ CALL transaction_test9b(report_count());
+END
+$$;
+NOTICE: count = 1
+NOTICE: count = 2
-- Test transaction in procedure with output parameters. This uses a
-- different portal strategy and different code paths in pquery.c.
CREATE PROCEDURE transaction_test10a(INOUT x int)
diff --git a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
index 8e4783c9a5..888ddccace 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
@@ -507,6 +507,29 @@ $$;
SELECT * FROM test2;
+-- another snapshot handling case: argument expressions of a CALL need
+-- to be evaluated with an up-to-date snapshot
+CREATE FUNCTION report_count() RETURNS int
+STABLE LANGUAGE sql
+AS $$ SELECT COUNT(*) FROM test2 $$;
+
+CREATE PROCEDURE transaction_test9b(cnt int)
+LANGUAGE plpgsql
+AS $$
+BEGIN
+ RAISE NOTICE 'count = %', cnt;
+END
+$$;
+
+DO $$
+BEGIN
+ CALL transaction_test9b(report_count());
+ INSERT INTO test2 VALUES(43);
+ CALL transaction_test9b(report_count());
+END
+$$;
+
+
-- Test transaction in procedure with output parameters. This uses a
-- different portal strategy and different code paths in pquery.c.
CREATE PROCEDURE transaction_test10a(INOUT x int)
end of thread, other threads:[~2021-09-21 15:49 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-09-21 12:52 BUG #17199: Calling stored procedure with stable function as argument results in wrong result PG Bug reporting form <noreply@postgresql.org>
2021-09-21 15:01 ` Tom Lane <tgl@sss.pgh.pa.us>
2021-09-21 15:49 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox