Re: Parallelize correlated subqueries that execute within each worker

2024-01-31 Thread James Coleman
On Wed, Jan 31, 2024 at 3:18 PM Robert Haas wrote: > > On Tue, Jan 30, 2024 at 9:56 PM James Coleman wrote: > > I don't follow the "Idle since July" since it just hasn't received > > review since then, so there's been nothing to reply to. > > It wasn't clear to me if you thought that the patch

Re: Parallelize correlated subqueries that execute within each worker

2024-01-31 Thread Robert Haas
On Tue, Jan 30, 2024 at 9:56 PM James Coleman wrote: > I don't follow the "Idle since July" since it just hasn't received > review since then, so there's been nothing to reply to. It wasn't clear to me if you thought that the patch was ready for review since July, or if it was waiting on you

Re: Parallelize correlated subqueries that execute within each worker

2024-01-31 Thread James Coleman
On Tue, Jan 30, 2024 at 10:34 PM Tom Lane wrote: > > James Coleman writes: > > I've finally had a chance to look at this, and I don't believe there's > > any real failure here, merely drift of how the planner works on master > > resulting in this query now being eligible for a different plan

Re: Parallelize correlated subqueries that execute within each worker

2024-01-30 Thread Tom Lane
James Coleman writes: > I've finally had a chance to look at this, and I don't believe there's > any real failure here, merely drift of how the planner works on master > resulting in this query now being eligible for a different plan shape. > I was a bit wary at first because the changing test

Re: Parallelize correlated subqueries that execute within each worker

2024-01-30 Thread James Coleman
On Tue, Jan 30, 2024 at 11:54 AM Robert Haas wrote: > > On Tue, Jan 30, 2024 at 11:17 AM Akshat Jaimini wrote: > > I think we should move this patch to the next CF as I believe that work is > > still going on resolving the last reported bug. > > We shouldn't just keep pushing this forward to

Re: Parallelize correlated subqueries that execute within each worker

2024-01-30 Thread James Coleman
On Tue, Jan 9, 2024 at 2:09 AM vignesh C wrote: > ... > > Given all of that I settled on this approach: > > 1. Modify is_parallel_safe() to by default ignore PARAM_EXEC params. > > 2. Add is_parallel_safe_with_params() that checks for the existence of > > such params. > > 3. Store the required

Re: Parallelize correlated subqueries that execute within each worker

2024-01-30 Thread Robert Haas
On Tue, Jan 30, 2024 at 11:17 AM Akshat Jaimini wrote: > I think we should move this patch to the next CF as I believe that work is > still going on resolving the last reported bug. We shouldn't just keep pushing this forward to the next CF. It's been idle since July. If it needs more work,

Re: Parallelize correlated subqueries that execute within each worker

2024-01-30 Thread Akshat Jaimini
I think we should move this patch to the next CF as I believe that work is still going on resolving the last reported bug.

Re: Parallelize correlated subqueries that execute within each worker

2024-01-23 Thread Robert Haas
On Tue, Jan 23, 2024 at 9:34 AM Akshat Jaimini wrote: > Hello! > I was going through the previous conversations for this particular patch and > it seems that this patch failed some tests previously? > Imo we should move it to the next CF so that the remaining issues can be > resolved

Re: Parallelize correlated subqueries that execute within each worker

2024-01-23 Thread Akshat Jaimini
Hello! I was going through the previous conversations for this particular patch and it seems that this patch failed some tests previously? Imo we should move it to the next CF so that the remaining issues can be resolved accordingly.

Re: Parallelize correlated subqueries that execute within each worker

2024-01-08 Thread vignesh C
On Tue, 4 Jul 2023 at 06:56, James Coleman wrote: > > On Sun, Jun 11, 2023 at 10:23 PM James Coleman wrote: > > > > ... > > > And while trying the v9 patch I came across a crash with the query > > > below. > > > > > > set min_parallel_table_scan_size to 0; > > > set parallel_setup_cost to 0; > >

Re: Parallelize correlated subqueries that execute within each worker

2023-07-03 Thread James Coleman
On Sun, Jun 11, 2023 at 10:23 PM James Coleman wrote: > > ... > > And while trying the v9 patch I came across a crash with the query > > below. > > > > set min_parallel_table_scan_size to 0; > > set parallel_setup_cost to 0; > > set parallel_tuple_cost to 0; > > > > explain (costs off) > > select

