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

Commitcd1f0d0

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 parenta40a5d9 commitcd1f0d0

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-
Relidschild_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+
#ifdefNOT_USED
390395
/* Yes; recursively process what we pulled up */
396+
Relidschild_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'::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