From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
Cc: | Stephen Frost <sfrost(at)snowman(dot)net>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Parallel Aggregates for string_agg and array_agg |
Date: | 2018-03-26 23:49:24 |
Message-ID: | 25812.1522108164@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2018-03-26 23:49:48 | Re: Proposal: http2 wire format |
Previous Message | Craig Ringer | 2018-03-26 23:40:08 | Re: Proposal: http2 wire format |