Re: oversight in parallel aggregate

On Mon, Apr 4, 2016 at 10:35 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> One of my EDB colleagues, while in the process of refactoring some
>> unrelated Advanced Server code, discovered that (1) there's no way to
>> mark an aggregate as anything other than parallel-unsafe but (2) it
>> doesn't matter because has_parallel_hazard ignores Aggrefs anyway.
>> These mistakes cancel each other out (sorta) if all of your aggregates
>> happen to be parallel-safe, but otherwise not so much. Barring
>> objections, I intend to speedily apply the attached patch to fix this.
>
> Um ... why is it a good idea to attach a parallel-safe annotation to an
> aggregate as such, rather than relying on the parallel-safe annotations
> of its implementation functions?
>
> This seems not entirely academic, since perhaps the functions are not
> all marked the same; which might be sensible. Perhaps the transition
> function can be pushed down but not the final function.

We could do it that way, and then just ignore the marking of the
aggregate function itself. However, that would require
has_parallel_hazard to do more syscache lookups, since it would have
to examine all of the functions bound into the aggregate instead of
just looking at the aggregate itself. I think that's probably not
worth it, because I struggle to think of why an aggregate function
would be ever be parallel-restricted or parallel-unsafe. I suppose
somebody could create a user-defined aggregate that has side effects,
but that seems like a corner case for which we shouldn't be
optimizing.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company