Re: Parallelize correlated subqueries that execute within each worker

2023-06-26 Thread Richard Guo
On Mon, Jun 12, 2023 at 10:23 AM James Coleman wrote: > BTW are you by any chance testing on ARM macOS? I reproduced the issue > there, but for some reason I did not reproduce the error (and the plan > wasn't parallelized) when I tested this on linux. Perhaps I missed > setting something up; it

Re: Parallelize correlated subqueries that execute within each worker

2023-06-11 Thread James Coleman
On Tue, Jun 6, 2023 at 4:36 AM Richard Guo wrote: > > > On Mon, Jan 23, 2023 at 10:00 PM James Coleman wrote: >> >> Which this patch we do in fact now see (as expected) rels with >> non-empty lateral_relids showing up in generate_[useful_]gather_paths. >> And the partial paths can now have

Re: Parallelize correlated subqueries that execute within each worker

2023-06-06 Thread Richard Guo
On Mon, Jan 23, 2023 at 10:00 PM James Coleman wrote: > Which this patch we do in fact now see (as expected) rels with > non-empty lateral_relids showing up in generate_[useful_]gather_paths. > And the partial paths can now have non-empty required outer rels. I'm > not able to come up with a

Re: Parallelize correlated subqueries that execute within each worker

2023-03-08 Thread Antonin Houska
James Coleman wrote: > On Mon, Feb 6, 2023 at 11:39 AM Antonin Houska wrote: > Attached is v9. ok, I've changed the status to RfC -- Antonin Houska Web: https://www.cybertec-postgresql.com

Re: Parallelize correlated subqueries that execute within each worker

2023-02-08 Thread James Coleman
On Mon, Feb 6, 2023 at 11:39 AM Antonin Houska wrote: > > James Coleman wrote: > > Which this patch we do in fact now see (as expected) rels with > > non-empty lateral_relids showing up in generate_[useful_]gather_paths. > > And the partial paths can now have non-empty required outer rels. I'm >

Re: Parallelize correlated subqueries that execute within each worker

2023-02-06 Thread Antonin Houska
James Coleman wrote: > On Sat, Jan 21, 2023 at 10:07 PM James Coleman wrote: > > ... > > While working through Tomas's comment about a conditional in the > > max_parallel_hazard_waker being guaranteed true I realized that in the > > current version of the patch the safe_param_ids tracking in >

Re: Parallelize correlated subqueries that execute within each worker

2023-01-23 Thread James Coleman
On Sat, Jan 21, 2023 at 10:07 PM James Coleman wrote: > ... > While working through Tomas's comment about a conditional in the > max_parallel_hazard_waker being guaranteed true I realized that in the > current version of the patch the safe_param_ids tracking in > is_parallel_safe isn't actually

Re: Parallelize correlated subqueries that execute within each worker

2023-01-21 Thread James Coleman
On Wed, Jan 18, 2023 at 9:34 PM James Coleman wrote: > > On Wed, Jan 18, 2023 at 2:09 PM Tomas Vondra > wrote: > > > > Hi, > > > > This patch hasn't been updated since September, and it got broken by > > 4a29eabd1d91c5484426bc5836e0a7143b064f5a which the incremental sort > > stuff a little bit.

Re: Parallelize correlated subqueries that execute within each worker

2023-01-18 Thread James Coleman
On Wed, Jan 18, 2023 at 2:09 PM Tomas Vondra wrote: > > Hi, > > This patch hasn't been updated since September, and it got broken by > 4a29eabd1d91c5484426bc5836e0a7143b064f5a which the incremental sort > stuff a little bit. But the breakage was rather limited, so I took a > stab at fixing it -

Re: Parallelize correlated subqueries that execute within each worker

2023-01-18 Thread Tomas Vondra
Hi, This patch hasn't been updated since September, and it got broken by 4a29eabd1d91c5484426bc5836e0a7143b064f5a which the incremental sort stuff a little bit. But the breakage was rather limited, so I took a stab at fixing it - attached is the result, hopefully correct. I also added a couple

Re: Parallelize correlated subqueries that execute within each worker

2023-01-04 Thread vignesh C
On Tue, 27 Sept 2022 at 08:26, James Coleman wrote: > > On Mon, Mar 21, 2022 at 8:48 PM Andres Freund wrote: > > > > Hi, > > > > On 2022-01-22 20:25:19 -0500, James Coleman wrote: > > > On the other hand this is a dramatically simpler patch series. > > > Assuming the approach is sound, it should

Re: Parallelize correlated subqueries that execute within each worker

2022-09-26 Thread James Coleman
On Mon, Mar 21, 2022 at 8:48 PM Andres Freund wrote: > > Hi, > > On 2022-01-22 20:25:19 -0500, James Coleman wrote: > > On the other hand this is a dramatically simpler patch series. > > Assuming the approach is sound, it should much easier to maintain than > > the previous version. > > > > The

Re: Parallelize correlated subqueries that execute within each worker

2022-08-02 Thread Jacob Champion
This entry has been waiting on author input for a while (our current threshold is roughly two weeks), so I've marked it Returned with Feedback. Once you think the patchset is ready for review again, you (or any interested party) can resurrect the patch entry by visiting

Re: Parallelize correlated subqueries that execute within each worker

2022-03-21 Thread Andres Freund
Hi, On 2022-01-22 20:25:19 -0500, James Coleman wrote: > On the other hand this is a dramatically simpler patch series. > Assuming the approach is sound, it should much easier to maintain than > the previous version. > > The final patch in the series is a set of additional checks I could >

Re: Parallelize correlated subqueries that execute within each worker

2022-01-22 Thread James Coleman
On Fri, Jan 21, 2022 at 3:20 PM Robert Haas wrote: > > On Fri, Jan 14, 2022 at 2:25 PM James Coleman wrote: > > I've been chewing on this a bit, and I was about to go re-read the > > code and see how easy it'd be to do exactly what you're suggesting in > > generate_gather_paths() (and verifying

Re: Parallelize correlated subqueries that execute within each worker

2022-01-21 Thread Tom Lane
Robert Haas writes: > I don't think there's an intrinsic problem with the idea of making a > tentative determination about parallel safety and then refining it > later, but I'm not sure why you think it would be a lot of work to > figure this out at the point where we generate gather paths. I

Re: Parallelize correlated subqueries that execute within each worker

2022-01-21 Thread Robert Haas
On Fri, Jan 14, 2022 at 2:25 PM James Coleman wrote: > I've been chewing on this a bit, and I was about to go re-read the > code and see how easy it'd be to do exactly what you're suggesting in > generate_gather_paths() (and verifying it doesn't need to happen in > other places). However there's

