On Sun, 15 Mar 2020 at 00:08, Tomas Vondra <tomas.vondra@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
