Hi all,
Following up on my previous mail: after looking further into 0002, I'd
like to limit this patch to 0001 and leave the ndistinct part out.
The reasons:
- 0001 is a real bound, 0002 is not. Every combination missing from the
MCV list is at most as frequent as the least common tracked item, so
capping there cannot make an estimate worse, and improves the cases
where the per-column estimate is above that frequency.
- 0001 is self-contained. It fixes the case that started this thread
(individually common values whose combination is missing from the
MCV list).
- 0002 is an estimate, not a bound, and it only helps when the columns
are dependent. When they are close to independent, the current
per-column estimate is already accurate.
v7, attached to my previous mail, addresses the comments. I'll leave
v7-0001 as it is and wait for a committer review.
Thanks Ilia, Chengpeng and Zsolt for the reviews.
Best regards,
Enrique.
Hi Chengpeng, thanks for the review!
For patch 0001, apart from Ilia's comments, I only have one minor issue
with a comment in `mcv_can_cap()`:
Expressions are not supported, they can match multiple rows.
is not quite accurate. Matching multiple rows is not specific to
expressions; ordinary equality clauses may do so as well when they do
not fully constrain all dimensions. Expressions are excluded here
instead because the current full-coverage check only considers columns.
Other than that, patch 0001 looks good to me.
I've addressed the comments in the attached v7 patch.
Regarding patch 0002, the formula seems reasonable as an estimate, but I
am not convinced that it can serve as an upper bound. Some non-MCV
combinations may be more frequent than the average, so using it as a cap
does not seem justified. It may be more or less accurate than the
existing per-column estimate depending on the data distribution.
Fair point, an average over the non-MCV combinations is not a true
upper bound, it's an estimation. I've been looking at how to reframe it
(it is essentially the same estimate var_eq_const() applies for single
columns on an MCV miss), but I think that discussion deserves its own
time (with a proper math analysis) and shouldn't hold back the rest.
So I'd propose we set 0002 aside for now and proceed with 0001, whose
reasoning is not affected by this: the least common MCV item is a
valid bound for any combination not in the list. I'll come back to
the ndistinct part in a follow-up.
Best regards,
Enrique.
