Fix things so that array_agg_finalfn does not modify or free its input

ArrayBuildState, per trouble report from Merlin Moncure.  By adopting
this fix, we are essentially deciding that aggregate final-functions
should not modify their inputs ever.  Adjust documentation and comments
to match that conclusion.

1 files changed, 15 insertions, 3 deletions

diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index b3973fbccb9..54c489a8969 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/arrayfuncs.c,v 1.157 2009/06/11 14:49:03 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/arrayfuncs.c,v 1.158 2009/06/20 18:45:28 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -4179,9 +4179,21 @@ accumArrayResult(ArrayBuildState *astate,
 		}
 	}
 
-	/* Use datumCopy to ensure pass-by-ref stuff is copied into mcontext */
+	/*
+	 * Ensure pass-by-ref stuff is copied into mcontext; and detoast it too
+	 * if it's varlena.  (You might think that detoasting is not needed here
+	 * because construct_md_array can detoast the array elements later.
+	 * However, we must not let construct_md_array modify the ArrayBuildState
+	 * because that would mean array_agg_finalfn damages its input, which
+	 * is verboten.  Also, this way frequently saves one copying step.)
+	 */
 	if (!disnull && !astate->typbyval)
-		dvalue = datumCopy(dvalue, astate->typbyval, astate->typlen);
+	{
+		if (astate->typlen == -1)
+			dvalue = PointerGetDatum(PG_DETOAST_DATUM_COPY(dvalue));
+		else
+			dvalue = datumCopy(dvalue, astate->typbyval, astate->typlen);
+	}
 
 	astate->dvalues[astate->nelems] = dvalue;
 	astate->dnulls[astate->nelems] = disnull;