apache / apache/beam

[Bug]: SQL AVG rounds to 10 significant digits, flipping the sign on large BIGINT values

Open
#39,751 0 comments 0 reactions 0 assignees View on GitHub
java P2
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.