From 9b6882fd8066a1f90f571a9d8a85fa44ecf843b5 Mon Sep 17 00:00:00 2001 From: Kumar Ujjawal Date: Thu, 15 Jan 2026 12:57:19 +0530 Subject: [PATCH 1/6] perf: Optimize round scalar performance --- datafusion/functions/Cargo.toml | 5 + datafusion/functions/benches/round.rs | 154 +++++++++++++++++++++++++ datafusion/functions/src/math/round.rs | 44 +++++++ 3 files changed, 203 insertions(+) create mode 100644 datafusion/functions/benches/round.rs diff --git a/datafusion/functions/Cargo.toml b/datafusion/functions/Cargo.toml index 939fcfd11fba..610ab1617a8d 100644 --- a/datafusion/functions/Cargo.toml +++ b/datafusion/functions/Cargo.toml @@ -320,3 +320,8 @@ required-features = ["math_expressions"] harness = false name = "floor_ceil" required-features = ["math_expressions"] + +[[bench]] +harness = false +name = "round" +required-features = ["math_expressions"] diff --git a/datafusion/functions/benches/round.rs b/datafusion/functions/benches/round.rs new file mode 100644 index 000000000000..ea59584919d6 --- /dev/null +++ b/datafusion/functions/benches/round.rs @@ -0,0 +1,154 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +extern crate criterion; + +use arrow::datatypes::{DataType, Field, Float32Type, Float64Type}; +use arrow::util::bench_util::create_primitive_array; +use criterion::{Criterion, SamplingMode, criterion_group, criterion_main}; +use datafusion_common::ScalarValue; +use datafusion_common::config::ConfigOptions; +use datafusion_expr::{ColumnarValue, ScalarFunctionArgs}; +use datafusion_functions::math::round; +use std::hint::black_box; +use std::sync::Arc; +use std::time::Duration; + +fn criterion_benchmark(c: &mut Criterion) { + let round_fn = round(); + let config_options = Arc::new(ConfigOptions::default()); + + for size in [1024, 4096, 8192] { + let mut group = c.benchmark_group(format!("round size={size}")); + group.sampling_mode(SamplingMode::Flat); + group.sample_size(10); + group.measurement_time(Duration::from_secs(10)); + + // Float64 array benchmark + let f64_array = Arc::new(create_primitive_array::(size, 0.1)); + let batch_len = f64_array.len(); + let f64_args = vec![ + ColumnarValue::Array(f64_array), + ColumnarValue::Scalar(ScalarValue::Int32(Some(2))), + ]; + + group.bench_function("round_f64_array", |b| { + b.iter(|| { + let args_cloned = f64_args.clone(); + black_box( + round_fn + .invoke_with_args(ScalarFunctionArgs { + args: args_cloned, + arg_fields: vec![ + Field::new("a", DataType::Float64, true).into(), + Field::new("b", DataType::Int32, false).into(), + ], + number_rows: batch_len, + return_field: Field::new("f", DataType::Float64, true).into(), + config_options: Arc::clone(&config_options), + }) + .unwrap(), + ) + }) + }); + + // Float32 array benchmark + let f32_array = Arc::new(create_primitive_array::(size, 0.1)); + let f32_args = vec![ + ColumnarValue::Array(f32_array), + ColumnarValue::Scalar(ScalarValue::Int32(Some(2))), + ]; + + group.bench_function("round_f32_array", |b| { + b.iter(|| { + let args_cloned = f32_args.clone(); + black_box( + round_fn + .invoke_with_args(ScalarFunctionArgs { + args: args_cloned, + arg_fields: vec![ + Field::new("a", DataType::Float32, true).into(), + Field::new("b", DataType::Int32, false).into(), + ], + number_rows: batch_len, + return_field: Field::new("f", DataType::Float32, true).into(), + config_options: Arc::clone(&config_options), + }) + .unwrap(), + ) + }) + }); + + // Scalar benchmark (the optimization we added) + let scalar_f64_args = vec![ + ColumnarValue::Scalar(ScalarValue::Float64(Some(std::f64::consts::PI))), + ColumnarValue::Scalar(ScalarValue::Int32(Some(2))), + ]; + + group.bench_function("round_f64_scalar", |b| { + b.iter(|| { + let args_cloned = scalar_f64_args.clone(); + black_box( + round_fn + .invoke_with_args(ScalarFunctionArgs { + args: args_cloned, + arg_fields: vec![ + Field::new("a", DataType::Float64, false).into(), + Field::new("b", DataType::Int32, false).into(), + ], + number_rows: 1, + return_field: Field::new("f", DataType::Float64, false) + .into(), + config_options: Arc::clone(&config_options), + }) + .unwrap(), + ) + }) + }); + + let scalar_f32_args = vec![ + ColumnarValue::Scalar(ScalarValue::Float32(Some(std::f32::consts::PI))), + ColumnarValue::Scalar(ScalarValue::Int32(Some(2))), + ]; + + group.bench_function("round_f32_scalar", |b| { + b.iter(|| { + let args_cloned = scalar_f32_args.clone(); + black_box( + round_fn + .invoke_with_args(ScalarFunctionArgs { + args: args_cloned, + arg_fields: vec![ + Field::new("a", DataType::Float32, false).into(), + Field::new("b", DataType::Int32, false).into(), + ], + number_rows: 1, + return_field: Field::new("f", DataType::Float32, false) + .into(), + config_options: Arc::clone(&config_options), + }) + .unwrap(), + ) + }) + }); + + group.finish(); + } +} + +criterion_group!(benches, criterion_benchmark); +criterion_main!(benches); diff --git a/datafusion/functions/src/math/round.rs b/datafusion/functions/src/math/round.rs index de70788128b8..c2e856bcd8e3 100644 --- a/datafusion/functions/src/math/round.rs +++ b/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 + _ => {} + } + } + round_columnar(&args.args[0], decimal_places, args.number_rows) } From c10b651b374cb849169ff0de70113b18953c735d Mon Sep 17 00:00:00 2001 From: Kumar Ujjawal Date: Fri, 16 Jan 2026 11:05:29 +0530 Subject: [PATCH 2/6] use round_float for consistency --- datafusion/functions/src/math/round.rs | 30 ++++++++++++++++---------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/datafusion/functions/src/math/round.rs b/datafusion/functions/src/math/round.rs index c2e856bcd8e3..3ecfb69e6463 100644 --- a/datafusion/functions/src/math/round.rs +++ b/datafusion/functions/src/math/round.rs @@ -31,7 +31,7 @@ use arrow::error::ArrowError; use datafusion_common::types::{ NativeType, logical_float32, logical_float64, logical_int32, }; -use datafusion_common::{Result, ScalarValue, exec_err}; +use datafusion_common::{DataFusionError, Result, ScalarValue, exec_err}; use datafusion_expr::sort_properties::{ExprProperties, SortProperties}; use datafusion_expr::{ Coercion, ColumnarValue, Documentation, ScalarFunctionArgs, ScalarUDFImpl, Signature, @@ -146,13 +146,22 @@ impl ScalarUDFImpl for RoundFunc { (&args.args[0], decimal_places) { // Extract decimal places as i32 + // Note: decimal_places is coerced to Int32 by the signature, so the non-Int32 + // arm should be unreachable in normal execution. We fall through to the array + // path as a safety measure. let dp = match dp_scalar { ScalarValue::Int32(Some(dp)) => *dp, ScalarValue::Int32(None) => { - return Ok(ColumnarValue::Scalar(ScalarValue::Float64(None))); + // Return type depends on input type, but for null dp we return null + return match value_scalar { + ScalarValue::Float32(_) => { + Ok(ColumnarValue::Scalar(ScalarValue::Float32(None))) + } + _ => Ok(ColumnarValue::Scalar(ScalarValue::Float64(None))), + }; } _ => { - // Fall through to array path for non-Int32 decimal places + // Unreachable in normal execution due to type coercion return round_columnar( &args.args[0], decimal_places, @@ -163,23 +172,22 @@ impl ScalarUDFImpl for RoundFunc { match value_scalar { ScalarValue::Float64(Some(v)) => { - let factor = 10_f64.powi(dp); - return Ok(ColumnarValue::Scalar(ScalarValue::Float64(Some( - (v * factor).round() / factor, - )))); + return round_float(*v, dp) + .map(|r| ColumnarValue::Scalar(ScalarValue::Float64(Some(r)))) + .map_err(DataFusionError::from); } 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, - )))); + return round_float(*v, dp) + .map(|r| ColumnarValue::Scalar(ScalarValue::Float32(Some(r)))) + .map_err(DataFusionError::from); } ScalarValue::Float32(None) => { return Ok(ColumnarValue::Scalar(ScalarValue::Float32(None))); } + // TODO: Add scalar fast path for decimal types // For decimals and other types: fall through to array path _ => {} } From 3cb991e8959d44216876a607bde95afc235433f6 Mon Sep 17 00:00:00 2001 From: Kumar Ujjawal Date: Fri, 16 Jan 2026 13:22:40 +0530 Subject: [PATCH 3/6] add decimal support --- datafusion/functions/src/math/round.rs | 101 ++++++++++++++++++++++--- 1 file changed, 89 insertions(+), 12 deletions(-) diff --git a/datafusion/functions/src/math/round.rs b/datafusion/functions/src/math/round.rs index 3ecfb69e6463..828bb0e981e8 100644 --- a/datafusion/functions/src/math/round.rs +++ b/datafusion/functions/src/math/round.rs @@ -141,31 +141,40 @@ impl ScalarUDFImpl for RoundFunc { &default_decimal_places }; - // Scalar fast path for float types - avoid array conversion overhead + // Scalar fast path for float and decimal 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 // Note: decimal_places is coerced to Int32 by the signature, so the non-Int32 - // arm should be unreachable in normal execution. We fall through to the array - // path as a safety measure. + // arm should be unreachable in normal execution. let dp = match dp_scalar { ScalarValue::Int32(Some(dp)) => *dp, ScalarValue::Int32(None) => { - // Return type depends on input type, but for null dp we return null + // Return null with correct type for null decimal_places return match value_scalar { ScalarValue::Float32(_) => { Ok(ColumnarValue::Scalar(ScalarValue::Float32(None))) } + ScalarValue::Decimal128(_, p, s) => Ok(ColumnarValue::Scalar( + ScalarValue::Decimal128(None, *p, *s), + )), + ScalarValue::Decimal256(_, p, s) => Ok(ColumnarValue::Scalar( + ScalarValue::Decimal256(None, *p, *s), + )), + ScalarValue::Decimal64(_, p, s) => Ok(ColumnarValue::Scalar( + ScalarValue::Decimal64(None, *p, *s), + )), + ScalarValue::Decimal32(_, p, s) => Ok(ColumnarValue::Scalar( + ScalarValue::Decimal32(None, *p, *s), + )), _ => Ok(ColumnarValue::Scalar(ScalarValue::Float64(None))), }; } _ => { - // Unreachable in normal execution due to type coercion - return round_columnar( - &args.args[0], - decimal_places, - args.number_rows, + return exec_err!( + "Internal error: round decimal_places should be Int32, got {:?}", + dp_scalar ); } }; @@ -187,9 +196,77 @@ impl ScalarUDFImpl for RoundFunc { ScalarValue::Float32(None) => { return Ok(ColumnarValue::Scalar(ScalarValue::Float32(None))); } - // TODO: Add scalar fast path for decimal types - // For decimals and other types: fall through to array path - _ => {} + ScalarValue::Decimal128(Some(v), precision, scale) => { + return round_decimal(*v, *scale, dp) + .map(|r| { + ColumnarValue::Scalar(ScalarValue::Decimal128( + Some(r), + *precision, + *scale, + )) + }) + .map_err(DataFusionError::from); + } + ScalarValue::Decimal128(None, precision, scale) => { + return Ok(ColumnarValue::Scalar(ScalarValue::Decimal128( + None, *precision, *scale, + ))); + } + ScalarValue::Decimal256(Some(v), precision, scale) => { + return round_decimal(*v, *scale, dp) + .map(|r| { + ColumnarValue::Scalar(ScalarValue::Decimal256( + Some(r), + *precision, + *scale, + )) + }) + .map_err(DataFusionError::from); + } + ScalarValue::Decimal256(None, precision, scale) => { + return Ok(ColumnarValue::Scalar(ScalarValue::Decimal256( + None, *precision, *scale, + ))); + } + ScalarValue::Decimal64(Some(v), precision, scale) => { + return round_decimal(*v, *scale, dp) + .map(|r| { + ColumnarValue::Scalar(ScalarValue::Decimal64( + Some(r), + *precision, + *scale, + )) + }) + .map_err(DataFusionError::from); + } + ScalarValue::Decimal64(None, precision, scale) => { + return Ok(ColumnarValue::Scalar(ScalarValue::Decimal64( + None, *precision, *scale, + ))); + } + ScalarValue::Decimal32(Some(v), precision, scale) => { + return round_decimal(*v, *scale, dp) + .map(|r| { + ColumnarValue::Scalar(ScalarValue::Decimal32( + Some(r), + *precision, + *scale, + )) + }) + .map_err(DataFusionError::from); + } + ScalarValue::Decimal32(None, precision, scale) => { + return Ok(ColumnarValue::Scalar(ScalarValue::Decimal32( + None, *precision, *scale, + ))); + } + // All supported scalar types are handled above + _ => { + return exec_err!( + "Internal error: unexpected scalar type for round: {:?}", + value_scalar + ); + } } } From 3a772811f64a27935e23018705afccface96230d Mon Sep 17 00:00:00 2001 From: Kumar Ujjawal Date: Sat, 17 Jan 2026 14:40:27 +0530 Subject: [PATCH 4/6] add unit test and refactor --- datafusion/functions/src/math/round.rs | 283 +++++++++++++++---------- 1 file changed, 174 insertions(+), 109 deletions(-) diff --git a/datafusion/functions/src/math/round.rs b/datafusion/functions/src/math/round.rs index 828bb0e981e8..fafe3a5e59d5 100644 --- a/datafusion/functions/src/math/round.rs +++ b/datafusion/functions/src/math/round.rs @@ -31,7 +31,7 @@ use arrow::error::ArrowError; use datafusion_common::types::{ NativeType, logical_float32, logical_float64, logical_int32, }; -use datafusion_common::{DataFusionError, Result, ScalarValue, exec_err}; +use datafusion_common::{Result, ScalarValue, exec_err, internal_err}; use datafusion_expr::sort_properties::{ExprProperties, SortProperties}; use datafusion_expr::{ Coercion, ColumnarValue, Documentation, ScalarFunctionArgs, ScalarUDFImpl, Signature, @@ -145,132 +145,63 @@ impl ScalarUDFImpl for RoundFunc { if let (ColumnarValue::Scalar(value_scalar), ColumnarValue::Scalar(dp_scalar)) = (&args.args[0], decimal_places) { - // Extract decimal places as i32 - // Note: decimal_places is coerced to Int32 by the signature, so the non-Int32 - // arm should be unreachable in normal execution. - let dp = match dp_scalar { - ScalarValue::Int32(Some(dp)) => *dp, - ScalarValue::Int32(None) => { - // Return null with correct type for null decimal_places - return match value_scalar { - ScalarValue::Float32(_) => { - Ok(ColumnarValue::Scalar(ScalarValue::Float32(None))) - } - ScalarValue::Decimal128(_, p, s) => Ok(ColumnarValue::Scalar( - ScalarValue::Decimal128(None, *p, *s), - )), - ScalarValue::Decimal256(_, p, s) => Ok(ColumnarValue::Scalar( - ScalarValue::Decimal256(None, *p, *s), - )), - ScalarValue::Decimal64(_, p, s) => Ok(ColumnarValue::Scalar( - ScalarValue::Decimal64(None, *p, *s), - )), - ScalarValue::Decimal32(_, p, s) => Ok(ColumnarValue::Scalar( - ScalarValue::Decimal32(None, *p, *s), - )), - _ => Ok(ColumnarValue::Scalar(ScalarValue::Float64(None))), - }; - } - _ => { - return exec_err!( - "Internal error: round decimal_places should be Int32, got {:?}", - dp_scalar - ); - } + if value_scalar.is_null() || dp_scalar.is_null() { + return ColumnarValue::Scalar(ScalarValue::Null) + .cast_to(args.return_type(), None); + } + + let dp = if let ScalarValue::Int32(Some(dp)) = dp_scalar { + *dp + } else { + return internal_err!( + "Unexpected datatype for decimal_places: {}", + dp_scalar.data_type() + ); }; match value_scalar { - ScalarValue::Float64(Some(v)) => { - return round_float(*v, dp) - .map(|r| ColumnarValue::Scalar(ScalarValue::Float64(Some(r)))) - .map_err(DataFusionError::from); - } - ScalarValue::Float64(None) => { - return Ok(ColumnarValue::Scalar(ScalarValue::Float64(None))); - } ScalarValue::Float32(Some(v)) => { - return round_float(*v, dp) - .map(|r| ColumnarValue::Scalar(ScalarValue::Float32(Some(r)))) - .map_err(DataFusionError::from); + let rounded = round_float(*v, dp)?; + Ok(ColumnarValue::Scalar(ScalarValue::from(rounded))) } - ScalarValue::Float32(None) => { - return Ok(ColumnarValue::Scalar(ScalarValue::Float32(None))); + ScalarValue::Float64(Some(v)) => { + let rounded = round_float(*v, dp)?; + Ok(ColumnarValue::Scalar(ScalarValue::from(rounded))) } ScalarValue::Decimal128(Some(v), precision, scale) => { - return round_decimal(*v, *scale, dp) - .map(|r| { - ColumnarValue::Scalar(ScalarValue::Decimal128( - Some(r), - *precision, - *scale, - )) - }) - .map_err(DataFusionError::from); - } - ScalarValue::Decimal128(None, precision, scale) => { - return Ok(ColumnarValue::Scalar(ScalarValue::Decimal128( - None, *precision, *scale, - ))); + let rounded = round_decimal(*v, *scale, dp)?; + let scalar = + ScalarValue::Decimal128(Some(rounded), *precision, *scale); + Ok(ColumnarValue::Scalar(scalar)) } ScalarValue::Decimal256(Some(v), precision, scale) => { - return round_decimal(*v, *scale, dp) - .map(|r| { - ColumnarValue::Scalar(ScalarValue::Decimal256( - Some(r), - *precision, - *scale, - )) - }) - .map_err(DataFusionError::from); - } - ScalarValue::Decimal256(None, precision, scale) => { - return Ok(ColumnarValue::Scalar(ScalarValue::Decimal256( - None, *precision, *scale, - ))); + let rounded = round_decimal(*v, *scale, dp)?; + let scalar = + ScalarValue::Decimal256(Some(rounded), *precision, *scale); + Ok(ColumnarValue::Scalar(scalar)) } ScalarValue::Decimal64(Some(v), precision, scale) => { - return round_decimal(*v, *scale, dp) - .map(|r| { - ColumnarValue::Scalar(ScalarValue::Decimal64( - Some(r), - *precision, - *scale, - )) - }) - .map_err(DataFusionError::from); - } - ScalarValue::Decimal64(None, precision, scale) => { - return Ok(ColumnarValue::Scalar(ScalarValue::Decimal64( - None, *precision, *scale, - ))); + let rounded = round_decimal(*v, *scale, dp)?; + let scalar = + ScalarValue::Decimal64(Some(rounded), *precision, *scale); + Ok(ColumnarValue::Scalar(scalar)) } ScalarValue::Decimal32(Some(v), precision, scale) => { - return round_decimal(*v, *scale, dp) - .map(|r| { - ColumnarValue::Scalar(ScalarValue::Decimal32( - Some(r), - *precision, - *scale, - )) - }) - .map_err(DataFusionError::from); - } - ScalarValue::Decimal32(None, precision, scale) => { - return Ok(ColumnarValue::Scalar(ScalarValue::Decimal32( - None, *precision, *scale, - ))); + let rounded = round_decimal(*v, *scale, dp)?; + let scalar = + ScalarValue::Decimal32(Some(rounded), *precision, *scale); + Ok(ColumnarValue::Scalar(scalar)) } - // All supported scalar types are handled above _ => { - return exec_err!( - "Internal error: unexpected scalar type for round: {:?}", - value_scalar - ); + internal_err!( + "Unexpected datatype for value: {}", + value_scalar.data_type() + ) } } + } else { + round_columnar(&args.args[0], decimal_places, args.number_rows) } - - round_columnar(&args.args[0], decimal_places, args.number_rows) } fn output_ordering(&self, input: &[ExprProperties]) -> Result { @@ -563,4 +494,138 @@ mod test { Err(DataFusionError::ArrowError(_, _)) | Err(DataFusionError::Execution(_)) )); } + + // Tests for scalar fast path + use super::RoundFunc; + use arrow::datatypes::{DataType, Field}; + use datafusion_common::config::ConfigOptions; + use datafusion_expr::{ScalarFunctionArgs, ScalarUDFImpl}; + + fn round_scalar( + value: ScalarValue, + decimal_places: Option, + ) -> Result { + let round_func = RoundFunc::new(); + let config_options = Arc::new(ConfigOptions::default()); + + let dp = decimal_places.unwrap_or(0); + let args = vec![ + ColumnarValue::Scalar(value.clone()), + ColumnarValue::Scalar(ScalarValue::Int32(Some(dp))), + ]; + + let return_type = round_func.return_type(&[value.data_type()])?; + + let result = round_func.invoke_with_args(ScalarFunctionArgs { + args, + arg_fields: vec![ + Field::new("a", value.data_type(), true).into(), + Field::new("b", DataType::Int32, false).into(), + ], + number_rows: 1, + return_field: Field::new("f", return_type, true).into(), + config_options, + })?; + + match result { + ColumnarValue::Scalar(s) => Ok(s), + ColumnarValue::Array(a) => ScalarValue::try_from_array(&a, 0), + } + } + + #[test] + fn test_round_scalar_f64() { + // Test basic rounding + let result = round_scalar(ScalarValue::Float64(Some(3.14159)), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Float64(Some(3.14))); + + // Test negative value + let result = round_scalar(ScalarValue::Float64(Some(-3.14159)), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Float64(Some(-3.14))); + + // Test negative decimal places + let result = + round_scalar(ScalarValue::Float64(Some(12345.55)), Some(-1)).unwrap(); + assert_eq!(result, ScalarValue::Float64(Some(12350.0))); + + // Test null + let result = round_scalar(ScalarValue::Float64(None), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Float64(None)); + } + + #[test] + fn test_round_scalar_f32() { + // Test basic rounding + let result = round_scalar(ScalarValue::Float32(Some(3.14159)), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Float32(Some(3.14))); + + // Test negative value + let result = round_scalar(ScalarValue::Float32(Some(-3.14159)), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Float32(Some(-3.14))); + + // Test null + let result = round_scalar(ScalarValue::Float32(None), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Float32(None)); + } + + #[test] + fn test_round_scalar_decimal128() { + // Test basic rounding - 314159 with scale 5 = 3.14159, rounds to 3.14 = 314000 + let result = + round_scalar(ScalarValue::Decimal128(Some(314159), 10, 5), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Decimal128(Some(314000), 10, 5)); + + // Test negative value - -3.14159 rounds to -3.14 = -314000 + let result = + round_scalar(ScalarValue::Decimal128(Some(-314159), 10, 5), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Decimal128(Some(-314000), 10, 5)); + + // Test null + let result = round_scalar(ScalarValue::Decimal128(None, 10, 5), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Decimal128(None, 10, 5)); + } + + #[test] + fn test_round_scalar_decimal256() { + use arrow::datatypes::i256; + + // Test basic rounding - 314159 with scale 5 = 3.14159, rounds to 3.14 = 314000 + let result = round_scalar( + ScalarValue::Decimal256(Some(i256::from(314159)), 50, 5), + Some(2), + ) + .unwrap(); + assert_eq!( + result, + ScalarValue::Decimal256(Some(i256::from(314000)), 50, 5) + ); + + // Test null + let result = round_scalar(ScalarValue::Decimal256(None, 50, 5), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Decimal256(None, 50, 5)); + } + + #[test] + fn test_round_scalar_decimal64() { + // Test basic rounding - 314159 with scale 5 = 3.14159, rounds to 3.14 = 314000 + let result = + round_scalar(ScalarValue::Decimal64(Some(314159), 10, 5), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Decimal64(Some(314000), 10, 5)); + + // Test null + let result = round_scalar(ScalarValue::Decimal64(None, 10, 5), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Decimal64(None, 10, 5)); + } + + #[test] + fn test_round_scalar_decimal32() { + // Test basic rounding - 31416 with scale 4 = 3.1416, rounds to 3.14 = 31400 + let result = + round_scalar(ScalarValue::Decimal32(Some(31416), 7, 4), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Decimal32(Some(31400), 7, 4)); + + // Test null + let result = round_scalar(ScalarValue::Decimal32(None, 7, 4), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Decimal32(None, 7, 4)); + } } From 1b99bb296423da3124fb26f9c4d43a7500aa0d95 Mon Sep 17 00:00:00 2001 From: Kumar Ujjawal Date: Sat, 17 Jan 2026 15:00:31 +0530 Subject: [PATCH 5/6] fix clippy issues --- datafusion/functions/src/math/round.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/datafusion/functions/src/math/round.rs b/datafusion/functions/src/math/round.rs index fafe3a5e59d5..3ed2d86d7156 100644 --- a/datafusion/functions/src/math/round.rs +++ b/datafusion/functions/src/math/round.rs @@ -536,12 +536,13 @@ mod test { #[test] fn test_round_scalar_f64() { // Test basic rounding - let result = round_scalar(ScalarValue::Float64(Some(3.14159)), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Float64(Some(3.14))); + let result = round_scalar(ScalarValue::Float64(Some(125.2345)), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Float64(Some(125.23))); // Test negative value - let result = round_scalar(ScalarValue::Float64(Some(-3.14159)), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Float64(Some(-3.14))); + let result = + round_scalar(ScalarValue::Float64(Some(-125.2345)), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Float64(Some(-125.23))); // Test negative decimal places let result = @@ -556,12 +557,13 @@ mod test { #[test] fn test_round_scalar_f32() { // Test basic rounding - let result = round_scalar(ScalarValue::Float32(Some(3.14159)), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Float32(Some(3.14))); + let result = round_scalar(ScalarValue::Float32(Some(125.2345)), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Float32(Some(125.23))); // Test negative value - let result = round_scalar(ScalarValue::Float32(Some(-3.14159)), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Float32(Some(-3.14))); + let result = + round_scalar(ScalarValue::Float32(Some(-125.2345)), Some(2)).unwrap(); + assert_eq!(result, ScalarValue::Float32(Some(-125.23))); // Test null let result = round_scalar(ScalarValue::Float32(None), Some(2)).unwrap(); From c66bb841d20cdfe3b4de77a8b69fc6d0984bb0c9 Mon Sep 17 00:00:00 2001 From: Kumar Ujjawal Date: Sat, 17 Jan 2026 21:30:46 +0530 Subject: [PATCH 6/6] remove unit tests for the slt --- datafusion/functions/src/math/round.rs | 136 ------------------------- 1 file changed, 136 deletions(-) diff --git a/datafusion/functions/src/math/round.rs b/datafusion/functions/src/math/round.rs index 3ed2d86d7156..8c25c57740d5 100644 --- a/datafusion/functions/src/math/round.rs +++ b/datafusion/functions/src/math/round.rs @@ -494,140 +494,4 @@ mod test { Err(DataFusionError::ArrowError(_, _)) | Err(DataFusionError::Execution(_)) )); } - - // Tests for scalar fast path - use super::RoundFunc; - use arrow::datatypes::{DataType, Field}; - use datafusion_common::config::ConfigOptions; - use datafusion_expr::{ScalarFunctionArgs, ScalarUDFImpl}; - - fn round_scalar( - value: ScalarValue, - decimal_places: Option, - ) -> Result { - let round_func = RoundFunc::new(); - let config_options = Arc::new(ConfigOptions::default()); - - let dp = decimal_places.unwrap_or(0); - let args = vec![ - ColumnarValue::Scalar(value.clone()), - ColumnarValue::Scalar(ScalarValue::Int32(Some(dp))), - ]; - - let return_type = round_func.return_type(&[value.data_type()])?; - - let result = round_func.invoke_with_args(ScalarFunctionArgs { - args, - arg_fields: vec![ - Field::new("a", value.data_type(), true).into(), - Field::new("b", DataType::Int32, false).into(), - ], - number_rows: 1, - return_field: Field::new("f", return_type, true).into(), - config_options, - })?; - - match result { - ColumnarValue::Scalar(s) => Ok(s), - ColumnarValue::Array(a) => ScalarValue::try_from_array(&a, 0), - } - } - - #[test] - fn test_round_scalar_f64() { - // Test basic rounding - let result = round_scalar(ScalarValue::Float64(Some(125.2345)), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Float64(Some(125.23))); - - // Test negative value - let result = - round_scalar(ScalarValue::Float64(Some(-125.2345)), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Float64(Some(-125.23))); - - // Test negative decimal places - let result = - round_scalar(ScalarValue::Float64(Some(12345.55)), Some(-1)).unwrap(); - assert_eq!(result, ScalarValue::Float64(Some(12350.0))); - - // Test null - let result = round_scalar(ScalarValue::Float64(None), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Float64(None)); - } - - #[test] - fn test_round_scalar_f32() { - // Test basic rounding - let result = round_scalar(ScalarValue::Float32(Some(125.2345)), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Float32(Some(125.23))); - - // Test negative value - let result = - round_scalar(ScalarValue::Float32(Some(-125.2345)), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Float32(Some(-125.23))); - - // Test null - let result = round_scalar(ScalarValue::Float32(None), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Float32(None)); - } - - #[test] - fn test_round_scalar_decimal128() { - // Test basic rounding - 314159 with scale 5 = 3.14159, rounds to 3.14 = 314000 - let result = - round_scalar(ScalarValue::Decimal128(Some(314159), 10, 5), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Decimal128(Some(314000), 10, 5)); - - // Test negative value - -3.14159 rounds to -3.14 = -314000 - let result = - round_scalar(ScalarValue::Decimal128(Some(-314159), 10, 5), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Decimal128(Some(-314000), 10, 5)); - - // Test null - let result = round_scalar(ScalarValue::Decimal128(None, 10, 5), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Decimal128(None, 10, 5)); - } - - #[test] - fn test_round_scalar_decimal256() { - use arrow::datatypes::i256; - - // Test basic rounding - 314159 with scale 5 = 3.14159, rounds to 3.14 = 314000 - let result = round_scalar( - ScalarValue::Decimal256(Some(i256::from(314159)), 50, 5), - Some(2), - ) - .unwrap(); - assert_eq!( - result, - ScalarValue::Decimal256(Some(i256::from(314000)), 50, 5) - ); - - // Test null - let result = round_scalar(ScalarValue::Decimal256(None, 50, 5), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Decimal256(None, 50, 5)); - } - - #[test] - fn test_round_scalar_decimal64() { - // Test basic rounding - 314159 with scale 5 = 3.14159, rounds to 3.14 = 314000 - let result = - round_scalar(ScalarValue::Decimal64(Some(314159), 10, 5), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Decimal64(Some(314000), 10, 5)); - - // Test null - let result = round_scalar(ScalarValue::Decimal64(None, 10, 5), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Decimal64(None, 10, 5)); - } - - #[test] - fn test_round_scalar_decimal32() { - // Test basic rounding - 31416 with scale 4 = 3.1416, rounds to 3.14 = 31400 - let result = - round_scalar(ScalarValue::Decimal32(Some(31416), 7, 4), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Decimal32(Some(31400), 7, 4)); - - // Test null - let result = round_scalar(ScalarValue::Decimal32(None, 7, 4), Some(2)).unwrap(); - assert_eq!(result, ScalarValue::Decimal32(None, 7, 4)); - } }