Panic on arithmetic overflow when executing some UDAFs

[MichaelScofield]

What type of bug is this?

Unexpected error

What subsystems are affected?

Query Engine

Minimal reproduce step

Start a standalone instance, create a table like this:

create table foo(ts timestamp time index, i int64)

Then insert some data:

insert into foo values(1, -1), (2, 9223372036854775807), (3, 1)

"9223372036854775807" is i64::MAX, to explicitly produce some arithmetic overflow.
Now executeselect polyval(i, 2) from foo, you will see the panic in greptimedb:

2024-02-26T07:33:51.671361Z ERROR on_query: common_telemetry::panic_hook: panicked at src/common/function/src/scalars/aggregate/polyval.rs:205:32:
attempt to multiply with overflow backtrace=   0: backtrace::backtrace::libunwind::trace
...

To see another panic with the same cause of arithmetic overflow, try executing select diff(i) from foo through http endpoint:

curl -i -X POST -d 'sql=select diff(i) from foo' http://127.0.0.1:4000/v1/sql

(The reason we have to invoke "diff" func like this is because it produces “list" datatype, which is not writable for mysql.)

What did you expect to see?

Correct result, or error(instead of panic) on "arithmetic overflow".

What did you see instead?

panic

What operating system did you use?

all

What version of GreptimeDB did you use?

main

Relevant log output and stack trace

No response

[MichaelScofield]

I see there may have 4 possible fix for this:

1. Always able to calculate (no overflow): make the "largest type" of i64 or u64 to be float, or string. No need to say this is the least elegant way to go.
2. Refactor current codes to error on overflow. This might require some subtle type refactor. For example, to check substraction overflow, we have make the input datatype impl CheckedSub trait. But then we need to take special care for float datatypes, which is a little annoying to impl.
3. Wrapping on overflow (Datafusion's "sum" impl does this).
4. Combine 2 and 3, use the same way how Datafusion deal with overflow (by taking Arrow's ArrowNativeTypeOp), then error on it if overflow happens. There are some overflow checking methods in ArrowNativeTypeOp we can use.

[waynexia]

Offloading the op and related check to arrow/datafusion makes sense to me.

This pattern occurs in other UDF impl in our codebase. Maybe we should consider switching to ArrowNativeTypeOp in other places as well?