Re: Fix BUG #17335: Duplicate result rows in Gather node - Mailing list pgsql-hackers

From David Rowley
Subject Re: Fix BUG #17335: Duplicate result rows in Gather node
Date
Msg-id CAApHDvqAu-20gMaiHfHyOKdZ1279bcFvWPoE9a+RF3hWSYeifQ@mail.gmail.com
Whole thread Raw
In response to Re: Fix BUG #17335: Duplicate result rows in Gather node  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Fix BUG #17335: Duplicate result rows in Gather node  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On Wed, 26 Jan 2022 at 05:32, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Therefore, what I think could be useful is some very-late-stage
> assertion check (probably in createplan.c) verifying that the
> child of a Gather is parallel-aware.  Or maybe the condition
> needs to be more general than that, but anyway the idea is for
> the back end of the planner to verify that we didn't build a
> silly plan.

I had a go at writing something along these lines, but I've ended up
with something I really don't like very much.

I ended up having to write a recursive path traversal function.  It's
generic and it can be given a callback function to do whatever we like
with the Path.  The problem is, that this seems like quite a bit of
code to maintain just for plan validation in Assert builds.

Currently, the patch validates 3 rules:

1) Ensure a parallel_aware path has only parallel_aware or
parallel_safe subpaths.
2) Ensure Gather is either single_copy or contains at least one
parallel_aware subnode.
3) Ensure GatherMerge contains at least one parallel_aware subnode.

I had to relax rule #1 a little as a Parallel Append can run subnodes
that are only parallel_safe and not parallel_aware.  The problem with
relaxing this rule is that it does not catch the case that this bug
report was about. I could maybe tweak that so there's a special case
for Append to allow parallel aware or safe and ensure all other nodes
have only parallel_safe subnodes. I just don't really like that
special case as it's likely to get broken/forgotten over time when we
add new nodes.

I'm unsure if just being able to enforce rules #2 and #3 make this worthwhile.

Happy to listen to other people's opinions and ideas on this.  Without
those, I'm unlikely to try to push this any further.

David

Attachment

pgsql-hackers by date:

Previous
From: Nathan Bossart
Date:
Subject: Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work
Next
From: John Naylor
Date:
Subject: Re: Stats collector's idx_blks_hit value is highly misleading in practice