Re: Propose RC1 for Friday ... - Mailing list pgsql-hackers

I said:
> Well, we could define it as a bug ;-) --- that is, a performance regression.
> I'd be happier about adding a dozen lines of code to sort quals by
> whether or not they contain a subplan than about flip-flopping on the
> original patch.  That would actually solve the class of problem you
> exhibited, whereas the other is just a band-aid that happens to work for
> your particular example.

The attached patch does the above.  I think it's a very low-risk change,
but am tossing it out on the list to see if anyone objects to applying
it in the 7.3 branch.  (I intend to put it in 7.4devel in any case.)
        regards, tom lane


*** src/backend/optimizer/plan/createplan.c.orig    Wed Nov  6 17:31:24 2002
--- src/backend/optimizer/plan/createplan.c    Thu Nov 14 13:18:04 2002
***************
*** 70,75 ****
--- 70,76 ----                      IndexOptInfo *index,                      Oid *opclass); static List
*switch_outer(List*clauses);
 
+ static List *order_qual_clauses(Query *root, List *clauses); static void copy_path_costsize(Plan *dest, Path *src);
staticvoid copy_plan_costsize(Plan *dest, Plan *src); static SeqScan *make_seqscan(List *qptlist, List *qpqual, Index
scanrelid);
***************
*** 182,187 ****
--- 183,191 ----      */     scan_clauses = get_actual_clauses(best_path->parent->baserestrictinfo); 
+     /* Sort clauses into best execution order */
+     scan_clauses = order_qual_clauses(root, scan_clauses);
+      switch (best_path->pathtype)     {         case T_SeqScan:
***************
*** 359,364 ****
--- 363,369 ---- {     Result       *plan;     List       *tlist;
+     List       *constclauses;     Plan       *subplan;      if (best_path->path.parent)
***************
*** 371,377 ****     else         subplan = NULL; 
!     plan = make_result(tlist, (Node *) best_path->constantqual, subplan);      return plan; }
--- 376,384 ----     else         subplan = NULL; 
!     constclauses = order_qual_clauses(root, best_path->constantqual);
! 
!     plan = make_result(tlist, (Node *) constclauses, subplan);      return plan; }
***************
*** 1210,1215 ****
--- 1217,1259 ----             t_list = lappend(t_list, clause);     }     return t_list;
+ }
+ 
+ /*
+  * order_qual_clauses
+  *        Given a list of qual clauses that will all be evaluated at the same
+  *        plan node, sort the list into the order we want to check the quals
+  *        in at runtime.
+  *
+  * Ideally the order should be driven by a combination of execution cost and
+  * selectivity, but unfortunately we have so little information about
+  * execution cost of operators that it's really hard to do anything smart.
+  * For now, we just move any quals that contain SubPlan references (but not
+  * InitPlan references) to the end of the list.
+  */
+ static List *
+ order_qual_clauses(Query *root, List *clauses)
+ {
+     List       *nosubplans;
+     List       *withsubplans;
+     List       *l;
+ 
+     /* No need to work hard if the query is subselect-free */
+     if (!root->hasSubLinks)
+         return clauses;
+ 
+     nosubplans = withsubplans = NIL;
+     foreach(l, clauses)
+     {
+         Node   *clause = lfirst(l);
+ 
+         if (contain_subplans(clause))
+             withsubplans = lappend(withsubplans, clause);
+         else
+             nosubplans = lappend(nosubplans, clause);
+     }
+ 
+     return nconc(nosubplans, withsubplans); }  /*