kumarUjjawal commented on code in PR #19831:
URL: https://github.com/apache/datafusion/pull/19831#discussion_r2697005569
##########
datafusion/functions/src/math/round.rs:
##########
@@ -141,6 +141,50 @@ impl ScalarUDFImpl for RoundFunc {
&default_decimal_places
};
+ // Scalar fast path for float types - avoid array conversion overhead
+ if let (ColumnarValue::Scalar(value_scalar),
ColumnarValue::Scalar(dp_scalar)) =
+ (&args.args[0], decimal_places)
+ {
+ // Extract decimal places as i32
+ let dp = match dp_scalar {
+ ScalarValue::Int32(Some(dp)) => *dp,
+ ScalarValue::Int32(None) => {
+ return
Ok(ColumnarValue::Scalar(ScalarValue::Float64(None)));
+ }
+ _ => {
+ // Fall through to array path for non-Int32 decimal places
+ return round_columnar(
+ &args.args[0],
+ decimal_places,
+ args.number_rows,
+ );
+ }
+ };
+
+ match value_scalar {
+ ScalarValue::Float64(Some(v)) => {
+ let factor = 10_f64.powi(dp);
+ return Ok(ColumnarValue::Scalar(ScalarValue::Float64(Some(
+ (v * factor).round() / factor,
+ ))));
+ }
+ ScalarValue::Float64(None) => {
+ return
Ok(ColumnarValue::Scalar(ScalarValue::Float64(None)));
+ }
+ ScalarValue::Float32(Some(v)) => {
+ let factor = 10_f32.powi(dp);
+ return Ok(ColumnarValue::Scalar(ScalarValue::Float32(Some(
+ (v * factor).round() / factor,
+ ))));
+ }
+ ScalarValue::Float32(None) => {
+ return
Ok(ColumnarValue::Scalar(ScalarValue::Float32(None)));
+ }
+ // For decimals and other types: fall through to array path
Review Comment:
The `round_decimal` function requires the scale parameter from the decimal
type (e.g., Decimal128(precision, scale)), which makes extracting and
processing scalars more involved than for floats.
For float scalars, we just call round_float(*v, dp) directly. For decimals,
we'd need to:
- Extract the native value from ScalarValue::Decimal128 (or other decimal
variants)
- Get the scale from the type
- Call round_decimal(value, scale, dp)
- Reconstruct the ScalarValue with precision/scale
If you're comfortable with these changes in this PR for the decimal then I
can introduce these as well. What do you think?
--
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]
