Minor code clarity improvements in array_agg functions, and add a comment

about how this is playing fast and loose with the type system.

diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c
index 4eeb64dbd81747b3d3055b7bf59578fc9ca594e3..6f18c6640599c20e1d78414915d60590e5b3b887 100644 (file)
--- a/src/backend/utils/adt/array_userfuncs.c
+++ b/src/backend/utils/adt/array_userfuncs.c
@@ -6,7 +6,7 @@
  * Copyright (c) 2003-2008, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/adt/array_userfuncs.c,v 1.24 2008/11/13 15:59:50 petere Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/adt/array_userfuncs.c,v 1.25 2008/11/14 00:12:08 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -467,33 +467,57 @@ create_singleton_array(FunctionCallInfo fcinfo,
                                                          typlen, typbyval, typalign);
 }
 
+
+/*
+ * ARRAY_AGG aggregate function
+ */
 Datum
 array_agg_transfn(PG_FUNCTION_ARGS)
 {
        Oid arg1_typeid = get_fn_expr_argtype(fcinfo->flinfo, 1);
+       ArrayBuildState *state;
+       Datum           elem;
 
        if (arg1_typeid == InvalidOid)
-               ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-                                               errmsg("could not determine input data type")));
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("could not determine input data type")));
 
        /* cannot be called directly because of internal-type argument */
        Assert(fcinfo->context && IsA(fcinfo->context, AggState));
 
-       PG_RETURN_POINTER(accumArrayResult(PG_ARGISNULL(0) ? NULL : (ArrayBuildState *) PG_GETARG_POINTER(0),
-                                                                          PG_ARGISNULL(1) ? (Datum) 0 : PG_GETARG_DATUM(1),
-                                                                          PG_ARGISNULL(1),
-                                                                          arg1_typeid,
-                                                                          ((AggState *) fcinfo->context)->aggcontext));
+       state = PG_ARGISNULL(0) ? NULL : (ArrayBuildState *) PG_GETARG_POINTER(0);
+       elem = PG_ARGISNULL(1) ? (Datum) 0 : PG_GETARG_DATUM(1);
+       state = accumArrayResult(state,
+                                                        elem,
+                                                        PG_ARGISNULL(1),
+                                                        arg1_typeid,
+                                                        ((AggState *) fcinfo->context)->aggcontext);
+
+       /*
+        * We cheat quite a lot here by assuming that a pointer datum will be
+        * preserved intact when nodeAgg.c thinks it is a value of type "internal".
+        * This will in fact work because internal is stated to be pass-by-value
+        * in pg_type.h, and nodeAgg will never do anything with a pass-by-value
+        * transvalue except pass it around in Datum form.  But it's mighty
+        * shaky seeing that internal is also stated to be 4 bytes wide in
+        * pg_type.h.  If nodeAgg did put the value into a tuple this would
+        * crash and burn on 64-bit machines.
+        */
+       PG_RETURN_POINTER(state);
 }
 
 Datum
 array_agg_finalfn(PG_FUNCTION_ARGS)
 {
+       ArrayBuildState *state;
+
        /* cannot be called directly because of internal-type argument */
        Assert(fcinfo->context && IsA(fcinfo->context, AggState));
 
        if (PG_ARGISNULL(0))
                PG_RETURN_NULL();   /* returns null iff no input values */
 
-       PG_RETURN_ARRAYTYPE_P(makeArrayResult((ArrayBuildState *) PG_GETARG_POINTER(0), CurrentMemoryContext));
+       state = (ArrayBuildState *) PG_GETARG_POINTER(0);
+       PG_RETURN_ARRAYTYPE_P(makeArrayResult(state, CurrentMemoryContext));
 }