opt: fix internal error "estimated row count must be non-zero" #53802
Conversation
Release justification: low risk, high benefit changes to existing functionality This commit fixes a rare error that could occur when a query had many highly selective filter predicates. This error occured when the estimated selectivity of a Select operator was 0. Prior to this commit, we prevented the selectivity of a single filter from ever going below 1e-10, but to get the overall selectivity we multiplied the individual selectivities together. We only needed 32 filter conditions in which the selectivity was 1e-10 for the overall selectivity to underflow the floating point representation and result in selectivity 0. This commit fixes the error by setting the selectivity to 1e-10 *after* multiplying the individual selectivities together if it is less than 1e-10. Fixes cockroachdb#53311 Release note (bug fix): Fixed a rare internal error that could occur during planning of queries with many highly selective predicates.
|
This change is |
There was a problem hiding this comment.
Do we also need to add similar logic inStatistics.ApplySelectivity? If the selectivity argument and s.Selectivity are non-zero, then s.Selectivity should be set to a non-zero value.
In that case, I'm wondering if it makes sense to create a special type for selectivities (maybe just an alias for float64), with a function on the type that multiplies two selectivities but ensures if both are non-zero that the result is non-zero. This would avoid the duplicated logic and hopefully make it harder for someone in the future to forget to add this logic to new code that multiplies selectivities together.
Reviewable status:
complete! 0 of 0 LGTMs obtained
There was a problem hiding this comment.
Reviewable status:
|
TFTRs!
We never call
I think that's a great idea. But since it will create changes in a lot more places, how about I save that for another PR after the stability period is over? I also plan to backport this PR, and that change will probably make the backport more difficult. I can open an issue to track it. |
Totally! |
There was a problem hiding this comment.
$$\ $$$$$$\ $$$$$$$$\ $$\ $$\
$$ | $$ __$$\\__$$ __|$$$\ $$$ |
$$ | $$ / \__| $$ | $$$$\ $$$$ |
$$ | $$ |$$$$\ $$ | $$\$$\$$ $$ |
$$ | $$ |\_$$ | $$ | $$ \$$$ $$ |
$$ | $$ | $$ | $$ | $$ |\$ /$$ |
$$$$$$$$\\$$$$$$ | $$ | $$ | \_/ $$ |
\________|\______/ \__| \__| \__|
|
Build succeeded: |
Release justification: low risk, high benefit changes to existing
functionality
This commit fixes a rare error that could occur when a query had
many highly selective filter predicates. This error occured when the
estimated selectivity of a Select operator was 0. Prior to this commit,
we prevented the selectivity of a single filter from ever going below
1e-10, but to get the overall selectivity we multiplied the individual
selectivities together. We only needed 32 filter conditions in which
the selectivity was 1e-10 for the overall selectivity to underflow the
floating point representation and result in selectivity 0.
This commit fixes the error by setting the selectivity to 1e-10 after
multiplying the individual selectivities together if it is less than
1e-10.
Fixes #53311
Release note (bug fix): Fixed a rare internal error that could occur
during planning of queries with many highly selective predicates.