From: | James Coleman <jtc331(at)gmail(dot)com> |
---|---|
To: | Greg Nancarrow <gregn4422(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, brian(at)brianlikespostgres(dot)com |
Subject: | Re: Consider parallel for lateral subqueries with limit |
Date: | 2021-11-03 13:49:20 |
Message-ID: | CAAaqYe_gPW9jFsriDRcrejv=xHT50i32bcWJLji=xk5A+VPtVw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Jul 16, 2021 at 3:16 PM James Coleman <jtc331(at)gmail(dot)com> wrote:
>
> On Thu, May 27, 2021 at 9:01 PM Greg Nancarrow <gregn4422(at)gmail(dot)com> wrote:
> >
> > On Tue, Dec 8, 2020 at 10:46 AM James Coleman <jtc331(at)gmail(dot)com> wrote:
> > >
> > > While I haven't actually tracked down to guarantee this is handled
> > > elsewhere, a thought experiment -- I think -- shows it must be so.
> > > Here's why: suppose we don't have a limit here, but the query return
> > > order is different in different backends. Then we would have the same
> > > problem you bring up. In that case this code is already setting
> > > consider_parallel=true on the rel. So I don't think we're changing any
> > > behavior here.
> > >
> >
> > AFAICS, the patch seems very reasonable and specifically targets
> > lateral subqueries with limit/offset. It seems like the uncorrelated
> > case is the only real concern.
> > I generally agree that the current patch is probably not changing any
> > behavior in the uncorrelated case (and like yourself, haven't yet
> > found a case for which it breaks), but I'm not sure Brian's concerns
> > can be ruled out entirely.
> >
> > How about a minor update to the patch to make it slightly more
> > restrictive, to exclude the case when there are no lateral
> > cross-references, so we'll be allowing parallelism only when we know
> > the lateral subquery will be evaluated anew for each source row?
> > I was thinking of the following patch modification:
> >
> > BEFORE:
> > - if (limit_needed(subquery))
> > + if (!rte->lateral && limit_needed(subquery))
> >
> > AFTER:
> > - if (limit_needed(subquery))
> > + if ((!rte->lateral || bms_is_empty(rel->lateral_relids)) &&
> > + limit_needed(subquery))
> >
> >
> > Thoughts?
>
> Apologies for the delayed response; this seems fine to me. I've
> attached patch v2.
Greg,
Do you believe this is now ready for committer?
Thanks,
James Coleman
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2021-11-03 14:02:20 | Re: Predefined role pg_maintenance for VACUUM, ANALYZE, CHECKPOINT. |
Previous Message | James Coleman | 2021-11-03 13:46:33 | Re: Parallelize correlated subqueries that execute within each worker |