Hussain Towaileb has posted comments on this change. Change subject: [ASTERIXDB-2460][FUN] Fix sum() overflow bug ......................................................................
Patch Set 8: (1 comment) https://asterix-gerrit.ics.uci.edu/#/c/3012/8/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/serializable/std/AbstractSerializableSumAggregateFunction.java File asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/aggregates/serializable/std/AbstractSerializableSumAggregateFunction.java: Line 199: if (aggType == ATypeTag.BIGINT) { > I think you need to add || argType=ATypeTag.SERIALIZED_SYSTEM_NULL_TYPE_TAG I believe we're always starting with SYSTEM_NULL in init() for all aggregates (that's our flag in case 0 values are passed). Since SYSTEM_NULL indicates that it's the first value arriving, we should read an int64 not a double, you're right. I guess that has been passing unnoticed since int64 and double representation of 0 (starting value) is the same, so it didn't matter which one we used (I think?). However, I'm going to update it as requested since we shouldn't blindly assume that when we have a SYSTEM_NULL then we have a value of 0 as well. As for the test case, I believe this is the scenario of the normal serial_sum_double test cases, they only have float values and the starting value is SYSTEM_NULL, hence we have SYSTEM_NULL -> Float directly, kindly correct me if I'm wrong. (You can check serial_sum_double.2.update.sqlpp and serial_sum_double.3.query.sqlpp). -- 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: 8 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: James Fang <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-HasComments: Yes
