matrixorigin / matrixorigin/matrixone

[Bug]: ROUND misrounds negative signed integers at negative precision

Open
#28,490 1 comment 0 reactions 1 assignee Claimed by @jiangxinmeng1 View on GitHub
area/compatibility kind/bug needs-triage
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.