From: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> |
---|---|
To: | "Marc G(dot) Fournier" <scrappy(at)hub(dot)org> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Ross J(dot) Reedstrom" <reedstrm(at)rice(dot)edu>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Propose RC1 for Friday ... |
Date: | 2002-11-14 20:27:38 |
Message-ID: | 200211142027.gAEKRcM25279@candle.pha.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>
--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2002-11-14 20:30:01 | Re: Propose RC1 for Friday ... |
Previous Message | Marc G. Fournier | 2002-11-14 20:20:18 | Re: Propose RC1 for Friday ... |