Jefffrey commented on code in PR #18519:
URL: https://github.com/apache/datafusion/pull/18519#discussion_r2499845820
##########
datafusion/functions/src/math/log.rs:
##########
@@ -72,37 +72,28 @@ impl Default for LogFunc {
impl LogFunc {
pub fn new() -> Self {
+ // Converts decimals & integers to float64, accepting other floats as
is
Review Comment:
Main fix here for signature; to me this is more readable than the previous
signature, and importantly it accepts any decimals regardless of
precision/scale.
~~Note to self: add tests for decimals of different precision/scale if they
don't exist~~
- Added by
https://github.com/apache/datafusion/pull/18519/commits/0da79bd8e912bd7272c304fb7dc675b8398ca84a
##########
datafusion/functions/src/math/log.rs:
##########
@@ -277,17 +261,28 @@ impl ScalarUDFImpl for LogFunc {
mut args: Vec<Expr>,
info: &dyn SimplifyInfo,
) -> Result<ExprSimplifyResult> {
+ let mut arg_types = args
+ .iter()
+ .map(|arg| info.get_data_type(arg))
+ .collect::<Result<Vec<_>>>()?;
+ let return_type = self.return_type(&arg_types)?;
+
+ // Null propagation
+ if arg_types.iter().any(|dt| dt.is_null()) {
+ return Ok(ExprSimplifyResult::Simplified(lit(
+ ScalarValue::Null.cast_to(&return_type)?
+ )));
+ }
Review Comment:
So any `log(null, x)`, `log(x, null)` or `log(null)` should be immediately
simplified to `null`.
~~I'll add a test case to show this actually has an effect.~~
- Added by
https://github.com/apache/datafusion/pull/18519/commits/0da79bd8e912bd7272c304fb7dc675b8398ca84a
--
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]
