On Sun, 15 Mar 2020 at 00:08, Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > > On Sat, Mar 14, 2020 at 05:56:10PM +0100, Tomas Vondra wrote: > > > >Attached is a patch series rebased on top of the current master, after > >committing the ScalarArrayOpExpr enhancements. I've updated the OR patch > >to get rid of the code duplication, and barring objections I'll get it > >committed shortly together with the two parts improving test coverage. > > I've pushed the two patches improving test coverage for functional > dependencies and MCV lists, which seems mostly non-controversial. I'll > wait a bit more with the two patches actually changing behavior (rebased > version attached, to keep cputube happy). >
Patch 0001 looks to be mostly ready. Just a couple of final comments: + if (is_or) + simple_sel = clauselist_selectivity_simple_or(root, stat_clauses, varRelid, + jointype, sjinfo, NULL, 1.0); + else Surely that should be passing 0.0 as the final argument, otherwise it will always just return simple_sel = 1.0. + * + * XXX We can't multiply with current value, because for OR clauses + * we start with 0.0, so we simply assign to s1 directly. + */ + s = statext_clauselist_selectivity(root, clauses, varRelid, + jointype, sjinfo, rel, + &estimatedclauses, true); That final part of the comment is no longer relevant (variable s1 no longer exists). Probably it could now just be deleted, since I think there are sufficient comments elsewhere to explain what's going on. Otherwise it looks good, and I think this will lead to some very worthwhile improvements. Regards, Dean
