Coerce and cast decimal sum #4968
Conversation
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
robert3005
left a comment
robert3005
left a comment
There was a problem hiding this comment.
I think we want a test as well
| // Test with large i64 values that require i128 accumulation | ||
| let large_val = i64::MAX / 4; | ||
| let decimal = DecimalArray::new( | ||
| buffer![large_val, large_val, large_val, large_val], |
There was a problem hiding this comment.
can you tweak this test to demonstrate that it would overflow the i64 range, but it still returns b/c it coerces up to i128? basically just change one of these to large_val + 1 or something?
a10y
left a comment
a10y
left a comment
There was a problem hiding this comment.
FWIW, AFAICT both DuckDB and DataFusion don't attempt to catch overflow on SUM, they just let it wrap
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I think because we widen the return type you can let it wrap and if it wraps you would have a type error anyway |
Signed-off-by: Robert Kruszewski <github@robertk.io>
At this point I'm considering adding "Decimal expert" to my CV 😢