Re: Assertion failure in HEAD and 13 after calling COMMIT in a stored proc - Mailing list pgsql-hackers

I wrote:
> The attached seems to be enough to resolve Jim's example.  I'd like
> to invent a test case that involves a detoast of the simple
> expression's result, too, to show that transiently pushing a
> snapshot for the duration of the expression is not the right fix.

Here we go.  This test case gives "cannot fetch toast data without an
active snapshot" in v11 and v12 branch tips.  Since those branches lack
the 73b06cf89 optimization, they push a snapshot while calling the
SQL-language function, thus it doesn't complain.  But what comes back
is toasted, and then we fail trying to detoast it.

            regards, tom lane

diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
index 918cc0913e..35845d1d6b 100644
--- 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 392456ae85..06bdd04774 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -35,6 +35,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"
@@ -6153,6 +6154,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 cc26788b9a..8e4783c9a5 100644
--- 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