Extended datatypes & signatures support for NULLIF function
#4737
Conversation
github-actions
bot
added
logical-expr
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Looks good to me -- thank you @korowa
One thing that might be helpful is to add some sql level tests, perhaps in the new sqllogictest
Or in new https://github.com/apache/arrow-datafusion/tree/master/datafusion/core/tests/sqllogictests
|
|
||
| use super::binary::array_eq_scalar; | ||
|
|
||
| /// Invoke a compute kernel on a primitive array and a Boolean Array |
There was a problem hiding this comment.
😍
Thank you for this cleanup!
| )), | ||
| (ColumnarValue::Scalar(lhs), ColumnarValue::Scalar(rhs)) => { | ||
| let val = match lhs.eq(rhs) { | ||
| true => ScalarValue::Null, |
There was a problem hiding this comment.
I wonder if this should actually be ScalarValue::Boolean(None) (aka a typed boolean null, rather than DataType::Null 🤔 )
There was a problem hiding this comment.
Nice catch, thank you!
Fixed scalar-scalar case to return empty value with data type inherited from first argument, also added .slt for nullif
github-actions
bot
added
core
korowa
force-pushed
the
nullif_extended
branch
from
71ee82c to
bfec2b8
Compare
| @@ -0,0 +1,98 @@ | |||
| # Licensed to the Apache Software Foundation (ASF) under one | |||
|
Benchmark runs are scheduled for baseline = 8ec511e and contender = a8f1f8a. a8f1f8a is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
3 participants
Which issue does this PR close?
Closes #4205 .
Rationale for this change
Adding more options to call
NULLIFin terms of signatures and inputs.What changes are included in this PR?
compute::nullifwithout intermediate type checkingNULLIFargumentsAre these changes tested?
Unit tests for nullif_func added
Are there any user-facing changes?