This is a tracking issue for a new version of the Sum aggregate, SumV2, that adheres to sql semantics with respect to empty arrays and all-null arrays.
We decided to not patch the existing Sum aggregate out of forwards-compat concerns, and to instead create a new aggregate function.
Motivation
Sum does not follow SQL semantics. In particular:
sum([]) == 0sum([null, null, ..., null]) == 0
This is a problem with GroupedSum as well.
Design
Add a new aggregate function SumV2 with the desired semantics and integrate into engine pushdown and stats provider.
Steps
Unresolved questions
- We also should integrate this behavior into
Mean, since that relies on old Sum. Should we also have a MeanV2?
- Should we rename old
sum to sum_legacy (keeping its id intact) and new sum to sum so that users will use this (more correct) scalarfn going forward?
Implementation history
Originally merged patched Sum in #9113.
Reverted in #9324 out of forward-compat concerns.
This is a tracking issue for a new version of the
Sumaggregate,SumV2, that adheres to sql semantics with respect to empty arrays and all-null arrays.We decided to not patch the existing
Sumaggregate out of forwards-compat concerns, and to instead create a new aggregate function.Motivation
Sumdoes not follow SQL semantics. In particular:sum([]) == 0sum([null, null, ..., null]) == 0This is a problem with
GroupedSumas well.Design
Add a new aggregate function
SumV2with the desired semantics and integrate into engine pushdown and stats provider.Steps
Unresolved questions
Mean, since that relies on oldSum. Should we also have aMeanV2?sumtosum_legacy(keeping its id intact) and new sum tosumso that users will use this (more correct) scalarfn going forward?Implementation history
Originally merged patched
Sumin #9113.Reverted in #9324 out of forward-compat concerns.