Fix thinko in previous patch for optimizing EXISTS-within-EXISTS.
authorTom Lane <[email protected]>
Mon, 20 Jun 2011 18:33:20 +0000 (14:33 -0400)
committerTom Lane <[email protected]>
Mon, 20 Jun 2011 18:33:54 +0000 (14:33 -0400)
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.

src/backend/optimizer/prep/prepjointree.c
src/test/regress/expected/subselect.out
src/test/regress/sql/subselect.sql

index 93e9a94778760038b94a2e8eb455016e5d857933..99ccbbe84212cb1d1d4d38d5c9a1918f80b3d424 100644 (file)
@@ -245,6 +245,7 @@ pull_up_sublinks_jointree_recurse(PlannerInfo *root, Node *jtnode,
         * as a sublink that is executed only for row pairs that meet the
         * other join conditions.  Fixing this seems to require considerable
         * restructuring of the executor, but maybe someday it can happen.
+        * (See also the comparable case in pull_up_sublinks_qual_recurse.)
         *
         * We don't expect to see any pre-existing JOIN_SEMI or JOIN_ANTI
         * nodes here.
@@ -330,9 +331,7 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
                j->rarg = pull_up_sublinks_jointree_recurse(root,
                                                            j->rarg,
                                                            &child_rels);
-               /* Pulled-up ANY/EXISTS quals can use those rels too */
-               child_rels = bms_add_members(child_rels, available_rels);
-               /* ... and any inserted joins get stacked onto j->rarg */
+               /* Any inserted joins get stacked onto j->rarg */
                j->quals = pull_up_sublinks_qual_recurse(root,
                                                         j->quals,
                                                         child_rels,
@@ -354,9 +353,7 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
                j->rarg = pull_up_sublinks_jointree_recurse(root,
                                                            j->rarg,
                                                            &child_rels);
-               /* Pulled-up ANY/EXISTS quals can use those rels too */
-               child_rels = bms_add_members(child_rels, available_rels);
-               /* ... and any inserted joins get stacked onto j->rarg */
+               /* Any inserted joins get stacked onto j->rarg */
                j->quals = pull_up_sublinks_qual_recurse(root,
                                                         j->quals,
                                                         child_rels,
@@ -376,7 +373,6 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
        /* If the immediate argument of NOT is EXISTS, try to convert */
        SubLink    *sublink = (SubLink *) get_notclausearg((Expr *) node);
        JoinExpr   *j;
-       Relids      child_rels;
 
        if (sublink && IsA(sublink, SubLink))
        {
@@ -386,17 +382,27 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
                                                   available_rels);
                if (j)
                {
+                   /*
+                    * For the moment, refrain from recursing underneath NOT.
+                    * As in pull_up_sublinks_jointree_recurse, recursing here
+                    * would result in inserting a join underneath an ANTI
+                    * join with which it could not commute, and that could
+                    * easily lead to a worse plan than what we've
+                    * historically generated.
+                    */
+#ifdef NOT_USED
                    /* Yes; recursively process what we pulled up */
+                   Relids      child_rels;
+
                    j->rarg = pull_up_sublinks_jointree_recurse(root,
                                                                j->rarg,
                                                                &child_rels);
-                   /* Pulled-up ANY/EXISTS quals can use those rels too */
-                   child_rels = bms_add_members(child_rels, available_rels);
-                   /* ... and any inserted joins get stacked onto j->rarg */
+                   /* Any inserted joins get stacked onto j->rarg */
                    j->quals = pull_up_sublinks_qual_recurse(root,
                                                             j->quals,
                                                             child_rels,
                                                             &j->rarg);
+#endif
                    /* Now insert the new join node into the join tree */
                    j->larg = *jtlink;
                    *jtlink = (Node *) j;
index 7f8c05bc800c57d60661a9fb21503cde49101347..10432b0c391fb3683008634d9f6c4cfce400f699 100644 (file)
@@ -530,3 +530,15 @@ select '1'::text in (select '1'::name union all select '1'::name);
  t
 (1 row)
 
+--
+-- Test case for planner bug with nested EXISTS handling
+--
+select a.thousand from tenk1 a, tenk1 b
+where a.thousand = b.thousand
+  and exists ( select 1 from tenk1 c where b.hundred = c.hundred
+                   and not exists ( select 1 from tenk1 d
+                                    where a.thousand = d.thousand ) );
+ thousand 
+----------
+(0 rows)
+
index 9d13c39c8ab7f6baedc3bf545786888f4bf2b6e8..c7846b829202a630e1a84daa9804c2afd8676b47 100644 (file)
@@ -341,3 +341,12 @@ from
 --
 
 select '1'::text in (select '1'::name union all select '1'::name);
+
+--
+-- Test case for planner bug with nested EXISTS handling
+--
+select a.thousand from tenk1 a, tenk1 b
+where a.thousand = b.thousand
+  and exists ( select 1 from tenk1 c where b.hundred = c.hundred
+                   and not exists ( select 1 from tenk1 d
+                                    where a.thousand = d.thousand ) );