From a6f0dee7757aa843eb021c051fb2e85b03efc05e Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 20 Jun 2005 20:44:57 +0000
Subject: [PATCH] plpgsql's exec_assign_value() freed the old value of a
 variable before copying/converting the new value, which meant that it failed
 badly on "var := var" if var is of pass-by-reference type.  Fix this and a
 similar hazard in exec_move_row(); not sure that the latter can manifest
 before 8.0, but patch it all the way back anyway.  Per report from Dave
 Chapeskie.

---
 src/pl/plpgsql/src/pl_exec.c | 48 +++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 0f42d048236..102b94b7894 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3,7 +3,7 @@
  *			  procedural language
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.93.2.1 2004/02/24 01:44:47 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.93.2.2 2005/06/20 20:44:57 tgl Exp $
  *
  *	  This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -2707,12 +2707,6 @@ exec_assign_value(PLpgSQL_execstate * estate,
 			 */
 			var = (PLpgSQL_var *) target;
 
-			if (var->freeval)
-			{
-				pfree(DatumGetPointer(var->value));
-				var->freeval = false;
-			}
-
 			newvalue = exec_cast_value(value, valtype, var->datatype->typoid,
 									   &(var->datatype->typinput),
 									   var->datatype->typelem,
@@ -2735,16 +2729,28 @@ exec_assign_value(PLpgSQL_execstate * estate,
 			if (!var->datatype->typbyval && !*isNull)
 			{
 				if (newvalue == value)
-					var->value = datumCopy(newvalue,
-										   false,
-										   var->datatype->typlen);
-				else
-					var->value = newvalue;
-				var->freeval = true;
+					newvalue = datumCopy(newvalue,
+										 false,
+										 var->datatype->typlen);
 			}
-			else
-				var->value = newvalue;
+
+			/*
+			 * Now free the old value.  (We can't do this any earlier
+			 * because of the possibility that we are assigning the
+			 * var's old value to it, eg "foo := foo".  We could optimize
+			 * out the assignment altogether in such cases, but it's too
+			 * infrequent to be worth testing for.)
+			 */
+			if (var->freeval)
+			{
+				pfree(DatumGetPointer(var->value));
+				var->freeval = false;
+			}
+
+			var->value = newvalue;
 			var->isnull = *isNull;
+			if (!var->datatype->typbyval && !*isNull)
+				var->freeval = true;
 			break;
 
 		case PLPGSQL_DTYPE_RECFIELD:
@@ -3363,6 +3369,14 @@ exec_move_row(PLpgSQL_execstate * estate,
 	 */
 	if (rec != NULL)
 	{
+		/*
+		 * copy input first, just in case it is pointing at variable's value
+		 */
+		if (HeapTupleIsValid(tup))
+			tup = heap_copytuple(tup);
+		if (tupdesc)
+			tupdesc = CreateTupleDescCopy(tupdesc);
+
 		if (rec->freetup)
 		{
 			heap_freetuple(rec->tup);
@@ -3376,7 +3390,7 @@ exec_move_row(PLpgSQL_execstate * estate,
 
 		if (HeapTupleIsValid(tup))
 		{
-			rec->tup = heap_copytuple(tup);
+			rec->tup = tup;
 			rec->freetup = true;
 		}
 		else if (tupdesc)
@@ -3398,7 +3412,7 @@ exec_move_row(PLpgSQL_execstate * estate,
 
 		if (tupdesc)
 		{
-			rec->tupdesc = CreateTupleDescCopy(tupdesc);
+			rec->tupdesc = tupdesc;
 			rec->freetupdesc = true;
 		}
 		else