andygrove commented on issue #485:
URL:
https://github.com/apache/datafusion-comet/issues/485#issuecomment-2144059894
It looks like `log` calls get mapped to this protobuf type:
```
message ScalarFunc {
string func = 1;
repeated Expr args = 2;
DataType return_type = 3;
}
```
In `planner.rs` we have `create_scalar_function_expr`, which calls
`create_comet_physical_fun`, which has special handling for scalar functions
that we have implemented in comet but then falls back to DataFusion built-in
functions for anything else:
```
if let Ok(fun) = BuiltinScalarFunction::from_str(fun_name) {
Ok(ScalarFunctionDefinition::BuiltIn(fun))
```
So this is where we get the DataFusion log functions.
`Log(0)` is undefined behavior and different systems return different
results.
I wonder if we should just document this behavior rather than try and make
Comet compatible.
If we do want to match Spark, we could wrap the log expr in another
expression, such as `if(expr == 0, null, log(expr))`. Comet does already have
support for if expressions:
```rust
IfExpr::new(if_expr, true_expr, false_expr)
```
--
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]
