From: Tom Lane <tgl@sss.pgh.pa.us>
To: notegihu@confused.at
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17199: Calling stored procedure with stable function as argument results in wrong result
Date: Tue, 21 Sep 2021 11:49:56 -0400
Message-ID: <2031644.1632239396@sss.pgh.pa.us> (raw)
In-Reply-To: <1980152.1632236471@sss.pgh.pa.us>
References: <17199-1ab2561f0d94af92@postgresql.org>
<1980152.1632236471@sss.pgh.pa.us>
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:
[text/x-diff] eval-CALL-arguments-with-fresh-snapshot.patch (3.7K, ../2031644.1632239396@sss.pgh.pa.us/2-eval-CALL-arguments-with-fresh-snapshot.patch)
download | inline diff:diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.cindex 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.outindex 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.sqlindex 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)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, notegihu@confused.at, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17199: Calling stored procedure with stable function as argument results in wrong result
In-Reply-To: <2031644.1632239396@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
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