Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit cd1f0d0

Browse files
committed
Fix thinko in previous patch for optimizing EXISTS-within-EXISTS.
When recursing after an optimization in pull_up_sublinks_qual_recurse, the available_rels value passed down must include only the relations that are in the righthand side of the new SEMI or ANTI join; it's incorrect to pull up a sub-select that refers to other relations, as seen in the added test case. Per report from BangarRaju Vadapalli. While at it, rethink the idea of recursing below a NOT EXISTS. That is essentially the same situation as pulling up ANY/EXISTS sub-selects that are in the ON clause of an outer join, and it has the same disadvantage: we'd force the two joins to be evaluated according to the syntactic nesting order, because the lower join will most likely not be able to commute with the ANTI join. That could result in having to form a rather large join product, whereas the handling of a correlated subselect is not quite that dumb. So until we can handle those cases better, #ifdef NOT_USED that case. (I think it's okay to pull up in the EXISTS/ANY cases, because SEMI joins aren't so inflexible about ordering.) Back-patch to 8.4, same as for previous patch in this area. Fortunately that patch hadn't made it into any shipped releases yet.
1 parent a40a5d9 commit cd1f0d0

File tree

3 files changed

+37
-10
lines changed

3 files changed

+37
-10
lines changed

src/backend/optimizer/prep/prepjointree.c

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ pull_up_sublinks_jointree_recurse(PlannerInfo *root, Node *jtnode,
246246
* as a sublink that is executed only for row pairs that meet the
247247
* other join conditions. Fixing this seems to require considerable
248248
* restructuring of the executor, but maybe someday it can happen.
249+
* (See also the comparable case in pull_up_sublinks_qual_recurse.)
249250
*
250251
* We don't expect to see any pre-existing JOIN_SEMI or JOIN_ANTI
251252
* nodes here.
@@ -331,9 +332,7 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
331332
j->rarg = pull_up_sublinks_jointree_recurse(root,
332333
j->rarg,
333334
&child_rels);
334-
/* Pulled-up ANY/EXISTS quals can use those rels too */
335-
child_rels = bms_add_members(child_rels, available_rels);
336-
/* ... and any inserted joins get stacked onto j->rarg */
335+
/* Any inserted joins get stacked onto j->rarg */
337336
j->quals = pull_up_sublinks_qual_recurse(root,
338337
j->quals,
339338
child_rels,
@@ -355,9 +354,7 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
355354
j->rarg = pull_up_sublinks_jointree_recurse(root,
356355
j->rarg,
357356
&child_rels);
358-
/* Pulled-up ANY/EXISTS quals can use those rels too */
359-
child_rels = bms_add_members(child_rels, available_rels);
360-
/* ... and any inserted joins get stacked onto j->rarg */
357+
/* Any inserted joins get stacked onto j->rarg */
361358
j->quals = pull_up_sublinks_qual_recurse(root,
362359
j->quals,
363360
child_rels,
@@ -377,7 +374,6 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
377374
/* If the immediate argument of NOT is EXISTS, try to convert */
378375
SubLink *sublink = (SubLink *) get_notclausearg((Expr *) node);
379376
JoinExpr *j;
380-
Relids child_rels;
381377

382378
if (sublink && IsA(sublink, SubLink))
383379
{
@@ -387,17 +383,27 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
387383
available_rels);
388384
if (j)
389385
{
386+
/*
387+
* For the moment, refrain from recursing underneath NOT.
388+
* As in pull_up_sublinks_jointree_recurse, recursing here
389+
* would result in inserting a join underneath an ANTI
390+
* join with which it could not commute, and that could
391+
* easily lead to a worse plan than what we've
392+
* historically generated.
393+
*/
394+
#ifdef NOT_USED
390395
/* Yes; recursively process what we pulled up */
396+
Relids child_rels;
397+
391398
j->rarg = pull_up_sublinks_jointree_recurse(root,
392399
j->rarg,
393400
&child_rels);
394-
/* Pulled-up ANY/EXISTS quals can use those rels too */
395-
child_rels = bms_add_members(child_rels, available_rels);
396-
/* ... and any inserted joins get stacked onto j->rarg */
401+
/* Any inserted joins get stacked onto j->rarg */
397402
j->quals = pull_up_sublinks_qual_recurse(root,
398403
j->quals,
399404
child_rels,
400405
&j->rarg);
406+
#endif
401407
/* Now insert the new join node into the join tree */
402408
j->larg = *jtlink;
403409
*jtlink = (Node *) j;

src/test/regress/expected/subselect.out

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,3 +530,15 @@ select '1'::text in (select '1'::name union all select '1'::name);
530530
t
531531
(1 row)
532532

533+
--
534+
-- Test case for planner bug with nested EXISTS handling
535+
--
536+
select a.thousand from tenk1 a, tenk1 b
537+
where a.thousand = b.thousand
538+
and exists ( select 1 from tenk1 c where b.hundred = c.hundred
539+
and not exists ( select 1 from tenk1 d
540+
where a.thousand = d.thousand ) );
541+
thousand
542+
----------
543+
(0 rows)
544+

src/test/regress/sql/subselect.sql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,3 +341,12 @@ from
341341
--
342342

343343
select '1'::text in (select '1'::name union all select '1'::name);
344+
345+
--
346+
-- Test case for planner bug with nested EXISTS handling
347+
--
348+
select a.thousand from tenk1 a, tenk1 b
349+
where a.thousand = b.thousand
350+
and exists ( select 1 from tenk1 c where b.hundred = c.hundred
351+
and not exists ( select 1 from tenk1 d
352+
where a.thousand = d.thousand ) );

0 commit comments

Comments
 (0)