Improve create_unique_path to not be fooled by unrelated clauses that happen

to be syntactically part of a semijoin clause.  For example given
WHERE EXISTS(SELECT ... WHERE upper.var = lower.var AND some-condition)
where some-condition is just a restriction on the lower relation, we can
use unique-ification on lower.var after having applied some-condition within
the scan on lower.

diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 5ac0b6c260f50392dc12b5abcb26fdc7a68930a7..28b2828c2dd34077f5ad5a889c997c4bb9b570df 100644 (file)
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/optimizer/util/pathnode.c,v 1.149 2009/01/01 17:23:44 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/optimizer/util/pathnode.c,v 1.150 2009/02/27 00:06:27 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -765,12 +765,26 @@ create_unique_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
         */
        oldcontext = MemoryContextSwitchTo(root->planner_cxt);
 
-       /*
+       /*----------
         * Look to see whether the semijoin's join quals consist of AND'ed
         * equality operators, with (only) RHS variables on only one side of
         * each one.  If so, we can figure out how to enforce uniqueness for
         * the RHS.
         *
+        * Note that the input join_quals list is the list of quals that are
+        * *syntactically* associated with the semijoin, which in practice means
+        * the synthesized comparison list for an IN or the WHERE of an EXISTS.
+        * Particularly in the latter case, it might contain clauses that aren't
+        * *semantically* associated with the join, but refer to just one side or
+        * the other.  We can ignore such clauses here, as they will just drop
+        * down to be processed within one side or the other.  (It is okay to
+        * consider only the syntactically-associated clauses here because for a
+        * semijoin, no higher-level quals could refer to the RHS, and so there
+        * can be no other quals that are semantically associated with this join.
+        * We do things this way because it is useful to be able to run this test
+        * before we have extracted the list of quals that are actually
+        * semantically associated with the particular join.)
+        *
         * Note that the in_operators list consists of the joinqual operators
         * themselves (but commuted if needed to put the RHS value on the right).
         * These could be cross-type operators, in which case the operator
@@ -778,6 +792,7 @@ create_unique_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
         * We assume here that that operator will be available from the btree
         * or hash opclass when the time comes ... if not, create_unique_plan()
         * will fail.
+        *----------
         */
        in_operators = NIL;
        uniq_exprs = NIL;
@@ -791,19 +806,52 @@ create_unique_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
                Node       *right_expr;
                Relids          left_varnos;
                Relids          right_varnos;
+               Relids          all_varnos;
 
-               /* must be binary opclause... */
-               if (!IsA(op, OpExpr))
-                       goto no_unique_path;
-               if (list_length(op->args) != 2)
+               /* Is it a binary opclause? */
+               if (!IsA(op, OpExpr) ||
+                       list_length(op->args) != 2)
+               {
+                       /* No, but does it reference both sides? */
+                       all_varnos = pull_varnos((Node *) op);
+                       if (!bms_overlap(all_varnos, sjinfo->syn_righthand) ||
+                               bms_is_subset(all_varnos, sjinfo->syn_righthand))
+                       {
+                               /*
+                                * Clause refers to only one rel, so ignore it --- unless it
+                                * contains volatile functions, in which case we'd better
+                                * punt.
+                                */
+                               if (contain_volatile_functions((Node *) op))
+                                       goto no_unique_path;
+                               continue;
+                       }
+                       /* Non-operator clause referencing both sides, must punt */
                        goto no_unique_path;
+               }
+
+               /* Extract data from binary opclause */
                opno = op->opno;
                left_expr = linitial(op->args);
                right_expr = lsecond(op->args);
-
-               /* check rel membership of arguments */
                left_varnos = pull_varnos(left_expr);
                right_varnos = pull_varnos(right_expr);
+               all_varnos = bms_union(left_varnos, right_varnos);
+
+               /* Does it reference both sides? */
+               if (!bms_overlap(all_varnos, sjinfo->syn_righthand) ||
+                       bms_is_subset(all_varnos, sjinfo->syn_righthand))
+               {
+                       /*
+                        * Clause refers to only one rel, so ignore it --- unless it
+                        * contains volatile functions, in which case we'd better punt.
+                        */
+                       if (contain_volatile_functions((Node *) op))
+                               goto no_unique_path;
+                       continue;
+               }
+
+               /* check rel membership of arguments */
                if (!bms_is_empty(right_varnos) &&
                        bms_is_subset(right_varnos, sjinfo->syn_righthand) &&
                        !bms_overlap(left_varnos, sjinfo->syn_righthand))
@@ -845,6 +893,10 @@ create_unique_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath,
                uniq_exprs = lappend(uniq_exprs, copyObject(right_expr));
        }
 
+       /* Punt if we didn't find at least one column to unique-ify */
+       if (uniq_exprs == NIL)
+               goto no_unique_path;
+
        /*
         * The expressions we'd need to unique-ify mustn't be volatile.
         */