sunchao commented on code in PR #5276:
URL: https://github.com/apache/datafusion-comet/pull/5276#discussion_r4167738412
##########
native/spark-expr/src/math_funcs/wide_decimal_binary_expr.rs:
##########
@@ -358,30 +364,41 @@ impl PhysicalExpr for WideDecimalBinaryExpr {
}
}
-/// Check if the i256 result fits in the output precision. In Ansi mode,
return an error
-/// on overflow. In Legacy/Try mode, record the overflow and return i128::MAX
as a sentinel
+/// Check if the rescaled i256 result fits in the output precision. In Ansi
mode, return an
+/// error on overflow. In Legacy/Try mode, record the overflow and return
i128::MAX as a sentinel
/// value that will be nullified by `null_if_overflow_precision`.
+///
+/// ANSI overflow messages format `report_value` at `report_scale` (Spark's
pre-toPrecision
+/// intermediate), not the rescaled result. Multiplication also applies Spark's
+/// MathContext(39, DOWN).
#[inline]
fn check_overflow_and_convert(
result: i256,
bound: i256,
neg_bound: i256,
precision: u8,
scale: i8,
+ report_value: i256,
Review Comment:
[P2] Adding the reporting parameters increases `check_overflow_and_convert`
from seven arguments to ten, causing the required Rust CI check to fail.
`.github/actions/rust-test/action.yaml` runs Clippy with `-D warnings`, which
makes `clippy::too_many_arguments` fatal and stops the job before its tests.
Could you add the targeted lint allowance used by analogous helpers, or reduce
the argument count, so this change passes the enforced check?
Evidence: At the reviewed head, running `cargo clippy --locked --offline -p
datafusion-comet-spark-expr --lib -- -D warnings` from `native/` exits 101 with
`error: this function has too many arguments (10/7)` at
`wide_decimal_binary_expr.rs:375`. This was the sole reported compilation
error. The base signature has seven arguments. The regular Linux Rust test job
invokes `.github/actions/rust-test`, whose line 30 enforces `cargo clippy
--color=never --all-targets --workspace -- -D warnings`.
--
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]
