Restore the portal-level snapshot for simple expressions, too.

Commits 84f5c2908 et al missed the need to cover plpgsql's "simple
expression" code path.  If the first thing we execute after a
COMMIT/ROLLBACK is one of those, rather than a full-fledged SPI command,
we must explicitly do EnsurePortalSnapshotExists() to make sure we have
an outer snapshot.  Note that it wouldn't be good enough to just push a
snapshot for the duration of the expression execution: what comes back
might be toasted, so we'd better have a snapshot protecting it.

The test case demonstrating this fact cheats a bit by marking a SQL
function immutable even though it fetches from a table.  That's
nothing that users haven't been seen to do, though.

Per report from Jim Nasby.  Back-patch to v11, like the previous fix.

Discussion: https://postgr.es/m/378885e4-f85f-fc28-6c91-c4d1c080bf26@amazon.com

diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
index 595af79d56596e8323329baf2a6282156ffe993a..817cf2c5cd6ce2bf4b05abc0b33fb73dac8f4247 100644 (file)
--- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
@@ -430,6 +430,24 @@ SELECT * FROM test1;
 ---+---
 (0 rows)
 
+-- detoast result of simple expression after commit
+CREATE TEMP TABLE test4(f1 text);
+ALTER TABLE test4 ALTER COLUMN f1 SET STORAGE EXTERNAL; -- disable compression
+INSERT INTO test4 SELECT repeat('xyzzy', 2000);
+-- immutable mark is a bit of a lie, but it serves to make call a simple expr
+-- that will return a still-toasted value
+CREATE FUNCTION data_source(i int) RETURNS TEXT LANGUAGE sql
+AS 'select f1 from test4' IMMUTABLE;
+DO $$
+declare x text;
+begin
+  for i in 1..3 loop
+    x := data_source(i);
+    commit;
+  end loop;
+  raise notice 'length(x) = %', length(x);
+end $$;
+NOTICE:  length(x) = 10000
 -- operations on composite types vs. internal transactions
 DO LANGUAGE plpgsql $$
 declare

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 0b56c4ee4f8d744cc784815bf5f4cd606002033b..61b717c85b1e96a301dc3e8dd336356aaa19f039 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -36,6 +36,7 @@
 #include "parser/parse_type.h"
 #include "parser/scansup.h"
 #include "storage/proc.h"
+#include "tcop/pquery.h"
 #include "tcop/tcopprot.h"
 #include "tcop/utility.h"
 #include "utils/array.h"
@@ -6123,6 +6124,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
        if (expr->expr_simple_in_use && expr->expr_simple_lxid == curlxid)
                return false;
 
+       /*
+        * Ensure that there's a portal-level snapshot, in case this simple
+        * expression is the first thing evaluated after a COMMIT or ROLLBACK.
+        * We'd have to do this anyway before executing the expression, so we
+        * might as well do it now to ensure that any possible replanning doesn't
+        * need to take a new snapshot.
+        */
+       EnsurePortalSnapshotExists();
+
        /*
         * Revalidate cached plan, so that we will notice if it became stale. (We
         * need to hold a refcount while using the plan, anyway.)  If replanning

diff --git a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
index 52b047ea6a2f25b031aa2a48ef84d6474ba7073d..587d74bd79a86e80b8b4556813afd8ab42b0c8e6 100644 (file)
--- a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
@@ -354,6 +354,27 @@ $$;
 SELECT * FROM test1;
 
 
+-- detoast result of simple expression after commit
+CREATE TEMP TABLE test4(f1 text);
+ALTER TABLE test4 ALTER COLUMN f1 SET STORAGE EXTERNAL; -- disable compression
+INSERT INTO test4 SELECT repeat('xyzzy', 2000);
+
+-- immutable mark is a bit of a lie, but it serves to make call a simple expr
+-- that will return a still-toasted value
+CREATE FUNCTION data_source(i int) RETURNS TEXT LANGUAGE sql
+AS 'select f1 from test4' IMMUTABLE;
+
+DO $$
+declare x text;
+begin
+  for i in 1..3 loop
+    x := data_source(i);
+    commit;
+  end loop;
+  raise notice 'length(x) = %', length(x);
+end $$;
+
+
 -- operations on composite types vs. internal transactions
 DO LANGUAGE plpgsql $$
 declare