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

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 majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073