Fix snapshot warning for some procedures

The problem arises with the combination of CALL with output parameters
and doing a COMMIT inside the procedure.  When a CALL has output
parameters, the portal uses the strategy PORTAL_UTIL_SELECT instead of
PORTAL_MULTI_QUERY.  Using PORTAL_UTIL_SELECT causes the portal's
snapshot to be registered with the current resource
owner (portal->holdSnapshot); see
9ee1cf04ab6bcefe03a11837b53f29ca9dc24c7a for the reason.

Normally, PortalDrop() unregisters the snapshot.  If not, then
ResourceOwnerRelease() will print a warning about a snapshot  on
transaction commit.  A transaction commit normally drops all
portals (PreCommit_Portals()), except the active portal.  So in case of
the active portal, we need to manually release the snapshot to avoid the
warning.

Reported-by: Prabhat Sahu <[email protected]>
Reviewed-by: Jonathan S. Katz <[email protected]>

diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index 04ea32f49f2107aea43eb41617128cc6c0ddc36e..d34cab0eb88c84085e2fca0362f043348e500ece 100644 (file)
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -689,13 +689,23 @@ PreCommit_Portals(bool isPrepare)
 
        /*
         * Do not touch active portals --- this can only happen in the case of
-        * a multi-transaction utility command, such as VACUUM.
+        * a multi-transaction utility command, such as VACUUM, or a commit in
+        * a procedure.
         *
         * Note however that any resource owner attached to such a portal is
-        * still going to go away, so don't leave a dangling pointer.
+        * still going to go away, so don't leave a dangling pointer.  Also
+        * unregister any snapshots held by the portal, mainly to avoid
+        * snapshot  warnings from ResourceOwnerRelease().
         */
        if (portal->status == PORTAL_ACTIVE)
        {
+           if (portal->holdSnapshot)
+           {
+               if (portal->resowner)
+                   UnregisterSnapshotFromOwner(portal->holdSnapshot,
+                                               portal->resowner);
+               portal->holdSnapshot = NULL;
+           }
            portal->resowner = NULL;
            continue;
        }

diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
index 0b5a039b89cf3e63b55a71c5c1c2e299cbcf6983..77a83adab54bb13177ae35ea9805556d2a341f65 100644 (file)
--- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
@@ -463,6 +463,36 @@ SELECT * FROM test2;
  42
 (1 row)
 
+-- 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)
+LANGUAGE plpgsql
+AS $$
+BEGIN
+  x := x + 1;
+  COMMIT;
+END;
+$$;
+CALL transaction_test10a(10);
+ x  
+----
+ 11
+(1 row)
+
+CREATE PROCEDURE transaction_test10b(INOUT x int)
+LANGUAGE plpgsql
+AS $$
+BEGIN
+  x := x - 1;
+  ROLLBACK;
+END;
+$$;
+CALL transaction_test10b(10);
+ x 
+---
+ 9
+(1 row)
+
 DROP TABLE test1;
 DROP TABLE test2;
 DROP TABLE test3;

diff --git a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
index 236db9bf2bfd4c6c2f900d5b59cf738cc234d46e..0ed9ab873a4daef1a3babec71d5624e5a3c5c5e1 100644 (file)
--- a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
@@ -387,6 +387,31 @@ $$;
 SELECT * FROM test2;
 
 
+-- 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)
+LANGUAGE plpgsql
+AS $$
+BEGIN
+  x := x + 1;
+  COMMIT;
+END;
+$$;
+
+CALL transaction_test10a(10);
+
+CREATE PROCEDURE transaction_test10b(INOUT x int)
+LANGUAGE plpgsql
+AS $$
+BEGIN
+  x := x - 1;
+  ROLLBACK;
+END;
+$$;
+
+CALL transaction_test10b(10);
+
+
 DROP TABLE test1;
 DROP TABLE test2;
 DROP TABLE test3;