theirix commented on code in PR #17023: URL: https://github.com/apache/datafusion/pull/17023#discussion_r2331486576
########## datafusion/functions/src/math/log.rs: ########## @@ -121,55 +198,68 @@ impl ScalarUDFImpl for LogFunc { fn invoke_with_args(&self, args: ScalarFunctionArgs) -> Result<ColumnarValue> { let args = ColumnarValue::values_to_arrays(&args.args)?; - let mut base = ColumnarValue::Scalar(ScalarValue::Float32(Some(10.0))); - - let mut x = &args[0]; - if args.len() == 2 { - x = &args[1]; - base = ColumnarValue::Array(Arc::clone(&args[0])); - } - // note in f64::log params order is different than in sql. e.g in sql log(base, x) == f64::log(x, base) - let arr: ArrayRef = match args[0].data_type() { - DataType::Float64 => match base { - ColumnarValue::Scalar(ScalarValue::Float32(Some(base))) => { - Arc::new(x.as_primitive::<Float64Type>().unary::<_, Float64Type>( - |value: f64| f64::log(value, base as f64), - )) - } - ColumnarValue::Array(base) => { - let x = x.as_primitive::<Float64Type>(); - let base = base.as_primitive::<Float64Type>(); - let result = arrow::compute::binary::<_, _, _, Float64Type>( - x, - base, - f64::log, - )?; - Arc::new(result) as _ - } - _ => { - return exec_err!("log function requires a scalar or array for base") - } - }, - - DataType::Float32 => match base { - ColumnarValue::Scalar(ScalarValue::Float32(Some(base))) => Arc::new( - x.as_primitive::<Float32Type>() - .unary::<_, Float32Type>(|value: f32| f32::log(value, base)), + let (base, value) = if args.len() == 2 { + // note in f64::log params order is different than in sql. e.g in sql log(base, x) == f64::log(x, base) + (ColumnarValue::Array(Arc::clone(&args[0])), &args[1]) + } else { + // log(num) - assume base is 10 + let ret_type = if args[0].data_type().is_null() { + &DataType::Float64 + } else { + args[0].data_type() + }; Review Comment: The handling of an additional optional argument makes it complicated and different from `return_type`. I don't see a lot of UDFs with variable-number arguments. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org