Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit5f129cf

Browse files
committed
Fix thinko in previous patch for optimizing EXISTS-within-EXISTS.
When recursing after an optimization in pull_up_sublinks_qual_recurse, theavailable_rels value passed down must include only the relations that arein the righthand side of the new SEMI or ANTI join; it's incorrect to pullup a sub-select that refers to other relations, as seen in the added testcase. Per report from BangarRaju Vadapalli.While at it, rethink the idea of recursing below a NOT EXISTS. That isessentially the same situation as pulling up ANY/EXISTS sub-selects thatare 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 nestingorder, because the lower join will most likely not be able to commute withthe ANTI join. That could result in having to form a rather large joinproduct, whereas the handling of a correlated subselect is not quite thatdumb. So until we can handle those cases better, #ifdef NOT_USED thatcase. (I think it's okay to pull up in the EXISTS/ANY cases, because SEMIjoins aren't so inflexible about ordering.)Back-patch to 8.4, same as for previous patch in this area. Fortunatelythat patch hadn't made it into any shipped releases yet.
1 parentb8e2877 commit5f129cf

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
@@ -245,6 +245,7 @@ pull_up_sublinks_jointree_recurse(PlannerInfo *root, Node *jtnode,
245245
* as a sublink that is executed only for row pairs that meet the
246246
* other join conditions. Fixing this seems to require considerable
247247
* restructuring of the executor, but maybe someday it can happen.
248+
* (See also the comparable case in pull_up_sublinks_qual_recurse.)
248249
*
249250
* We don't expect to see any pre-existing JOIN_SEMI or JOIN_ANTI
250251
* nodes here.
@@ -330,9 +331,7 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
330331
j->rarg=pull_up_sublinks_jointree_recurse(root,
331332
j->rarg,
332333
&child_rels);
333-
/* Pulled-up ANY/EXISTS quals can use those rels too */
334-
child_rels=bms_add_members(child_rels,available_rels);
335-
/* ... and any inserted joins get stacked onto j->rarg */
334+
/* Any inserted joins get stacked onto j->rarg */
336335
j->quals=pull_up_sublinks_qual_recurse(root,
337336
j->quals,
338337
child_rels,
@@ -354,9 +353,7 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
354353
j->rarg=pull_up_sublinks_jointree_recurse(root,
355354
j->rarg,
356355
&child_rels);
357-
/* Pulled-up ANY/EXISTS quals can use those rels too */
358-
child_rels=bms_add_members(child_rels,available_rels);
359-
/* ... and any inserted joins get stacked onto j->rarg */
356+
/* Any inserted joins get stacked onto j->rarg */
360357
j->quals=pull_up_sublinks_qual_recurse(root,
361358
j->quals,
362359
child_rels,
@@ -376,7 +373,6 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
376373
/* If the immediate argument of NOT is EXISTS, try to convert */
377374
SubLink*sublink= (SubLink*)get_notclausearg((Expr*)node);
378375
JoinExpr*j;
379-
Relidschild_rels;
380376

381377
if (sublink&&IsA(sublink,SubLink))
382378
{
@@ -386,17 +382,27 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
386382
available_rels);
387383
if (j)
388384
{
385+
/*
386+
* For the moment, refrain from recursing underneath NOT.
387+
* As in pull_up_sublinks_jointree_recurse, recursing here
388+
* would result in inserting a join underneath an ANTI
389+
* join with which it could not commute, and that could
390+
* easily lead to a worse plan than what we've
391+
* historically generated.
392+
*/
393+
#ifdefNOT_USED
389394
/* Yes; recursively process what we pulled up */
395+
Relidschild_rels;
396+
390397
j->rarg=pull_up_sublinks_jointree_recurse(root,
391398
j->rarg,
392399
&child_rels);
393-
/* Pulled-up ANY/EXISTS quals can use those rels too */
394-
child_rels=bms_add_members(child_rels,available_rels);
395-
/* ... and any inserted joins get stacked onto j->rarg */
400+
/* Any inserted joins get stacked onto j->rarg */
396401
j->quals=pull_up_sublinks_qual_recurse(root,
397402
j->quals,
398403
child_rels,
399404
&j->rarg);
405+
#endif
400406
/* Now insert the new join node into the join tree */
401407
j->larg=*jtlink;
402408
*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'::textin (select'1'::nameunion allselect'1'::name);
344+
345+
--
346+
-- Test case for planner bug with nested EXISTS handling
347+
--
348+
selecta.thousandfrom tenk1 a, tenk1 b
349+
wherea.thousand=b.thousand
350+
and exists (select1from tenk1 cwhereb.hundred=c.hundred
351+
and not exists (select1from tenk1 d
352+
wherea.thousand=d.thousand ) );

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp