github-actions[bot] commented on code in PR #68738:
URL: https://github.com/apache/doris/pull/68738#discussion_r4215868316
##########
be/test/exprs/function/function_round_test.cpp:
##########
@@ -974,6 +975,28 @@ FloatTestDataSet round_bankers_float64_cases = {{2.5, 0,
2.0},
{123.123456, 4, 123.123500},
{123456789.123456, 4,
123456789.123500}};
+const FloatTestDataSet large_scale_round_cases = {
+ {1.234e-20, 20, 1e-20}, {1.23456789e20, 20, 1.23456789e20},
+ {1e20, -19, 1e20}, {1e20, -20, 1e20},
+ {1e20, -25, 0}, {1e20, std::numeric_limits<Int16>::min(), 0}};
+const FloatTestDataSet large_scale_round_bankers_cases = {
+ {1.234e-20, 20, 1e-20},
+ {5e19, -20, 0},
Review Comment:
[P1] Correct the FLOAT banker test expectation. This row is also run as
TYPE_FLOAT. Casting 5e19 to Float32 stores 50000001002043867136, just above the
5e19 tie; `nearbyint(input / 1e20)` therefore returns 1 and the result is
FLOAT(1e20), while this fixture expects 0. The new test will fail for each
FLOAT constness combination. Use separate FLOAT and DOUBLE expectations or a
representable FLOAT tie.
##########
be/src/exprs/function/round.h:
##########
@@ -459,6 +461,56 @@ struct Dispatcher {
FloatRoundingImpl<T, rounding_mode, scale_mode,
tie_breaking_mode>,
IntegerRoundingImpl<T, rounding_mode, scale_mode,
tie_breaking_mode>>>;
+ static typename PrimitiveTypeTraits<T>::CppType
apply_float_rounding_with_large_scale(
+ typename PrimitiveTypeTraits<T>::CppType value, Int32 scale_arg) {
+ using ValueType = typename PrimitiveTypeTraits<T>::CppType;
+ const double input = static_cast<double>(value);
+ if (!std::isfinite(input)) {
+ return value;
+ }
+
+ if (scale_arg > 19) {
+ const double unit = std::pow(10.0,
-static_cast<double>(scale_arg));
Review Comment:
[P2] Keep enough precision for a subnormal decimal quantum. At scale 323,
`pow(10,-323)` rounds to `2 * DBL_TRUE_MIN`, although the requested unit is
slightly larger. `round(DBL_TRUE_MIN, 323)` then sees an artificial 0.5 tie and
returns `2 * DBL_TRUE_MIN`; the actual input is below half of `1e-323`, so the
correct result is zero. Add a subnormal boundary case and avoid deciding the
decimal bucket from this rounded unit.
##########
be/src/exprs/function/round.h:
##########
@@ -459,6 +461,56 @@ struct Dispatcher {
FloatRoundingImpl<T, rounding_mode, scale_mode,
tie_breaking_mode>,
IntegerRoundingImpl<T, rounding_mode, scale_mode,
tie_breaking_mode>>>;
+ static typename PrimitiveTypeTraits<T>::CppType
apply_float_rounding_with_large_scale(
+ typename PrimitiveTypeTraits<T>::CppType value, Int32 scale_arg) {
+ using ValueType = typename PrimitiveTypeTraits<T>::CppType;
+ const double input = static_cast<double>(value);
+ if (!std::isfinite(input)) {
+ return value;
+ }
+
+ if (scale_arg > 19) {
+ const double unit = std::pow(10.0,
-static_cast<double>(scale_arg));
+ if (unit == 0) {
+ return value;
+ }
+ const double upper_spacing =
+ std::abs(static_cast<double>(std::nextafter(
+ value,
std::numeric_limits<ValueType>::infinity())) -
+ input);
+ const double lower_spacing =
+ std::abs(input - static_cast<double>(std::nextafter(
+ value,
-std::numeric_limits<ValueType>::infinity())));
+ if (unit < std::min(upper_spacing, lower_spacing) / 2) {
+ return value;
+ }
+ const double scaled = input / unit;
+ if (!std::isfinite(scaled)) {
+ // At this magnitude the requested decimal precision is finer
+ // than the input type can represent.
+ return value;
+ }
+ return
static_cast<ValueType>(roundWithMode<tie_breaking_mode>(scaled, rounding_mode) *
Review Comment:
[P2] Reconstruct the selected decimal multiple accurately. For
`ceil(2.5e-20, 20)`, the integer choice is correctly 3, but `3 * pow(10,-20)`
yields `2.9999999999999997e-20`, one DOUBLE ULP below the intended decimal
ceiling `3e-20`. The negative-scale branch similarly returns
`1.0000000000000001e23` for `ceil(1.0,-23)` on this libm. The rounded binary
power must not become the final decimal grid value.
##########
be/src/exprs/function/round.h:
##########
@@ -459,6 +461,56 @@ struct Dispatcher {
FloatRoundingImpl<T, rounding_mode, scale_mode,
tie_breaking_mode>,
IntegerRoundingImpl<T, rounding_mode, scale_mode,
tie_breaking_mode>>>;
+ static typename PrimitiveTypeTraits<T>::CppType
apply_float_rounding_with_large_scale(
+ typename PrimitiveTypeTraits<T>::CppType value, Int32 scale_arg) {
+ using ValueType = typename PrimitiveTypeTraits<T>::CppType;
+ const double input = static_cast<double>(value);
+ if (!std::isfinite(input)) {
+ return value;
+ }
+
+ if (scale_arg > 19) {
+ const double unit = std::pow(10.0,
-static_cast<double>(scale_arg));
+ if (unit == 0) {
+ return value;
+ }
+ const double upper_spacing =
+ std::abs(static_cast<double>(std::nextafter(
+ value,
std::numeric_limits<ValueType>::infinity())) -
+ input);
+ const double lower_spacing =
+ std::abs(input - static_cast<double>(std::nextafter(
+ value,
-std::numeric_limits<ValueType>::infinity())));
+ if (unit < std::min(upper_spacing, lower_spacing) / 2) {
+ return value;
+ }
+ const double scaled = input / unit;
Review Comment:
[P2] Avoid crossing decimal grid boundaries in the `double` quotient. DOUBLE
`round(1.4999999999999998e-20,20)` becomes an artificial `1.5` tie here and
returns `2e-20` instead of `1e-20`. Directed modes also fail:
`ceil(1.3e-20,21)` computes `13.000000000000002` and returns about `1.4e-20`
instead of `1.3e-20`; the negative-scale `floor(1.3e26,-25)` likewise chooses
12 instead of 13 units. Preserve enough precision for both half and integer
boundaries before applying the rounding mode.
##########
be/src/exprs/function/round.h:
##########
@@ -476,13 +528,39 @@ struct Dispatcher {
size_t scale = 1;
FunctionRoundingImpl<ScaleMode::Zero>::apply(col->get_data(), scale, vec_res);
} else if (scale_arg > 0) {
- size_t scale = int_exp10(scale_arg);
-
FunctionRoundingImpl<ScaleMode::Positive>::apply(col->get_data(), scale,
- vec_res);
+ if constexpr (is_float_or_double(T)) {
+ if (scale_arg > 19) {
+ for (size_t i = 0; i < vec_res.size(); ++i) {
Review Comment:
[P3] Compute the constant-scale power once per block. This loop calls
`apply_float_rounding_with_large_scale` for every row, and the helper calls
`pow(10,-scale_arg)` each time even though `scale_arg` is constant here. The
previous vector path computed the scale once. Pass a precomputed quantum into
the row helper for both positive and negative large scales.
##########
be/src/exprs/function/round.h:
##########
@@ -459,6 +461,56 @@ struct Dispatcher {
FloatRoundingImpl<T, rounding_mode, scale_mode,
tie_breaking_mode>,
IntegerRoundingImpl<T, rounding_mode, scale_mode,
tie_breaking_mode>>>;
+ static typename PrimitiveTypeTraits<T>::CppType
apply_float_rounding_with_large_scale(
+ typename PrimitiveTypeTraits<T>::CppType value, Int32 scale_arg) {
+ using ValueType = typename PrimitiveTypeTraits<T>::CppType;
+ const double input = static_cast<double>(value);
+ if (!std::isfinite(input)) {
+ return value;
+ }
+
+ if (scale_arg > 19) {
+ const double unit = std::pow(10.0,
-static_cast<double>(scale_arg));
+ if (unit == 0) {
+ return value;
+ }
+ const double upper_spacing =
+ std::abs(static_cast<double>(std::nextafter(
+ value,
std::numeric_limits<ValueType>::infinity())) -
+ input);
+ const double lower_spacing =
+ std::abs(input - static_cast<double>(std::nextafter(
+ value,
-std::numeric_limits<ValueType>::infinity())));
+ if (unit < std::min(upper_spacing, lower_spacing) / 2) {
+ return value;
+ }
+ const double scaled = input / unit;
+ if (!std::isfinite(scaled)) {
+ // At this magnitude the requested decimal precision is finer
+ // than the input type can represent.
+ return value;
+ }
+ return
static_cast<ValueType>(roundWithMode<tie_breaking_mode>(scaled, rounding_mode) *
+ unit);
+ }
+
+ const double scale = std::pow(10.0, -static_cast<double>(scale_arg));
+ if (!std::isfinite(scale)) {
+ if constexpr (rounding_mode == RoundingMode::Floor) {
+ return input < 0 ? -std::numeric_limits<ValueType>::infinity()
+ : std::copysign(ValueType {0}, value);
+ } else if constexpr (rounding_mode == RoundingMode::Ceil) {
+ return input > 0 ? std::numeric_limits<ValueType>::infinity()
+ : std::copysign(ValueType {0}, value);
+ } else {
+ return std::copysign(ValueType {0}, value);
+ }
+ }
+
+ const double rounded = roundWithMode<tie_breaking_mode>(input / scale,
rounding_mode);
Review Comment:
[P2] Preserve directed rounding when the quotient underflows. For DOUBLE
`ceil(1e-30, -300)`, `pow(10,300)` is finite, but `1e-30 / 1e300` becomes zero
before `ceil`, so this returns zero instead of `1e300`; `floor(-1e-310,-20)`
similarly returns signed zero instead of `-1e20`. Handle a nonzero input whose
finite quotient underflows before calling the rounding mode.
--
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]
