alamb commented on code in PR #2144:
URL: https://github.com/apache/arrow-datafusion/pull/2144#discussion_r842028466
##########
datafusion/physical-expr/src/expressions/not.rs:
##########
@@ -86,10 +86,29 @@ impl PhysicalExpr for NotExpr {
)))
}
ColumnarValue::Scalar(scalar) => {
- let bool_value: bool = scalar.try_into()?;
- Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(
- !bool_value,
- ))))
+ match scalar {
+ ScalarValue::Utf8(utf8_val) => {
+ // NOT op supports `NOT(NULL)` usage that returns
`NULL` directly
+ if utf8_val.is_none() {
+ Ok(ColumnarValue::Scalar(ScalarValue::Utf8(None)))
Review Comment:
I think this may be problematic as the output type of `not()` is supposed to
be `Boolean`. Does returning
```suggestion
Ok(ColumnarValue::Scalar(ScalarValue::Boolean(None)))
```
work?
##########
datafusion/physical-expr/src/expressions/not.rs:
##########
@@ -104,14 +123,14 @@ pub fn not(
arg: Arc<dyn PhysicalExpr>,
input_schema: &Schema,
) -> Result<Arc<dyn PhysicalExpr>> {
- let data_type = arg.data_type(input_schema)?;
- if data_type != DataType::Boolean {
- Err(DataFusionError::Internal(format!(
- "NOT '{:?}' can't be evaluated because the expression's type is
{:?}, not boolean",
+ match arg.data_type(input_schema)? {
+ // NULL is present as literal `ScalarValue::Utf8(None)` in logical
expresion,
+ // to support `NOT(NULL)`, Utf8 arg is passed to `NotExpr` for further
evaluate.
+ DataType::Boolean | DataType::Utf8 => Ok(Arc::new(NotExpr::new(arg))),
Review Comment:
What about detecting if the input is a `ScalarValue` and if that
`ScalarValue::is_null()` returns true (rather than checking for utf8`?
--
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]
