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

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