opt: fix internal error due to node with MaxCost added to memo #84366
Conversation
This commit fixes an internal error caused by calculating the selectivity of a join as NaN. The solution is to avoid dividing by 0 when calculating the selectivity. Fixes cockroachdb#84236 Release note (bug fix): Fixed an internal error "node ... with MaxCost added to the memo" that could occur during planning when calculating the cardinality of an outer join when one of the inputs had 0 rows.
|
This change is |
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @msirek)
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:
pkg/sql/opt/memo/statistics_builder.go line 4083 at r1 (raw file):
) props.Selectivity { outRowCount := nonNullSelectivity.AsFloat()*(inputRowCount-inputNullCount) + nullSelectivity.AsFloat()*inputNullCount sel := props.MakeSelectivityFromFraction(outRowCount, inputRowCount)
Would it aid in future debugging if the selectivityInRange panicked with an assertion error if math.IsNan(sel)?
|
Previously, mgartner (Marcus Gartner) wrote…
Yea good call -- I will add that in a separate PR since bors is already running on this one |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 995c71b to blathers/backport-release-21.2-84366: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 21.2.x failed. See errors above. error creating merge commit from 995c71b to blathers/backport-release-22.1-84366: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
This commit addresses a leftover comment from cockroachdb#84366. Release note: None
84360: sql/sem/builtins: move definitions map to new package r=Xiang-Gu a=ajwerner Previously, the definition of builtin functions live in the `builtins` package. This was undesirable because various other packages need to acceess builtins properties by name, but it has a been a headache to achieve this without importing the `builtins` package, which stands pretty high in the dependecy chain (e.g. `seqexpr`, `memo`). This PR moves builtins definition into a new registry package that the `builtins` package calls to register builtin functions, which happens in the `init()` function. This way, other lower level packages, who wish to access builtins properties, need only to import the newly created `builtinsregistry` package. Release note: None 84376: opt: add assertion that selectivity is never NaN r=rytaft a=rytaft This commit addresses a leftover comment from #84366. Release note: None 84392: roachtest: wait for a 5x replication instead of 3x r=lidorcarmel a=lidorcarmel Flaky test: we wait for a 3x replication and then drain 3 nodes. Then we sometimes have ranges with all 3 replicas on those 3 nodes, stuck forever. Instead the test should wait for a 5x replication before starting the drain. Fixes #84128. Release note: None Co-authored-by: Andrew Werner <[email protected]> Co-authored-by: Rebecca Taft <[email protected]> Co-authored-by: Lidor Carmel <[email protected]>
This commit fixes an internal error caused by calculating the
selectivity of a join as NaN. The solution is to avoid dividing by
0 when calculating the selectivity.
Fixes #84236
Release note (bug fix): Fixed an internal error "node ... with
MaxCost added to the memo" that could occur during planning when
calculating the cardinality of an outer join when one of the inputs
had 0 rows.