Re: Cannot create an aggregate function with variadic parameters andenabled for parallel execution

On 15/05/18 17:52, Tom Lane wrote:
>
> For the combine function, it seems fine to just say that the function
> signature is alway "combine(stype, stype) returns stype" even in
> polymorphic cases.  "combine(anyarray, anyarray) returns anyarray"
> is a perfectly legal function declaration, so there's no problem at
> that end.  At runtime, the combine function could inspect its argument
> to find out what the array element type is.  In cases that are like
> "combine(internal, internal) returns internal", the function declaration
> is still acceptable.  The representation of the internal-type state
> value would need to contain enough information for the combine function
> to do its work without additional type lookups --- but that would likely
> need to be true anyway, so it does not seem like a big penalty.
>
> For the serial/deserial functions, the function signatures are
> prespecified and don't depend on the aggregate argument types, so
> there's no declaration issue.  Again, the state value would need
> to be sufficiently self-contained for the functions to not need
> additional info to figure out how to serialize it --- but again,
> I do not see much problem there.

Sounds reasonable, aggregate arguments type info is not necessary for
combine/de-/serialize declarations, and for execution it could be
passed in the state variable.


> So I think we're okay with solving this as per your patch, though
> it needs some more comments IMO.  Will push.

Do you mean it needs to be discussed more, or it needs more
inline code comments?

>
> BTW, I noticed while looking at this that this error message in
> nodeAgg.c seems to be exactly backwards:
>
>          /*
>           * Ensure that a combine function to combine INTERNAL states is not
>           * strict. This should have been checked during CREATE AGGREGATE, but
>           * the strict property could have been changed since then.
>           */
>          if (pertrans->transfn.fn_strict && aggtranstype == INTERNALOID)
>              ereport(ERROR,
>                      (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
>                       errmsg("combine function for aggregate %u must be declared as STRICT",
>                              aggref->aggfnoid)));
>      }
>
> Should be "must *not* be declared STRICT", no?

Agree, this must have been a typo.