Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Restore the portal-level snapshot for simple expressions, too.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 22 Jun 2021 21:48:39 +0000 (17:48 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 22 Jun 2021 21:48:39 +0000 (17:48 -0400)
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

src/pl/plpgsql/src/expected/plpgsql_transaction.out
src/pl/plpgsql/src/pl_exec.c
src/pl/plpgsql/src/sql/plpgsql_transaction.sql

index 76cbdca0c56aaccab1943a2a54e2cf43e4d42aae..57ab0bc0e7d6efbbde673e8a238673aa969e6437 100644 (file)
@@ -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
index 0ce382e123251ea7b14a2ba4ac002b15b3419378..96bb77e0b1e30f68cd70afb3aba9d1a8f6008909 100644 (file)
@@ -38,6 +38,7 @@
 #include "plpgsql.h"
 #include "storage/proc.h"
 #include "tcop/cmdtag.h"
+#include "tcop/pquery.h"
 #include "tcop/tcopprot.h"
 #include "tcop/utility.h"
 #include "utils/array.h"
@@ -5958,6 +5959,15 @@ exec_eval_simple_expr(PLpgSQL_execstate *estate,
        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();
+
    /*
     * Check to see if the cached plan has been invalidated.  If not, and this
     * is the first use in the current transaction, save a plan refcount in
index cc26788b9ae7cfe4ca808480f6c634b8bf10f428..8e4783c9a5149986e40304125854ed3012d8d41e 100644 (file)
@@ -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