[Bug]: SQL AVG rounds to 10 significant digits, flipping the sign on large BIGINT values
- Dominant language
- Java
- Stars
- 8.7k
- Forks
- 4.7k
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 196
Description
### What happened?
SQL `AVG` rounds every result to **10 significant digits**, so an average over a single row does not equal that row. On `BIGINT` it can come back with the opposite sign.
`BeamBuiltinAggregations.java:93`
```java
private static MathContext mc = new MathContext(10, RoundingMode.HALF_UP);
```
`:468`
```java
protected BigDecimal prepareOutput(KV accumulator) {
return accumulator.getValue().divide(new BigDecimal(accumulator.getKey()), mc);
}
```
Every AVG subtype (`:481` INT32, `:493` INT64, `:505` INT16, `:517` INT8, `:529` FLOAT, `:541` DOUBLE, `:553` DECIMAL) goes through it.
Running that division directly, one row in:
| type | input | `AVG` returns |
|---|---|---|
| `BIGINT` | `9223372036854775807` | **`-9223372036709551616`** — sign flipped |
| `BIGINT` | `-9223372036854775808` | **`9223372036709551616`** — sign flipped |
| `BIGINT` | `1786500000123` | `1786500000000` — last three digits zeroed |
| `DECIMAL` | `123456789.99` | `123456790.0` |
The third row is the one that makes this ordinary rather than exotic: any id, epoch-millis or cent-denominated amount above 10 digits is silently rounded.
### Why the path is live
- `BeamRuleSets.java:117` has `// CoreRules.AGGREGATE_REDUCE_FUNCTIONS` **commented out**, so `AVG` is not rewritten into `SUM/COUNT` and survives to `BeamAggregationRel` → `AggregationCombineFnAdapter.createCombineFn` → `BeamBuiltinAggregations.create`.
- `BeamRelDataTypeSystem` does not override `deriveAvgAggType`, so Calcite's default applies and the declared output type is the argument type.
### Why I am not sending a patch
The obvious change — `MathContext.DECIMAL128`, or dropping the `MathContext` — is **also wrong**, just differently:
- `AVG` over `DECIMAL(18,2)` should be `DECIMAL(18,2)` per SQL and per Calcite's `deriveAvgAggType`. `DECIMAL128` gives scale 33 for `{1,1,2}`, which leaks 34-digit noise into user `Row`s where `BigDecimal.equals` is scale-sensitive.
- Dropping the `MathContext` entirely turns a non-terminating division such as `AVG` of `{1,2}` over 3 rows into `ArithmeticException`.
- Some `DOUBLE` results get uglier: `AVG` of `{0.1, 0.2}` is `0.15` today and `0.15000000000000002` without the rounding.
The fix that actually matches the declared type looks like threading the output `RelDataType`'s precision and scale from `AggregateCall` into the `CombineFn`. That information exists at `AggregationCombineFnAdapter:142` but is discarded — only the unparameterised `field.getType()` reaches `BeamBuiltinAggregations.create`. Changing that touches `AVG`, `VAR_*`, `STDDEV_*`, `COVAR_*` and their coders, which is a design call for someone who owns this area rather than something to bolt on.
Happy to implement whichever direction a maintainer prefers.
### Note
#39507 is open against this same file (`Add SINGLE_VALUE aggregate function`), so whoever picks this up may want to sequence after it.
### Issue Priority
Priority: 2 (default / most bugs should be filed as P2)
### Issue Components
- [x] Component: Java SDK
- [x] Component: dsl-sql
Contributor guide
Research direction
Start with BeamBuiltinAggregations.java:93 and the AVG path around lines 468-553, then trace AggregationCombineFnAdapter.createCombineFn and its line 142 where type information is discarded. Review BeamRuleSets.java:117 and BeamRelDataTypeSystem before deciding the output-type behavior; done requires a maintainer-approved design covering AVG and the related aggregate functions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java, sql
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 42/100