On 5/1/18 19:56, Andrew Gierth wrote:
>  Peter> insert into test1 values (1, repeat('foo', 2000));
> 
> That value is no good because it's too compressible; it'll be left
> inline in the main table rather than being externalized, so the value of
> 'x' in the DO-block is still self-contained (though it's still toasted
> in the sense of being VARATT_IS_EXTENDED).

Right.  I added

alter table test1 alter column b set storage external;

then I can see the error.

The attached test fixes this issue by flattening the toast values before
storing them into PL/pgSQL variables.  It could use another check to see
if there are other code paths that need similar adjustments, but I think
it's the right idea in general.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 294b68508cade1dd0df5b78c0f0ec12d7ce761b8 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 3 May 2018 10:54:13 -0400
Subject: [PATCH v1] PL/pgSQL: Flatten TOAST data in nonatomic context

When in a nonatomic execution context, when storing a potentially
toasted datum into a PL/pgSQL variable, we need to flatten the
datum (i.e., remove any references to TOAST data).  Otherwise, a
transaction end combined with, say, a concurrent VACUUM, between storing
the datum and reading it, could remove the TOAST data, and then the data
in the variable would no longer be readable.
---
 src/pl/plpgsql/src/pl_exec.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 228d1c0d00..28a6957286 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -20,6 +20,7 @@
 #include "access/htup_details.h"
 #include "access/transam.h"
 #include "access/tupconvert.h"
+#include "access/tuptoaster.h"
 #include "catalog/pg_proc.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
@@ -6621,6 +6622,16 @@ exec_move_row(PLpgSQL_execstate *estate,
 {
        ExpandedRecordHeader *newerh = NULL;
 
+       /*
+        * If not in atomic context, flatten TOAST references.  Otherwise the
+        * TOAST data might disappear if a transaction is committed.
+        */
+       if (!estate->atomic)
+       {
+               if (tup)
+                       tup = toast_flatten_tuple(tup, tupdesc);
+       }
+
        /*
         * If target is RECORD, we may be able to avoid field-by-field 
processing.
         */

base-commit: 30c66e77be1d890c3cca766259c0bec80bcac1b5
-- 
2.17.0

Reply via email to