risinglight icon indicating copy to clipboard operation
risinglight copied to clipboard

executor: sum aggregation cause overflow panic

Open Fedomn opened this issue 2 years ago • 2 comments

Currently, our sum aggregation used the primitive datatype add function to calculate results. So sometimes the calculation will cause overflow panic.

Maybe we need to use the Decimal or Float64 type for the sum calculation.

Minimal reproduce step

create table t (v int);
insert into t values (2147483647), (2), (3);
select sum(v) from t;
-- thread 'tokio-runtime-worker' panicked at 'attempt to add with overflow', src/executor/aggregation/sum.rs:36:1

What did you expect to see?

create table t (v int);
insert into t values (2147483647), (2), (3);

select sum(v) from t;
+------------+
| sum(v)     |
+------------+
| 2147483652 |
+------------+

Fedomn avatar Feb 22 '22 15:02 Fedomn

We can follow the Postgres convention. IIRC, it will use i64 to sum i32 and use Decimal to sum i64.

I think we can start working on it when we have a fully functional expression framework. Currently, it's hard to write such aggregation in our system.

skyzh avatar Feb 22 '22 15:02 skyzh

Just link Postgres convention docs https://www.postgresql.org/docs/current/functions-aggregate.html

It used numeric for sum(bigint) and avg(bigint), and numeric seems similar with decimal

Fedomn avatar Feb 23 '22 03:02 Fedomn