Re: Parallelize correlated subqueries that execute within each worker

2022-01-14 Thread James Coleman
On Mon, Nov 15, 2021 at 10:01 AM Robert Haas wrote: > > On Wed, Nov 3, 2021 at 1:34 PM James Coleman wrote: > > As I understand the current code, parallel plans are largely chosen > > based not on where it's safe to insert a Gather node but rather by > > determining if a given path is parallel

Re: Parallelize correlated subqueries that execute within each worker

2022-01-14 Thread James Coleman
On Fri, Dec 3, 2021 at 2:35 AM Michael Paquier wrote: > > On Mon, Nov 15, 2021 at 10:01:37AM -0500, Robert Haas wrote: > > On Wed, Nov 3, 2021 at 1:34 PM James Coleman wrote: > >> As I understand the current code, parallel plans are largely chosen > >> based not on where it's safe to insert a

Re: Parallelize correlated subqueries that execute within each worker

2021-12-02 Thread Michael Paquier
On Mon, Nov 15, 2021 at 10:01:37AM -0500, Robert Haas wrote: > On Wed, Nov 3, 2021 at 1:34 PM James Coleman wrote: >> As I understand the current code, parallel plans are largely chosen >> based not on where it's safe to insert a Gather node but rather by >> determining if a given path is

Re: Parallelize correlated subqueries that execute within each worker

2021-11-15 Thread Robert Haas
On Wed, Nov 3, 2021 at 1:34 PM James Coleman wrote: > As I understand the current code, parallel plans are largely chosen > based not on where it's safe to insert a Gather node but rather by > determining if a given path is parallel safe. Through that lens params > are a bit of an odd man out --

