Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-02-08 Thread Robert Haas
On Tue, Feb 8, 2022 at 4:11 PM David Rowley wrote: > 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

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-02-08 Thread Zhihong Yu
On Tue, Feb 8, 2022 at 1:11 PM David Rowley wrote: > Thanks for having a look at this. > > On Fri, 4 Feb 2022 at 13:48, Robert Haas 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

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-02-08 Thread David Rowley
Thanks for having a look at this. On Fri, 4 Feb 2022 at 13:48, Robert Haas 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

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-02-03 Thread Robert Haas
On Thu, Feb 3, 2022 at 7:08 PM David Rowley wrote: > Currently, the patch validates 3 rules: > > 1) Ensure a parallel_aware path has only parallel_aware or > parallel_safe subpaths. I think that every path that is parallel_aware must also be parallel_safe. So checking for either parallel_aware

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-02-03 Thread David Rowley
On Wed, 26 Jan 2022 at 05:32, Tom Lane 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

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-01-25 Thread David Rowley
On Wed, 26 Jan 2022 at 05:32, Tom Lane 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

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-01-25 Thread Tom Lane
Yura Sokolov writes: > В Вт, 25/01/2022 в 21:20 +1300, David Rowley пишет: >> The reason I didn't think it was worth adding a new test was that no >> tests were added in the original commit. Existing tests did cover it, > Existed tests didn't catched the issue. It is pitty fix is merged >

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-01-25 Thread Yura Sokolov
В Вт, 25/01/2022 в 21:20 +1300, David Rowley пишет: > On Tue, 25 Jan 2022 at 20:03, David Rowley wrote: > > On Tue, 25 Jan 2022 at 17:35, Yura Sokolov wrote: > > > And another attempt to fix tests volatility. > > > > FWIW, I had not really seen the point in adding a test for this. I > > did

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-01-25 Thread David Rowley
On Tue, 25 Jan 2022 at 20:03, David Rowley wrote: > > On Tue, 25 Jan 2022 at 17:35, Yura Sokolov wrote: > > And another attempt to fix tests volatility. > > FWIW, I had not really seen the point in adding a test for this. I > did however see a point in it with your original patch. It seemed >

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-01-24 Thread David Rowley
On Tue, 25 Jan 2022 at 17:35, Yura Sokolov wrote: > And another attempt to fix tests volatility. FWIW, I had not really seen the point in adding a test for this. I did however see a point in it with your original patch. It seemed useful there to verify that Gather and GatherMerge did what we

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-01-24 Thread Yura Sokolov
В Пн, 24/01/2022 в 16:24 +0300, Yura Sokolov пишет: > В Вс, 23/01/2022 в 14:56 +0300, Yura Sokolov пишет: > > В Чт, 20/01/2022 в 09:32 +1300, David Rowley пишет: > > > On Fri, 31 Dec 2021 at 00:14, Yura Sokolov > > > wrote: > > > > Suggested quick (and valid) fix in the patch attached: > > > > -

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-01-24 Thread Yura Sokolov
В Вс, 23/01/2022 в 14:56 +0300, Yura Sokolov пишет: > В Чт, 20/01/2022 в 09:32 +1300, David Rowley пишет: > > On Fri, 31 Dec 2021 at 00:14, Yura Sokolov wrote: > > > Suggested quick (and valid) fix in the patch attached: > > > - If Append has single child, then copy its parallel awareness. > > >

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-01-23 Thread Yura Sokolov
В Чт, 20/01/2022 в 09:32 +1300, David Rowley пишет: > On Fri, 31 Dec 2021 at 00:14, Yura Sokolov 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

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-01-19 Thread David Rowley
On Fri, 31 Dec 2021 at 00:14, Yura Sokolov 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

Re: Fix BUG #17335: Duplicate result rows in Gather node

2022-01-16 Thread Yura Sokolov
В Сб, 01/01/2022 в 15:19 +1300, David Rowley пишет: > On Fri, 31 Dec 2021 at 00:14, Yura Sokolov wrote: > > Problem: > > - Append path is created with explicitely parallel_aware = true > > - It has two child, one is trivial, other is parallel_aware = false . > > Trivial child is dropped. > > -

Re: Fix BUG #17335: Duplicate result rows in Gather node

2021-12-31 Thread David Rowley
On Fri, 31 Dec 2021 at 00:14, Yura Sokolov wrote: > Problem: > - Append path is created with explicitely parallel_aware = true > - It has two child, one is trivial, other is parallel_aware = false . > Trivial child is dropped. > - Gather/GatherMerge path takes Append path as a child and thinks

Re: Fix BUG #17335: Duplicate result rows in Gather node

2021-12-30 Thread Dilip Kumar
On Thu, Dec 30, 2021 at 4:44 PM Yura Sokolov wrote: > Good day, hackers. > > Problem: > - Append path is created with explicitely parallel_aware = true > - It has two child, one is trivial, other is parallel_aware = false . > Trivial child is dropped. > - Gather/GatherMerge path takes Append

Fix BUG #17335: Duplicate result rows in Gather node

2021-12-30 Thread Yura Sokolov
Good day, hackers. Problem: - Append path is created with explicitely parallel_aware = true - It has two child, one is trivial, other is parallel_aware = false . Trivial child is dropped. - Gather/GatherMerge path takes Append path as a child and thinks its child is parallel_aware = true. -