Parallel Aggregate costs don't consider combine/serial/deserial funcs

From: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Parallel Aggregate costs don't consider combine/serial/deserial funcs
Date: 2016-04-10 12:47:29
Message-ID: CAKJS1f-qkKdqonV1PsA0f2jMX9bUa51xVXiTNE10pY5D6zD+5g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

I realised a few days ago that the parallel aggregate code does not
cost for the combine, serialisation and deserialisation functions at
all.

I've attached a patch which fixes this.

One small point which I was a little unsure of in the attached is,
should the "if (aggref->aggdirectargs)" part of
count_agg_clauses_walker() be within the "if
(!context->combineStates)". I simply couldn't decide. We currently
have no aggregates which this affects anyway, per; select * from
pg_aggregate where aggcombinefn <> 0 and aggkind <> 'n'; so for now
I've left it outwith.

Another thing I thought of is that it's not too nice that I have to
pass 3 bools to count_agg_clauses() in order to tell it what to do. I
was tempted to invent some bitmask flags for this, then modify
create_agg_path() to use the same flags, but I thought I'd better not
cause too much churn with this patch.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment Content-Type Size
parallel_aggs_costs_fix.patch application/octet-stream 8.6 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2016-04-10 14:58:28 Re: VS 2015 support in src/tools/msvc
Previous Message Amit Kapila 2016-04-10 12:45:55 Re: Move PinBuffer and UnpinBuffer to atomics