On Thu, May 27, 2021 at 9:01 PM Greg Nancarrow <gregn4...@gmail.com> wrote: > > On Tue, Dec 8, 2020 at 10:46 AM James Coleman <jtc...@gmail.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. Thanks, James Coleman
v2-0001-Allow-parallel-LATERAL-subqueries-with-LIMIT-OFFS.patch
Description: Binary data