Question about using AggCheckCallContext in a C function

Hi everyone. A while back, I wrote a function to sum array contents (think
1-D matrix addition), and am using it in a custom SUM aggregate. Here's an
example:

CREATE TABLE foo (arr INT[]);
INSERT INTO foo VALUES ('{1, 2, 3}'), ('{4, 5, 6}');

SELECT SUM(arr) FROM foo;
sum
---------
{5,7,9}
(1 row)

This works, but I got some great advice from Craig Ringer about how I can
make it even faster using AggCheckCallContext(). See http://stackoverflow.com/a/16996606/6198

I'm trying to implement this now, and I'm running into occasional memory
corruption. Sometimes the back-end segfaults, sometimes I get seemingly-
random "ERROR: could not open relation with OID xyz" errors.

Here's the relevant code snippet:

// Assumptions:
// 1. We will never be called with a null array. The CREATE FUNCTION call should specify STRICT to prevent PostgreSQL from doing this.
// 2. The arrays will never contain null values. It's up to the caller to ensure this.
// 3. The arrays will always be the same length. It's up to the caller to ensure this.

ArrayType *array1, *array2;
Datum *arrayData1, *arrayData2;
int arrayLength1;

array2 = PG_GETARG_ARRAYTYPE_P(1);

if (AggCheckCallContext(fcinfo, NULL))
{
// We are being called by an aggregate.

if (PG_ARGISNULL(0))
{
// This can happen on the very first call as PostgreSQL sets up the "temporary transition value" for the aggregate.

// NOTE: This never seems to happen! Is it because the function is STRICT?

// Return a copy of the second array.
PG_RETURN_ARRAYTYPE_P(copy_intArrayType(array2));
}
else
{
// This means that the first array is a "temporary transition value", and we can safely modify it directly with no side effects.
// This avoids the overhead of creating a new array object.

array1 = PG_GETARG_ARRAYTYPE_P(0);

// Get a direct pointer to each array's contents.
arrayData1 = ARR_DATA_PTR(array1);
arrayData2 = ARR_DATA_PTR(array2);

// Get the length of the first array (should be the same as the second).
arrayLength1 = (ARR_DIMS(array1))[0];

// Add the contents of the second array to the first.
for (i = 0; i < arrayLength1; i++)
{
arrayData1[i] += arrayData2[i];
}

// Return the updated array.
PG_RETURN_ARRAYTYPE_P(array1);
}
}
else
{
// We are not being called by an aggregate.
// <omitted>
}

After poring over the code in nodeAgg.c, and looking at the in8inc()
function, I think I know what the problem is: the typical use of
AggCheckCallContext() is not compatible with TOAST-able data types.

When the state transition function is STRICT (as mine is), and
advance_transition_function() comes across the first non-NULL value,
it makes a copy using datumCopy(). However, from what I can tell,
datumCopy() is not really "compatible" with TOAST for this
particular use case.

Am I on the right track here? If so, what's the best way to solve
this problem? I would appreciate any help you can offer.

Sincerely,
Matt Solnit <msolnit(at)soasta(dot)com>