On Mon, Sep 11, 2017 at 7:43 PM, Yuto Hayamizu <y.hayam...@gmail.com> wrote: > Suppose that there are three qual clauses in a scan node, current > postgres estimates per-tuple cost of the filter to be: > cost(A) + cost(B) + cost(C) > > And basic idea of the attached patch is: > cost(A) + clauselist_selectivity({A}) * cost(B) + > clauselist_selectivity({A, B}) * cost(C)
I am no planner expert and I haven't tested or studied the patch in detail, but here is some feedback for what it's worth. This idea seems to makes intuitive sense. I see that you use order_qual_clauses() to know what order they'll run in, so I'm wondering if there is any reason we shouldn't do it up front and keep it during path building, instead of running it again at plan creation time. Is there some way it could ever produce a different result at the two times? Why not also apply this logic to qpquals of joins, foreign scans, subplans? That is, instead of replacing cost_qual_eval with this code for baserels, I wonder if we should teach cost_qual_eval how to do this so those other users could also benefit (having perhaps already ordered the qual clauses earlier). This is one of those patches that risks having an impact on many query plans. Yikes. Of all the regression test queries, only updatable_views complained though, and that involves leakproof functions. I see that those get some kind of special treatment in order_qual_clauses(). + ordered_clauses = order_qual_clauses(root, rel->baserestrictinfo); + clause_list = ordered_clauses; Is clause_list necessary? Can't you just use ordered_clauses for the outer and inner loops? + List *clause_list_for_sel = NULL; The convention is to use NIL for empty lists (a nod to the Lisp machine they prototyped this project on). + /* Make a temporary clause list for selectivity calcuation */ s/calcuation/calculation/ -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers