Oddity with parallel safety test for scan/join target in grouping_planner

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Oddity with parallel safety test for scan/join target in grouping_planner
Date: 2019-02-26 12:25:34
Message-ID: 5C75303E.8020303@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi hackers,

While working on [1], I noticed $Subject, that is:

/*
* If we have grouping or aggregation to do, the topmost scan/join
* plan node must emit what the grouping step wants; otherwise, it
* should emit grouping_target.
*/
have_grouping = (parse->groupClause || parse->groupingSets ||
parse->hasAggs || root->hasHavingQual);
if (have_grouping)
{
scanjoin_target = make_group_input_target(root, final_target);
--> scanjoin_target_parallel_safe =
is_parallel_safe(root, (Node *) grouping_target->exprs);
}
else
{
scanjoin_target = grouping_target;
scanjoin_target_parallel_safe = grouping_target_parallel_safe;
}

The parallel safety of the final scan/join target is determined from the
grouping target, not that target, which would cause to generate inferior
parallel plans as shown below:

pgbench=# explain verbose select aid+bid, sum(abalance), random() from
pgbench_accounts group by aid+bid;
QUERY PLAN
------------------------------------------------------------------------------------------------------------
GroupAggregate (cost=137403.01..159903.01 rows=1000000 width=20)
Output: ((aid + bid)), sum(abalance), random()
Group Key: ((pgbench_accounts.aid + pgbench_accounts.bid))
-> Sort (cost=137403.01..139903.01 rows=1000000 width=8)
Output: ((aid + bid)), abalance
Sort Key: ((pgbench_accounts.aid + pgbench_accounts.bid))
-> Gather (cost=10.00..24070.67 rows=1000000 width=8)
Output: (aid + bid), abalance
Workers Planned: 2
-> Parallel Seq Scan on public.pgbench_accounts
(cost=0.00..20560.67 rows=416667 width=12)
Output: aid, bid, abalance
(11 rows)

The final scan/join target {(aid + bid), abalance} is definitely
parallel safe, but the target evaluation isn't parallelized across
workers, which is not good. Attached is a patch for fixing this.

Best regards,
Etsuro Fujita

[1] https://commitfest.postgresql.org/22/1950/

Attachment Content-Type Size
target_parallel_safe.patch text/x-patch 591 bytes

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Eugen Konkov 2019-02-26 12:35:39 Re: BUG #15646: Inconsistent behavior for current_setting/set_config
Previous Message Fabien COELHO 2019-02-26 12:19:50 Re: pgbench MAX_ARGS