pepijnve commented on code in PR #18191:
URL: https://github.com/apache/datafusion/pull/18191#discussion_r2450683839
##########
datafusion/functions/src/core/nvl2.rs:
##########
@@ -95,8 +96,71 @@ impl ScalarUDFImpl for NVL2Func {
Ok(arg_types[1].clone())
}
+ fn return_field_from_args(&self, args: ReturnFieldArgs) ->
Result<FieldRef> {
+ let nullable =
+ args.arg_fields[1].is_nullable() ||
args.arg_fields[2].is_nullable();
+ let return_type = args.arg_fields[1].data_type().clone();
+ Ok(Field::new(self.name(), return_type, nullable).into())
+ }
+
fn invoke_with_args(&self, args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
- nvl2_func(&args.args)
+ let [test, if_non_null, if_null] = take_function_args(self.name(),
args.args)?;
Review Comment:
> any expression handed directly to that API must still execute correctly
without being rewritten to a CASE statement first.
One subtlety here is that there is a change in semantics before and after
simplification. `nvl2(1, 1, 1 / 0)` will fail pre simplification but will work
correctly once simplified due to the switch from eager to lazy evaluation. I
think I would prefer a clear failure over a subtle difference in behaviour.
If we do want to keep the `invoke_with_args` implementations, one option
could be to consider #17997 (or some variant of that idea) so that it can also
be implemented lazily.
Regarding code maintenance/duplication, `nvl2` is an instance of the
`ExpressionOrExpression` evaluation method from `CaseExpr`. Perhaps a slightly
modified version of `CaseExpr::expr_or_expr` could be made so that `nvl` and
`nvl2` could call that? I think what I'm trying to say is that maybe code reuse
via `simplify` is maybe not the best idea.
--
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]
