Jefffrey commented on code in PR #18191:
URL: https://github.com/apache/datafusion/pull/18191#discussion_r2450054616
##########
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)?;
+
+ match test {
+ ColumnarValue::Scalar(test_scalar) => {
+ if test_scalar.is_null() {
+ Ok(if_null)
+ } else {
+ Ok(if_non_null)
+ }
+ }
+ ColumnarValue::Array(test_array) => {
+ let len = test_array.len();
+
+ let if_non_null_array = match if_non_null {
+ ColumnarValue::Array(array) => array,
+ ColumnarValue::Scalar(scalar) =>
scalar.to_array_of_size(len)?,
+ };
+
+ let if_null_array = match if_null {
+ ColumnarValue::Array(array) => array,
+ ColumnarValue::Scalar(scalar) =>
scalar.to_array_of_size(len)?,
+ };
+
+ let mask = is_not_null(&test_array)?;
+ let result = zip(&mask, &if_non_null_array, &if_null_array)?;
+ Ok(ColumnarValue::Array(result))
+ }
+ }
+ }
+
+ fn simplify(
+ &self,
+ args: Vec<Expr>,
+ _info: &dyn SimplifyInfo,
+ ) -> Result<ExprSimplifyResult> {
+ if args.len() != 3 {
+ return plan_err!("nvl2 must have exactly three arguments");
+ }
+
+ let mut args = args.into_iter();
+ let test = args.next().unwrap();
+ let if_non_null = args.next().unwrap();
+ let if_null = args.next().unwrap();
Review Comment:
```suggestion
let [test, if_non_null, if_null] = take_function_args(self.name(),
args)?;
```
##########
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:
That's a good point 🤔
Personally I'm of the mind to remove this impl and have it return error;
part of the benefit of this PR is reducing the amount of code we'd need to
maintain.
--
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]
