Skip to content

Commit

Permalink
sql: fix decimal evaluation edge cases
Browse files Browse the repository at this point in the history
Previously, the logic for decimal and float division, floor division
and mod operators was incorrect for a few edge cases involving `NaN`
or `Infinity` values. For example, `'NaN'::DECIMAL / 0` would throw
a division-by-zero error when it should evaluate to `NaN` and
`0/'inf'::DECIMAL` returned `0E-2019` instead of just `0`.

This patch updates the special-case logic to mirror that of postgres,
so division-by-zero errors always check the `NaN` case and the division
by infinity case returns a `0` without extra digits.

Fixes cockroachdb#40929
Fixes cockroachdb#103633

Release note (bug fix): Fixed edge cases in decimal and float evaluation
for division operators. `'NaN'::DECIMAL / 0` will now return `NaN` instead
of a division-by-zero error, and `0 / 'inf'::DECIMAL` will return `0`
instead of `0E-2019`.
  • Loading branch information
DrewKimball committed Sep 12, 2023
1 parent 50c96a6 commit 51743a3
Show file tree
Hide file tree
Showing 15 changed files with 1,059 additions and 257 deletions.
14 changes: 2 additions & 12 deletions pkg/cmd/roachtest/tests/sqlsmith.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,18 +231,8 @@ WITH into_db = 'defaultdb', unsafe_restore_incompatible_version;
if err != nil {
es := err.Error()
if strings.Contains(es, "internal error") {
// TODO(yuzefovich): we temporarily ignore internal errors
// that are because of #40929.
var expectedError bool
for _, exp := range []string{
"could not parse \"0E-2019\" as type decimal",
} {
expectedError = expectedError || strings.Contains(es, exp)
}
if !expectedError {
logStmt(stmt)
t.Fatalf("error: %s\nstmt:\n%s;", err, stmt)
}
logStmt(stmt)
t.Fatalf("error: %s\nstmt:\n%s;", err, stmt)
} else if strings.Contains(es, "Empty statement returned by generate") ||
stmt == "" {
// Either were unable to generate a statement or
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/colexec/colexecagg/hash_avg_agg.eg.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/sql/colexec/colexecagg/hash_sum_agg.eg.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/sql/colexec/colexecagg/ordered_avg_agg.eg.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/sql/colexec/colexecagg/ordered_sum_agg.eg.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/sql/colexec/colexecagg/window_avg_agg.eg.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions pkg/sql/colexec/colexecagg/window_sum_agg.eg.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 51743a3

Please sign in to comment.