If we are going to go for a beta6, I vote we reverse out the patch.  Of
course, I prefer neither.

Do we have to do a delay/feature analysis on this?

Marc, there will always be 7.3.1 to fix any problems.  They will surely
happen so I think it is safe to push forward for tomorrow's RC1.  Of
course, if you disagree, let's back it out.

---------------------------------------------------------------------------

Marc G. Fournier wrote:
> 
> I'd ask for a quick beta6 ... even knowing everyone would hate me :)
> 
> 
> 
> On Thu, 14 Nov 2002, Tom Lane wrote:
> 
> > 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);
> >   static void 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);
> >   }
> >
> >   /*
> >
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to [EMAIL PROTECTED] so that your
> message can get through to the mailing list cleanly
> 

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  [EMAIL PROTECTED]               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

Reply via email to