Re: Propose RC1 for Friday ...

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);
> }
>
> /*
>