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

From Zhihong Yu
Subject Re: Fix BUG #17335: Duplicate result rows in Gather node
Date
Msg-id CALNJ-vQnosZZSgGxV9sL0ovdFmVyMr2Fs9gNvG9oan4UaXj7kA@mail.gmail.com
Whole thread Raw
In response to Re: Fix BUG #17335: Duplicate result rows in Gather node  (David Rowley <dgrowleyml@gmail.com>)
List pgsql-hackers


On Tue, Feb 8, 2022 at 1:11 PM David Rowley <dgrowleyml@gmail.com> wrote:
Thanks for having a look at this.

On Fri, 4 Feb 2022 at 13:48, Robert Haas <robertmhaas@gmail.com> wrote:
> I think the actual rule is: every path under a Gather or GatherMerge
> must be parallel-safe.

I've adjusted the patch so that it counts parallel_aware and
parallel_safe Paths independently and verifies everything below a
Gather[Merge] is parallel_safe.

The diff stat currently looks like:

src/backend/optimizer/plan/createplan.c | 230
1 file changed, 230 insertions(+)

I still feel this is quite a bit of code for what we're getting here.
I'd be more for it if the path traversal function existed for some
other reason and I was just adding the callback functions and Asserts.

I'm keen to hear what others think about that.

David
Hi,

+           break;
+           case T_MergeAppend:

The case for T_MergeAppend should be left indented.

+       case T_Result:
+           if (IsA(path, ProjectionPath))

Since the remaining sub-cases don't have subpath, they are covered by the final `else` block - MinMaxAggPath and GroupResultPath don't need to be checked.

For contains_a_parallel_aware_path(), it seems path_type_counter() can return bool indicating whether the walker should return early (when parallel aware count reaches 1).

Cheers

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: make MaxBackends available in _PG_init
Next
From: Robert Haas
Date:
Subject: Re: Fix BUG #17335: Duplicate result rows in Gather node