Re: BUG #17199: Calling stored procedure with stable function as argument results in wrong result

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

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)