pepijnve commented on code in PR #18191:
URL: https://github.com/apache/datafusion/pull/18191#discussion_r2450458092
##########
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:
The change in `coalesce` (and now indirectly also `nvl`/`ifnull`) already
broke this though. If unsimplified execution is desirable, perhaps `nvl` should
be restored too because to not have arbitrary behaviour depending on the used
UDF. In other words, I think you have to be consistent about this. Either all
physical exprs should work or you shouldn’t bother with this. Cherry picking is
a bit pointless in my opinion.
--
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]
