github-actions[bot] commented on code in PR #68251:
URL: https://github.com/apache/doris/pull/68251#discussion_r4056397127
##########
be/src/exprs/function/round.h:
##########
@@ -108,41 +108,42 @@ struct IntegerRoundingComputation {
static size_t prepare(size_t scale) { return scale; }
- /// Integer overflow is Ok.
static ALWAYS_INLINE T compute_impl(T x, T scale, T target_scale) {
+ T quotient = x / scale;
+ const T remainder = x % scale;
Review Comment:
[P2] Avoid a second Decimal256 division per active row
For `T = wide::Int256`, `/` and `%` independently call the custom
`_impl::divide` shift/subtract routine. Thus each Floor/Ceil row that reaches
`compute_impl` now expresses two full 256-bit divisions where the old path used
one (identity and saturated-factor rows bypass this code). Please derive the
remainder as `x - quotient * scale`—the product magnitude cannot exceed `x`—or
expose a single divmod primitive; a non-dividing parity test can also avoid `%
2` on bankers ties. The removed rescaling pass may offset some total runtime,
but it does not remove this avoidable long division.
##########
be/src/exprs/function/round.h:
##########
@@ -176,45 +191,35 @@ class DecimalRoundingImpl {
public:
static NO_INLINE void apply(const Container& in, UInt32 in_scale,
Container& out,
- Int16 out_scale) {
- Int16 scale_arg = in_scale - out_scale;
+ Int16 out_scale, Int16 result_scale) {
+ Int32 scale_arg = static_cast<Int32>(in_scale) - out_scale;
Review Comment:
[P2] Cover the scale-difference widening at `INT16_MIN`
This widening fixes a reachable boundary that the new tests do not exercise:
scale arguments accept `-32768`, so Decimal128 scale 38 produces a difference
of 32806. The old Int16 assignment narrows that to a negative value on the
supported toolchain and incorrectly takes the memcpy path (for example, Round
of Decimal128 `0.9` returns the copied payload instead of zero). Please add
`INT16_MIN` cases through both a constant scale and a scale column so the
container and scalar overload changes each have a pre-fix-failing oracle.
--
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]
