Dmitry Lychagin has posted comments on this change. Change subject: [ASTERIXDB-2460][FUN] Fix sum() overflow bug ......................................................................
Patch Set 6: (2 comments) https://asterix-gerrit.ics.uci.edu/#/c/3012/6/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/AbstractSumAggregateFunction.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/std/AbstractSumAggregateFunction.java: Line 55: private double sum; we need to sum into a long value if all inputs are integers. This problem existed before this change, but let's use this opportunity to fix it. so let's have two fields sumInt64, sumDouble. as long as we're getting integer values from the input we sum them into sumInt64. as soon as we receive float/double we convert sumInt64 value into sumDouble and continue using sumDouble for all subsequent input values. We write out sumInt64 as int64 type value if we never saw float/double in the input, otherwise we write out sumDouble as double type. Let's throw an overflow error for now if adding an integer value to sumInt64 would result in an overflow (see NumericAddDescriptor.evaluateInteger()). This approach also needs to be implemented in the serializable implementation. Line 64: protected ISerializerDeserializer serde = null; we only have two kinds of serdes now, so let's just create both of the here instead of doing it every time in finish() -- To view, visit https://asterix-gerrit.ics.uci.edu/3012 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I987417770b3bfbda6af29a27acc8c96dc8a99eb8 Gerrit-PatchSet: 6 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <[email protected]> Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-HasComments: Yes
