matrixorigin / matrixorigin/matrixone
[Bug]: ROUND misrounds negative signed integers at negative precision
- Dominant language
- Go
- Stars
- 1.9k
- Forks
- 311
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 768
Description
## Environment
- MatrixOne: latest `main`, commit `f0c31cd4b830be32442cf329e0a3fb08aa9c16c3`
- MySQL comparison: 8.3.0
## Problem
`ROUND()` produces the wrong result when its first argument uses a signed integer overload, the value is negative, and the precision is negative. Values below the half-way point are rounded down instead of toward the nearest multiple, and values that are already exact multiples are moved down by a whole unit of the requested precision.
This affects `TINYINT`, `SMALLINT`, `INT`, and `BIGINT`, as well as integer expressions, prepared statements, and persisted CTAS results. The same values represented as `DECIMAL` or `DOUBLE` are calculated correctly.
## Minimal reproduction
```sql
SELECT ROUND(-1, -1), ROUND(-10, -1), ROUND(-11, -1);
SELECT ROUND(-100, -2), ROUND(-101, -2);
```
MatrixOne returns:
```text
-10, -20, -20
-200, -200
```
The nearest multiples, also returned by MySQL 8.3, are:
```text
0, -10, -10
-100, -100
```
The exact-multiple cases are especially clear: `ROUND(-10, -1)` must not change `-10`, and `ROUND(-100, -2)` must not change `-100`.
## Coverage
The behavior was reproduced three times through each relevant path:
- constants around the rounding boundary: `-1`, `-4`, `-5`, `-6`, `-9`, `-10`, `-11`, `-14`, `-15`, `-16`, `-19`, and `-20`;
- hundreds-scale boundaries from `-100` through `-200`;
- `TINYINT`, `SMALLINT`, `INT`, and `BIGINT` columns;
- integer expressions and SQL `PREPARE`/`EXECUTE` parameters;
- CTAS, which persists the incorrect values.
Controls:
- positive signed integers are rounded correctly;
- negative `DECIMAL` and `DOUBLE` values are rounded correctly;
- precision zero/positive, `NULL`, and `TRUNCATE()` controls are correct;
- the server remains healthy after every run.
## Code analysis
The signed-integer branch in `pkg/sql/plan/function/func_binary.go` computes a negative remainder and compares it with a positive threshold:
```go
step2 := x % scale
if step2 <= scale/2 {
x = step1 - scale
}
```
For every negative remainder, including zero, `step2 <= scale/2` is true. As a result, the implementation always selects the lower bucket for a negative integer. The comparison needs to use the magnitude of the remainder (or an equivalent sign-safe rule) and preserve exact multiples.
Contributor guide
Assessment
This issue has not been assessed yet.