Detoast plpgsql variables if they might live across a transaction boundary.

Up to now, it's been safe for plpgsql to store TOAST pointers in its
variables because the ActiveSnapshot for whatever query called the plpgsql
function will surely protect such TOAST values from being vacuumed away,
even if the owning table rows are committed dead.  With the introduction of
procedures, that assumption is no longer good in "non atomic" executions
of plpgsql code.  We adopt the slightly brute-force solution of detoasting
all TOAST pointers at the time they are stored into variables, if we're in
a non-atomic context, just in case the owning row goes away.

Some care is needed to avoid long-term memory leaks, since plpgsql tends
to run with CurrentMemoryContext pointing to its call-lifespan context,
but we shouldn't assume that no memory is leaked by heap_tuple_fetch_attr.
In plpgsql proper, we can do the detoasting work in the "eval_mcontext".

Most of the code thrashing here is due to the need to add this capability
to expandedrecord.c as well as plpgsql proper.  In expandedrecord.c,
we can't assume that the caller's context is short-lived, so make use of
the short-term sub-context that was already invented for checking domain
constraints.  In view of this repurposing, it seems good to rename that
variable and associated code from "domain_check_cxt" to "short_term_cxt".

Peter Eisentraut and Tom Lane

Discussion: https://postgr.es/m/5AC06865.9050005@anastigmatix.net

1 files changed, 9 insertions, 5 deletions

diff --git a/src/include/utils/expandedrecord.h b/src/include/utils/expandedrecord.h
index a95c9cce22e..c999f44f38f 100644
--- a/src/include/utils/expandedrecord.h
+++ b/src/include/utils/expandedrecord.h
@@ -127,8 +127,10 @@ typedef struct ExpandedRecordHeader
 	char	   *fstartptr;		/* start of its data area */
 	char	   *fendptr;		/* end+1 of its data area */
 
+	/* Some operations on the expanded record need a short-lived context */
+	MemoryContext er_short_term_cxt;	/* short-term memory context */
+
 	/* Working state for domain checking, used if ER_FLAG_IS_DOMAIN is set */
-	MemoryContext er_domain_check_cxt;	/* short-term memory context */
 	struct ExpandedRecordHeader *er_dummy_header;	/* dummy record header */
 	void	   *er_domaininfo;	/* cache space for domain_check() */
 
@@ -171,7 +173,7 @@ extern ExpandedRecordHeader *make_expanded_record_from_tupdesc(TupleDesc tupdesc
 extern ExpandedRecordHeader *make_expanded_record_from_exprecord(ExpandedRecordHeader *olderh,
 									MemoryContext parentcontext);
 extern void expanded_record_set_tuple(ExpandedRecordHeader *erh,
-						  HeapTuple tuple, bool copy);
+						  HeapTuple tuple, bool copy, bool expand_external);
 extern Datum make_expanded_record_from_datum(Datum recorddatum,
 								MemoryContext parentcontext);
 extern TupleDesc expanded_record_fetch_tupdesc(ExpandedRecordHeader *erh);
@@ -186,13 +188,15 @@ extern Datum expanded_record_fetch_field(ExpandedRecordHeader *erh, int fnumber,
 extern void expanded_record_set_field_internal(ExpandedRecordHeader *erh,
 								   int fnumber,
 								   Datum newValue, bool isnull,
+								   bool expand_external,
 								   bool check_constraints);
 extern void expanded_record_set_fields(ExpandedRecordHeader *erh,
-						   const Datum *newValues, const bool *isnulls);
+						   const Datum *newValues, const bool *isnulls,
+						   bool expand_external);
 
 /* outside code should never call expanded_record_set_field_internal as such */
-#define expanded_record_set_field(erh, fnumber, newValue, isnull) \
-	expanded_record_set_field_internal(erh, fnumber, newValue, isnull, true)
+#define expanded_record_set_field(erh, fnumber, newValue, isnull, expand_external) \
+	expanded_record_set_field_internal(erh, fnumber, newValue, isnull, expand_external, true)
 
 /*
  * Inline-able fast cases.  The expanded_record_fetch_xxx functions above