I wrote: > The main thing that remains undone is to get some test coverage --- > AFAICS, none of these new functions get exercised in the standard > regression tests.
Oh, I thought of another thing that would need to get done, if we decide to commit this. array_agg_serialize/deserialize only work if the array element type has send/receive functions. The planner's logic to decide whether partial aggregation is possible doesn't look any deeper than whether the aggregate has serialize/deserialize, but I think we have to teach it that if it's these particular serialize/deserialize functions, it needs to check the array element type. Otherwise we'll get runtime failures if partial aggregation is attempted on array_agg(). This would not matter for any core datatypes, since they all have send/receive functions, but I imagine it's still pretty common for extension types not to. For my money it'd be sufficient to hard-code the test like if ((aggserialfn == F_ARRAY_AGG_SERIALIZE || aggdeserialfn == F_ARRAY_AGG_DESERIALIZE) && !array_element_type_has_send_and_recv(exprType(aggregate input))) Someday we might need more flexible testing, but that can be solved some other time. regards, tom lane