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.

diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 19a1c6e767e8a534aef536f2cd9502cdd65d488b..fd8ce2bce05acfb8c98af9353fd2ddb480f9b0dc 100644 (file)
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -2835,12 +2835,6 @@ exec_assign_value(PLpgSQL_execstate *estate,
                                                         errmsg("NULL cannot be assigned to variable \"%s\" declared NOT NULL",
                                                                        var->refname)));
 
-                               if (var->freeval)
-                               {
-                                       pfree(DatumGetPointer(var->value));
-                                       var->freeval = false;
-                               }
-
                                /*
                                 * If type is by-reference, make sure we have a freshly
                                 * palloc'd copy; the originally passed value may not live
@@ -2851,16 +2845,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;
                        }
 
@@ -3069,11 +3075,13 @@ exec_assign_value(PLpgSQL_execstate *estate,
                                                        oldarrayisnull;
                                Oid                     arraytypeid,
                                                        arrayelemtypeid;
-                               int16           elemtyplen;
+                               int16           arraytyplen,
+                                                       elemtyplen;
                                bool            elemtypbyval;
                                char            elemtypalign;
-                               Datum           oldarrayval,
+                               Datum           oldarraydatum,
                                                        coerced_value;
+                               ArrayType  *oldarrayval;
                                ArrayType  *newarrayval;
 
                                /*
@@ -3102,7 +3110,7 @@ exec_assign_value(PLpgSQL_execstate *estate,
 
                                /* Fetch current value of array datum */
                                exec_eval_datum(estate, target, InvalidOid,
-                                                       &arraytypeid, &oldarrayval, &oldarrayisnull);
+                                                               &arraytypeid, &oldarraydatum, &oldarrayisnull);
 
                                arrayelemtypeid = get_element_type(arraytypeid);
                                if (!OidIsValid(arrayelemtypeid))
@@ -3110,6 +3118,12 @@ exec_assign_value(PLpgSQL_execstate *estate,
                                                        (errcode(ERRCODE_DATATYPE_MISMATCH),
                                                  errmsg("subscripted object is not an array")));
 
+                               get_typlenbyvalalign(arrayelemtypeid,
+                                                                        &elemtyplen,
+                                                                        &elemtypbyval,
+                                                                        &elemtypalign);
+                               arraytyplen = get_typlen(arraytypeid);
+
                                /*
                                 * Evaluate the subscripts, switch into left-to-right
                                 * order
@@ -3127,14 +3141,36 @@ exec_assign_value(PLpgSQL_execstate *estate,
                                }
 
                                /*
-                                * Skip the assignment if we have any nulls, either in the
-                                * original array value, the subscripts, or the righthand
-                                * side. This is pretty bogus but it corresponds to the
-                                * current behavior of ExecEvalArrayRef().
+                                * Skip the assignment if we have any nulls in the subscripts
+                                * or the righthand side. This is pretty bogus but it
+                                * corresponds to the current behavior of ExecEvalArrayRef().
                                 */
-                               if (oldarrayisnull || havenullsubscript || *isNull)
+                               if (havenullsubscript || *isNull)
                                        return;
 
+                               /*
+                                * If the original array is null, cons up an empty array
+                                * so that the assignment can proceed; we'll end with a
+                                * one-element array containing just the assigned-to
+                                * subscript.  This only works for varlena arrays, though;
+                                * for fixed-length array types we skip the assignment.
+                                * Again, this corresponds to the current behavior of
+                                * ExecEvalArrayRef().
+                                */
+                               if (oldarrayisnull)
+                               {
+                                       if (arraytyplen > 0)            /* fixed-length array? */
+                                               return;
+
+                                       oldarrayval = construct_md_array(NULL, 0, NULL, NULL,
+                                                                                                        arrayelemtypeid,
+                                                                                                        elemtyplen,
+                                                                                                        elemtypbyval,
+                                                                                                        elemtypalign);
+                               }
+                               else
+                                       oldarrayval = (ArrayType *) DatumGetPointer(oldarraydatum);
+
                                /* Coerce source value to match array element type. */
                                coerced_value = exec_simple_cast_value(value,
                                                                                                           valtype,
@@ -3145,16 +3181,11 @@ exec_assign_value(PLpgSQL_execstate *estate,
                                /*
                                 * Build the modified array value.
                                 */
-                               get_typlenbyvalalign(arrayelemtypeid,
-                                                                        &elemtyplen,
-                                                                        &elemtypbyval,
-                                                                        &elemtypalign);
-
-                               newarrayval = array_set((ArrayType *) DatumGetPointer(oldarrayval),
+                               newarrayval = array_set(oldarrayval,
                                                                                nsubscripts,
                                                                                subscriptvals,
                                                                                coerced_value,
-                                                                               get_typlen(arraytypeid),
+                                                                               arraytyplen,
                                                                                elemtyplen,
                                                                                elemtypbyval,
                                                                                elemtypalign,
@@ -3692,6 +3723,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);
@@ -3705,7 +3744,7 @@ exec_move_row(PLpgSQL_execstate *estate,
 
                if (HeapTupleIsValid(tup))
                {
-                       rec->tup = heap_copytuple(tup);
+                       rec->tup = tup;
                        rec->freetup = true;
                }
                else if (tupdesc)
@@ -3726,7 +3765,7 @@ exec_move_row(PLpgSQL_execstate *estate,
 
                if (tupdesc)
                {
-                       rec->tupdesc = CreateTupleDescCopy(tupdesc);
+                       rec->tupdesc = tupdesc;
                        rec->freetupdesc = true;
                }
                else