github-actions[bot] commented on code in PR #65729:
URL: https://github.com/apache/doris/pull/65729#discussion_r3597145140
##########
be/src/exprs/aggregate/aggregate_function_avg.h:
##########
@@ -262,6 +261,7 @@ class AggregateFunctionAvg<T, TResult, Data> final
NO_SANITIZE_UNDEFINED void deserialize_and_merge_from_column_range(
AggregateDataPtr __restrict place, const IColumn& column, size_t
begin, size_t end,
Arena&) const override {
+ ALLOW_FP_REASSOCIATION
Review Comment:
Please add focused floating-state correctness coverage for this new path.
`ALLOW_FP_REASSOCIATION` only changes this loop when the AVG state sum is
floating, but the BE AVG unit test constructs `DataTypeInt64`, and ordinary
DOUBLE query tests do not control how many partial states reach this range
merge. A deterministic DOUBLE unit test should merge enough serialized states
to cover the vector body plus tail, a nonzero inclusive subrange into a
nonempty destination, and verify the exact count and average with an explicit
tolerance. The benchmark demonstrates speed, but it cannot catch a
range/tail/count regression here.
##########
be/src/common/compiler_util.h:
##########
@@ -56,4 +56,12 @@
#define NO_SANITIZE_UNDEFINED __attribute__((__no_sanitize__("undefined")))
#else
#define NO_SANITIZE_UNDEFINED
-#endif
\ No newline at end of file
+#endif
+
+// Allow reassociation only when the evaluation order is not part of the
operation's contract.
+// This may change the least significant bits of floating-point results.
Review Comment:
`reassociate(on)` is not bounded to least-significant-bit drift. With this
AVG state's exact loop shape, 32 count-one serialized states repeating `[2^53,
1, 0, 0, -2^53, 0, 0, 0]` produce strict average `0` but reassociated average
`0.125`; overflow/cancellation can likewise change `+Inf` to a finite result.
Since this is a reusable common macro, please document that callers must accept
potentially material numeric and classification changes. If only low-bit
variation is acceptable, this pragma does not provide that bound.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
