On Fri, 31 Dec 2021 at 00:14, Yura Sokolov <y.sokolov@postgrespro.ru> wrote:
> Suggested quick (and valid) fix in the patch attached:
> - If Append has single child, then copy its parallel awareness.
I've been looking at this and I've gone through changing my mind about
what's the right fix quite a number of times.
My current thoughts are that I don't really like the fact that we can
have plans in the following shape:
Finalize Aggregate
-> Gather
Workers Planned: 1
-> Partial Aggregate
-> Parallel Hash Left Join
Hash Cond: (gather_append_1.fk = gather_append_2.fk)
-> Index Scan using gather_append_1_ix on gather_append_1
Index Cond: (f = true)
-> Parallel Hash
-> Parallel Seq Scan on gather_append_2
It's only made safe by the fact that Gather will only use 1 worker.
To me, it just seems too fragile to assume that's always going to be
the case. I feel like this fix just relies on the fact that
create_gather_path() and create_gather_merge_path() do
"pathnode->num_workers = subpath->parallel_workers;". If someone
decided that was to work a different way, then we risk this breaking
again. Additionally, today we have Gather and GatherMerge, but we may
one day end up with more node types that gather results from parallel
workers, or even a completely different way of executing plans.
I think a safer way to fix this is to just not remove the
Append/MergeAppend node if the parallel_aware flag of the only-child
and the Append/MergeAppend don't match. I've done that in the
attached.
I believe the code at the end of add_paths_to_append_rel() can remain as is.
David