Re: Parallelize correlated subqueries that execute within each worker

2021-11-03 Thread James Coleman
Hi Robert, thanks for the detailed reply. On Wed, Nov 3, 2021 at 10:48 AM Robert Haas wrote: > > As a preliminary comment, it would be quite useful to get Tom Lane's > opinion on this, since it's not an area I understand especially well, > and I think he understands it better than anyone. > > On

Re: Parallelize correlated subqueries that execute within each worker

2021-11-03 Thread Tom Lane
Robert Haas writes: > One thing I discovered when I was looking at this a few years ago is > that there was only one query in the regression tests where extParam > and allParam were not the same. The offending query was select 1 = > all(select (select 1)), and the resulting plan has a Materialize

Re: Parallelize correlated subqueries that execute within each worker

2021-11-03 Thread Robert Haas
On Wed, Nov 3, 2021 at 11:14 AM Tom Lane wrote: > FWIW, I've never been very happy with those fields either. IIRC the > design in that area was all Vadim's, but to the extent that there's > any usable documentation of extParam/allParam, it was filled in by me > while trying to understand what

Re: Parallelize correlated subqueries that execute within each worker

2021-11-03 Thread Tom Lane
Robert Haas writes: > When I last worked on this, I had hoped that extParam or allParam > would be the thing that would answer the question: are there any > parameters used under this node that are not also set under this node? > But I seem to recall that neither seemed to be answering precisely

Re: Parallelize correlated subqueries that execute within each worker

2021-11-03 Thread Robert Haas
As a preliminary comment, it would be quite useful to get Tom Lane's opinion on this, since it's not an area I understand especially well, and I think he understands it better than anyone. On Fri, May 7, 2021 at 12:30 PM James Coleman wrote: > The basic idea is that we need to track (both on

Re: Parallelize correlated subqueries that execute within each worker

2021-11-03 Thread James Coleman
On Wed, Sep 8, 2021 at 8:47 AM James Coleman wrote: > See updated patch series attached. Jaime, I noticed on 3-October you moved this into "waiting on author"; I don't see anything waiting in this thread, however. Am I missing something? I'm planning to change it back to "needs review".

Re: Parallelize correlated subqueries that execute within each worker

2021-09-08 Thread James Coleman
On Tue, Sep 7, 2021 at 11:06 AM Zhihong Yu wrote: > > > > On Tue, Sep 7, 2021 at 6:17 AM James Coleman wrote: >> >> On Wed, Sep 1, 2021 at 7:06 AM Daniel Gustafsson wrote: >> > >> > > On 7 May 2021, at 18:30, James Coleman wrote: >> > >> > > ..here we are now, and I finally have this patch

Re: Parallelize correlated subqueries that execute within each worker

2021-09-07 Thread Zhihong Yu
On Tue, Sep 7, 2021 at 6:17 AM James Coleman wrote: > On Wed, Sep 1, 2021 at 7:06 AM Daniel Gustafsson wrote: > > > > > On 7 May 2021, at 18:30, James Coleman wrote: > > > > > ..here we are now, and I finally have this patch cleaned up > > > enough to share. > > > > This patch no longer

Re: Parallelize correlated subqueries that execute within each worker

2021-09-07 Thread James Coleman
On Wed, Sep 1, 2021 at 7:06 AM Daniel Gustafsson wrote: > > > On 7 May 2021, at 18:30, James Coleman wrote: > > > ..here we are now, and I finally have this patch cleaned up > > enough to share. > > This patch no longer applies to HEAD, can you please submit a rebased version? See attached.

Re: Parallelize correlated subqueries that execute within each worker

2021-09-01 Thread Daniel Gustafsson
> On 7 May 2021, at 18:30, James Coleman wrote: > ..here we are now, and I finally have this patch cleaned up > enough to share. This patch no longer applies to HEAD, can you please submit a rebased version? -- Daniel Gustafsson https://vmware.com/

Parallelize correlated subqueries that execute within each worker

2021-05-07 Thread James Coleman
In a bug report back in November [1] a subthread explored why parallel query is excluded any time we have "Plan nodes which reference a correlated SubPlan". Amit's understanding was that the reasoning had to do with inability to easily pass (potentially variable length) Param values between