You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/08/17 21:08:04 UTC
[GitHub] [arrow-datafusion] sarahyurick opened a new pull request, #3189: [DRAFT] Support "IS TRUE/FALSE" syntax
sarahyurick opened a new pull request, #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189
# Which issue does this PR close?
Closes #3159
# What changes are included in this PR?
So far, I've added some basic changes so that IS TRUE works on a scalar (`ColumnarValue::Scalar(scalar)`), like `SELECT TRUE IS TRUE as t`, `SELECT FALSE IS TRUE as t`, `SELECT 2 IS TRUE as t`, etc.
**Mainly, I am looking for feedback on how to handle the `ColumnarValue::Array(array)` case.**
Please also let me know if there are any major issues with this implementation so far. Originally, @alamb suggested using `IsNotDistinctFrom`, but I was having trouble figuring out how that was supposed to work. Since we can already parse `IS TRUE` the initial error (without the changes from this PR) is `Error: NotImplemented("Unsupported ast node IsTrue(Identifier(Ident { value: \"b\", quote_style: None })) in sqltorel")`. Whenever I would try to link `IsTrue` to `IsNotDistinctFrom`, I would run into trouble because `IsTrue` looks like `<expr> IS TRUE` while `IsNotDistinctFrom` looks like `<expr> IS NOT DISTINCT FROM <expr>`. Maybe I'm missing something obvious, but the way I decided to go about this implementation was to mimic the implementations for IsNull and IsNotNull, since they don't have a RHS expression either.
Thanks in advance!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: [DRAFT] Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r948424016
##########
datafusion/physical-expr/src/expressions/is_true.rs:
##########
@@ -0,0 +1,140 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS TRUE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::compute; // ?
+use arrow::{
+ array::BooleanArray, // ?
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS TRUE expression
+#[derive(Debug)]
+pub struct IsTrueExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsTrueExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsTrueExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS TRUE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsTrueExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ println!("{:?}", arg);
+ match arg {
+ /*
+ ColumnarValue::Array(array) => Ok(ColumnarValue::Array(Arc::new(
+ compute::is_not_null(array.as_ref())?,
+ ))),
+ */
+ ColumnarValue::Array(array) => {
+ println!("{:?}", array);
+ println!("{}", array.len());
+ // let array_slice = array.slice(1,1);
+ // let my_slice: &dyn arrow::array::Array = &*array_slice;
+ // let my_array = ColumnarValue::Array(array).into_array(3);
+ // let my_data = my_array.data();
+ Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(true)))) // TODO
Review Comment:
This is the main section that I'm still trying to figure out - I'm having trouble grabbing the values from the BooleanArray and doing an "is true" check.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950513732
##########
datafusion/physical-expr/src/expressions/is_false.rs:
##########
@@ -0,0 +1,143 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS FALSE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS FALSE expression
+#[derive(Debug)]
+pub struct IsFalseExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsFalseExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsFalseExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS FALSE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsFalseExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ let array_len = array.len();
+ let my_array = ColumnarValue::Array(array).into_array(array_len);
+ let true_array = BooleanArray::from(vec![Some(true)]);
+ let false_array = BooleanArray::from(vec![Some(false)]);
+ let null_array = BooleanArray::from(vec![None]);
+ let mut result_vec = vec![];
+ for i in 0..array_len {
+ let current = (*my_array).slice(i, 1);
+ if (*current).eq(&false_array) {
+ result_vec.push(Some(true));
+ } else if (*current).eq(&true_array) || (*current).eq(&null_array) {
+ result_vec.push(Some(false));
+ } else {
+ return Err(DataFusionError::Execution(format!("Cannot apply 'IS FALSE' to arguments of type '<{:?}> IS FALSE'. Supported form(s): '<BOOLEAN> IS FALSE'", current.data_type())))
+ }
+ }
+
+ let return_array = BooleanArray::from(result_vec);
+ Ok(ColumnarValue::Array(Arc::new(return_array)))
+ }
+ ColumnarValue::Scalar(scalar) => {
+ match scalar {
+ ScalarValue::Boolean(Some(false)) => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(true)))),
+ ScalarValue::Null => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(false)))),
+ e => return Err(DataFusionError::Execution(format!("Cannot apply 'IS FALSE' to arguments of type '<{}> IS FALSE'. Supported form(s): '<BOOLEAN> IS FALSE'", e.get_datatype())))
+ }
Review Comment:
`Result<bool>` looks appropriate to me here since there is no case that I can see where we would want to return a `None`. Thanks for making these changes.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950529018
##########
datafusion/physical-expr/src/expressions/is_true.rs:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS TRUE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS TRUE expression
+#[derive(Debug)]
+pub struct IsTrueExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsTrueExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsTrueExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS TRUE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsTrueExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ let array_len = array.len();
+ let my_array = ColumnarValue::Array(array).into_array(array_len);
+ let true_array = BooleanArray::from(vec![Some(true)]);
+ let false_array = BooleanArray::from(vec![Some(false)]);
+ let null_array = BooleanArray::from(vec![None]);
+ let mut result_vec = vec![];
+ for i in 0..array_len {
+ let current = (*my_array).slice(i, 1);
+ if (*current).eq(&true_array) {
+ result_vec.push(Some(true));
+ } else if (*current).eq(&false_array) || (*current).eq(&null_array) {
+ result_vec.push(Some(false));
+ } else {
+ return Err(DataFusionError::Execution(format!("Cannot apply 'IS TRUE' to arguments of type '<{:?}> IS TRUE'. Supported form(s): '<BOOLEAN> IS TRUE'", current.data_type())))
+ }
+ }
+
+ let return_array = BooleanArray::from(result_vec);
+ Ok(ColumnarValue::Array(Arc::new(return_array)))
Review Comment:
Ok great, thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: [DRAFT] Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r948428333
##########
datafusion/common/src/scalar.rs:
##########
@@ -726,6 +726,15 @@ impl ScalarValue {
}
}
+ /// whether this value is true or not.
+ pub fn is_true(&self) -> bool {
+ println!("{:?}", &self);
+ match self {
+ ScalarValue::Boolean(v) => v.eq(&Some(true)),
+ _ => false,
Review Comment:
This is how I ended up handling the scalar case.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r951598003
##########
datafusion/sql/src/planner.rs:
##########
@@ -1882,6 +1882,14 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
self.sql_expr_to_logical_expr(*expr, schema, ctes)?,
))),
+ SQLExpr::IsTrue(expr) => Ok(Expr::IsTrue(Box::new(
+ self.sql_expr_to_logical_expr(*expr, schema, ctes)?,
+ ))),
Review Comment:
Okay, I think that makes sense to me. The only thing I was wondering was how this would handle non-boolean/non-null inputs, since we should error for all other datatypes?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r949574542
##########
datafusion/common/src/scalar.rs:
##########
@@ -726,6 +726,24 @@ impl ScalarValue {
}
}
+ /// whether this value is true or not.
+ pub fn is_true(&self) -> bool {
+ match self {
+ ScalarValue::Boolean(v) => v.eq(&Some(true)),
Review Comment:
We can make more use of pattern matching here:
```suggestion
ScalarValue::Boolean(&Some(true)) => true,
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#issuecomment-1219755117
@sarahyurick Could you target this to the `sqlparser-0.21` branch? We'll need to merge PRs there in the order specified in https://github.com/apache/arrow-datafusion/issues/3192
Also, see discussion on this feature branch approach in https://github.com/apache/arrow-datafusion/pull/3191
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#issuecomment-1218968134
@sarahyurick I tried some `expr IS TRUE` type queries in Postgres and it only supports the case where `expr` is a boolean type. Does Dask SQL have different behavior to this?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#issuecomment-1219708683
> @sarahyurick I tried some `expr IS TRUE` type queries in Postgres and it only supports the case where `expr` is a boolean type. Does Dask SQL have different behavior to this?
Good point! Dask SQL main currently throws a `Cannot apply 'IS TRUE' to arguments of type '<BIGINT> IS TRUE'. Supported form(s): '<BOOLEAN> IS TRUE'` for all other data types, so it would probably be best to match this.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on pull request #3189: [sqlparser-0.21] Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#issuecomment-1219879150
IS TRUE and IS FALSE should both be fully functional. I'm not sure what's going on with the proto files, though.
After those are fixed, it should just be formatting checks and tests, if any.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick closed pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick closed pull request #3189: Support "IS TRUE/FALSE" syntax
URL: https://github.com/apache/arrow-datafusion/pull/3189
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r949565366
##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -312,6 +312,10 @@ message LogicalExprNode {
CubeNode cube = 23;
RollupNode rollup = 24;
+
+ // boolean operations
+ IsTrue is_true = 25;
+ IsFalse is_false = 26;
Review Comment:
There is a mismatch between these names and the names used in the code. This change fixed the proto errors for me.
```suggestion
IsTrue is_true_expr = 25;
IsFalse is_false_expr = 26;
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r949568622
##########
datafusion/proto/proto/datafusion.proto:
##########
@@ -312,6 +312,10 @@ message LogicalExprNode {
CubeNode cube = 23;
RollupNode rollup = 24;
+
+ // boolean operations
+ IsTrue is_true = 25;
+ IsFalse is_false = 26;
Review Comment:
Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950526104
##########
datafusion/physical-expr/src/expressions/is_false.rs:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS FALSE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS FALSE expression
+#[derive(Debug)]
+pub struct IsFalseExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsFalseExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsFalseExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS FALSE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsFalseExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
Review Comment:
See my comments for `IsTrue`. We would want to make similar changes here.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950519839
##########
datafusion/physical-expr/src/expressions/is_true.rs:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS TRUE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS TRUE expression
+#[derive(Debug)]
+pub struct IsTrueExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsTrueExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsTrueExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS TRUE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsTrueExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ let array_len = array.len();
+ let my_array = ColumnarValue::Array(array).into_array(array_len);
+ let true_array = BooleanArray::from(vec![Some(true)]);
+ let false_array = BooleanArray::from(vec![Some(false)]);
+ let null_array = BooleanArray::from(vec![None]);
+ let mut result_vec = vec![];
+ for i in 0..array_len {
+ let current = (*my_array).slice(i, 1);
+ if (*current).eq(&true_array) {
+ result_vec.push(Some(true));
+ } else if (*current).eq(&false_array) || (*current).eq(&null_array) {
+ result_vec.push(Some(false));
+ } else {
+ return Err(DataFusionError::Execution(format!("Cannot apply 'IS TRUE' to arguments of type '<{:?}> IS TRUE'. Supported form(s): '<BOOLEAN> IS TRUE'", current.data_type())))
+ }
+ }
+
+ let return_array = BooleanArray::from(result_vec);
+ Ok(ColumnarValue::Array(Arc::new(return_array)))
Review Comment:
Here is my suggested approach. The key changes are:
- It is more efficient to use an Arrow array builder, rather than build a vec and then convert it into an Arrow array
- We can just use `arary.value(i)` to get individual values now that we are downcasting to a `BinaryArray`
```suggestion
match array.as_any().downcast_ref::<BooleanArray>() {
Some(bool_array) => {
let array_len = array.len();
let mut result_builder = BooleanBuilder::new(array_len);
for i in 0..array_len {
result_builder.append_value(!bool_array.is_null(i) && !bool_array.value(i));
}
Ok(ColumnarValue::Array(Arc::new(result_builder.finish())))
}
_ => Err(DataFusionError::Execution(format!("Cannot apply 'IS FALSE' to arguments of type '<{:?}> IS FALSE'. Supported form(s): '<BOOLEAN> IS FALSE'", array.data_type())))
}
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950525011
##########
datafusion/physical-expr/src/expressions/is_true.rs:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS TRUE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS TRUE expression
+#[derive(Debug)]
+pub struct IsTrueExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsTrueExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsTrueExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS TRUE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsTrueExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ let array_len = array.len();
+ let my_array = ColumnarValue::Array(array).into_array(array_len);
+ let true_array = BooleanArray::from(vec![Some(true)]);
+ let false_array = BooleanArray::from(vec![Some(false)]);
+ let null_array = BooleanArray::from(vec![None]);
+ let mut result_vec = vec![];
+ for i in 0..array_len {
+ let current = (*my_array).slice(i, 1);
+ if (*current).eq(&true_array) {
+ result_vec.push(Some(true));
+ } else if (*current).eq(&false_array) || (*current).eq(&null_array) {
+ result_vec.push(Some(false));
+ } else {
+ return Err(DataFusionError::Execution(format!("Cannot apply 'IS TRUE' to arguments of type '<{:?}> IS TRUE'. Supported form(s): '<BOOLEAN> IS TRUE'", current.data_type())))
+ }
+ }
+
+ let return_array = BooleanArray::from(result_vec);
+ Ok(ColumnarValue::Array(Arc::new(return_array)))
Review Comment:
There is probably an even more efficient way of doing this with an arrow kernel. I'll leave it for others to make suggestions about that since I am less familiar at this level.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950828871
##########
datafusion/physical-expr/src/expressions/is_false.rs:
##########
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS FALSE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray, BooleanBuilder},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS FALSE expression
+#[derive(Debug)]
+pub struct IsFalseExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsFalseExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsFalseExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS FALSE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsFalseExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ match array.as_any().downcast_ref::<BooleanArray>() {
+ Some(bool_array) => {
+ let array_len = array.len();
+ let mut result_builder = BooleanBuilder::new(array_len);
+ for i in 0..array_len {
+ result_builder.append_value(!bool_array.is_null(i) && !bool_array.value(i));
+ }
+ Ok(ColumnarValue::Array(Arc::new(result_builder.finish())))
Review Comment:
You can probably do the same thing with less code (and more efficiently) using the `FromIter` method:
```suggestion
let result: BooleanArray = bool_array.iter()
.map(|v| v.map(|v| !v).or(Some(false)))
.collect();
```
##########
datafusion/sql/src/planner.rs:
##########
@@ -1882,6 +1882,14 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
self.sql_expr_to_logical_expr(*expr, schema, ctes)?,
))),
+ SQLExpr::IsTrue(expr) => Ok(Expr::IsTrue(Box::new(
+ self.sql_expr_to_logical_expr(*expr, schema, ctes)?,
+ ))),
Review Comment:
I think you might be able to avoid having to extend `Expr` and add a `PhysicalExpr` at all if you rewrote `IS TRUE` in the sql planner to `IS NOT DISTINCT FROM`
Something like
```suggestion
SQLExpr::IsTrue(expr) => Ok(Expr::BinaryExpr {
left: Box::new(self.sql_expr_to_logical_expr(*expr, schema, ctes)?),
op: Operator::IsNotDistinctFrom,
right: Box::new(lit(true)),
}),
```
That way a query like `SELECT x IS TRUE` becomes `SELECT x IS NOT DISTINCT FROM TRUE`
Here is an example from postgres showing they are equivalent:
```sql
alamb=# select column1 is true, column1 is not distinct from true from (values (true), (false), (null)) as sq;
?column? | ?column?
----------+----------
t | t
f | f
f | f
(3 rows)
```
The same type of transformation applies to `IS FALSE`
Then this PR would likely be 8 lines of code and then the sql level tests
##########
datafusion/physical-expr/src/expressions/is_false.rs:
##########
@@ -0,0 +1,127 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS FALSE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray, BooleanBuilder},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS FALSE expression
+#[derive(Debug)]
+pub struct IsFalseExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsFalseExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsFalseExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS FALSE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsFalseExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ match array.as_any().downcast_ref::<BooleanArray>() {
+ Some(bool_array) => {
+ let array_len = array.len();
+ let mut result_builder = BooleanBuilder::new(array_len);
+ for i in 0..array_len {
+ result_builder.append_value(!bool_array.is_null(i) && !bool_array.value(i));
+ }
+ Ok(ColumnarValue::Array(Arc::new(result_builder.finish())))
Review Comment:
(same comment applies to is_true.rs)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r949622961
##########
datafusion/common/src/scalar.rs:
##########
@@ -726,6 +726,24 @@ impl ScalarValue {
}
}
+ /// whether this value is true or not.
+ pub fn is_true(&self) -> bool {
+ match self {
+ ScalarValue::Boolean(v) => v.eq(&Some(true)),
+ ScalarValue::Null => false,
+ other => panic!("Cannot apply 'IS TRUE' to arguments of type '{} IS TRUE'. Supported form(s): '<BOOLEAN> IS TRUE'", other)
Review Comment:
Okay, let me know if my changes in `is_true.rs` and `is_false.rs` are what you had in mind.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r951524119
##########
datafusion/sql/src/planner.rs:
##########
@@ -1882,6 +1882,14 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
self.sql_expr_to_logical_expr(*expr, schema, ctes)?,
))),
+ SQLExpr::IsTrue(expr) => Ok(Expr::IsTrue(Box::new(
+ self.sql_expr_to_logical_expr(*expr, schema, ctes)?,
+ ))),
Review Comment:
My preference would be to keep the logical plan as close to the original query as possible. Different query engines will have different approaches for how to implement this in the physical plan. I also think that it is better UX for the user to see IsTrue/False in the logical plan if that's what their query contained.
I think it is a good idea for DataFusion to map the logical IsTrue /False to the physical expression IsDistinctFrom and that removes the need for the new physical expressions here.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: [DRAFT] Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r948424016
##########
datafusion/physical-expr/src/expressions/is_true.rs:
##########
@@ -0,0 +1,140 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS TRUE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::compute; // ?
+use arrow::{
+ array::BooleanArray, // ?
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS TRUE expression
+#[derive(Debug)]
+pub struct IsTrueExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsTrueExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsTrueExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS TRUE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsTrueExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ println!("{:?}", arg);
+ match arg {
+ /*
+ ColumnarValue::Array(array) => Ok(ColumnarValue::Array(Arc::new(
+ compute::is_not_null(array.as_ref())?,
+ ))),
+ */
+ ColumnarValue::Array(array) => {
+ println!("{:?}", array);
+ println!("{}", array.len());
+ // let array_slice = array.slice(1,1);
+ // let my_slice: &dyn arrow::array::Array = &*array_slice;
+ // let my_array = ColumnarValue::Array(array).into_array(3);
+ // let my_data = my_array.data();
+ Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(true)))) // TODO
Review Comment:
This is the main section that I'm still trying to figure out - I'm having trouble grabbing the values from the BooleanArray and doing an "is true" check for each value. It looks like for `is_not_null` (which I modeled this file after), a built-in function was able to be used, but I'm not really sure the best way to go about it for this case.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
alamb commented on PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#issuecomment-1232842274
BTW @sarahyurick -- thank you for sticking with this. It is great to see the functionality coming along so nicely 👌
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r949578951
##########
datafusion/common/src/scalar.rs:
##########
@@ -726,6 +726,24 @@ impl ScalarValue {
}
}
+ /// whether this value is true or not.
+ pub fn is_true(&self) -> bool {
+ match self {
+ ScalarValue::Boolean(v) => v.eq(&Some(true)),
+ ScalarValue::Null => false,
+ other => panic!("Cannot apply 'IS TRUE' to arguments of type '{} IS TRUE'. Supported form(s): '<BOOLEAN> IS TRUE'", other)
Review Comment:
This code is called from `IsTrueExpr.evaluate`, which returns a `Result`. I think we should move this logic out of `ScalarValue` and have it in `IsTrueExpr` so that we can return an `Err` and avoid a `panic!`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950091118
##########
datafusion/physical-expr/src/expressions/is_false.rs:
##########
@@ -0,0 +1,143 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS FALSE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS FALSE expression
+#[derive(Debug)]
+pub struct IsFalseExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsFalseExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsFalseExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS FALSE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsFalseExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ let array_len = array.len();
+ let my_array = ColumnarValue::Array(array).into_array(array_len);
+ let true_array = BooleanArray::from(vec![Some(true)]);
+ let false_array = BooleanArray::from(vec![Some(false)]);
+ let null_array = BooleanArray::from(vec![None]);
+ let mut result_vec = vec![];
+ for i in 0..array_len {
+ let current = (*my_array).slice(i, 1);
+ if (*current).eq(&false_array) {
+ result_vec.push(Some(true));
+ } else if (*current).eq(&true_array) || (*current).eq(&null_array) {
+ result_vec.push(Some(false));
+ } else {
+ return Err(DataFusionError::Execution(format!("Cannot apply 'IS FALSE' to arguments of type '<{:?}> IS FALSE'. Supported form(s): '<BOOLEAN> IS FALSE'", current.data_type())))
+ }
+ }
+
+ let return_array = BooleanArray::from(result_vec);
+ Ok(ColumnarValue::Array(Arc::new(return_array)))
+ }
+ ColumnarValue::Scalar(scalar) => {
+ match scalar {
+ ScalarValue::Boolean(Some(false)) => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(true)))),
+ ScalarValue::Null => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(false)))),
+ e => return Err(DataFusionError::Execution(format!("Cannot apply 'IS FALSE' to arguments of type '<{}> IS FALSE'. Supported form(s): '<BOOLEAN> IS FALSE'", e.get_datatype())))
+ }
Review Comment:
Thanks for making the change to return `Result`. I realize now that we could have left this in `ScalarValue` and just changed the method signature there to return `Result` as well. Moving it back to `ScalarValue` would make this logic more easily re-usable from Dask SQL. Sorry to miss that in my previous review.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r949386462
##########
datafusion/physical-expr/src/expressions/is_true.rs:
##########
@@ -0,0 +1,140 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS TRUE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::compute; // ?
+use arrow::{
+ array::BooleanArray, // ?
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS TRUE expression
+#[derive(Debug)]
+pub struct IsTrueExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsTrueExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsTrueExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS TRUE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsTrueExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ println!("{:?}", arg);
+ match arg {
+ /*
+ ColumnarValue::Array(array) => Ok(ColumnarValue::Array(Arc::new(
+ compute::is_not_null(array.as_ref())?,
+ ))),
+ */
+ ColumnarValue::Array(array) => {
+ println!("{:?}", array);
+ println!("{}", array.len());
+ // let array_slice = array.slice(1,1);
+ // let my_slice: &dyn arrow::array::Array = &*array_slice;
+ // let my_array = ColumnarValue::Array(array).into_array(3);
+ // let my_data = my_array.data();
+ Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(true)))) // TODO
Review Comment:
At this point, `array` is an `Arc<(dyn arrow::array::Array + 'static)>`, which I'm having trouble working with.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] codecov-commenter commented on pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#issuecomment-1219958251
# [Codecov](https://codecov.io/gh/apache/arrow-datafusion/pull/3189?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3189](https://codecov.io/gh/apache/arrow-datafusion/pull/3189?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (efbc98b) into [master](https://codecov.io/gh/apache/arrow-datafusion/commit/929eb6d860fb60ba994b24397ad3c3eb7d839cdf?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (929eb6d) will **decrease** coverage by `0.21%`.
> The diff coverage is `0.77%`.
> :exclamation: Current head efbc98b differs from pull request most recent head 450f776. Consider uploading reports for the commit 450f776 to get more accurate results
```diff
@@ Coverage Diff @@
## master #3189 +/- ##
==========================================
- Coverage 85.87% 85.66% -0.22%
==========================================
Files 291 293 +2
Lines 52885 53013 +128
==========================================
- Hits 45415 45413 -2
- Misses 7470 7600 +130
```
| [Impacted Files](https://codecov.io/gh/apache/arrow-datafusion/pull/3189?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [datafusion/common/src/scalar.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9jb21tb24vc3JjL3NjYWxhci5ycw==) | `84.18% <0.00%> (-0.56%)` | :arrow_down: |
| [datafusion/core/src/datasource/listing/helpers.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9kYXRhc291cmNlL2xpc3RpbmcvaGVscGVycy5ycw==) | `95.01% <ø> (ø)` | |
| [datafusion/core/src/physical\_plan/planner.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9waHlzaWNhbF9wbGFuL3BsYW5uZXIucnM=) | `80.55% <0.00%> (-0.49%)` | :arrow_down: |
| [datafusion/expr/src/expr.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy9leHByLnJz) | `81.76% <0.00%> (-3.64%)` | :arrow_down: |
| [datafusion/expr/src/expr\_rewriter.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy9leHByX3Jld3JpdGVyLnJz) | `85.34% <0.00%> (-0.75%)` | :arrow_down: |
| [datafusion/expr/src/expr\_visitor.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy9leHByX3Zpc2l0b3IucnM=) | `62.19% <0.00%> (-1.56%)` | :arrow_down: |
| [datafusion/expr/src/utils.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy91dGlscy5ycw==) | `90.76% <ø> (ø)` | |
| [...tafusion/optimizer/src/common\_subexpr\_eliminate.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9vcHRpbWl6ZXIvc3JjL2NvbW1vbl9zdWJleHByX2VsaW1pbmF0ZS5ycw==) | `93.81% <0.00%> (-0.49%)` | :arrow_down: |
| [datafusion/optimizer/src/simplify\_expressions.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9vcHRpbWl6ZXIvc3JjL3NpbXBsaWZ5X2V4cHJlc3Npb25zLnJz) | `83.76% <0.00%> (-0.18%)` | :arrow_down: |
| [...tafusion/physical-expr/src/expressions/is\_false.rs](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZGF0YWZ1c2lvbi9waHlzaWNhbC1leHByL3NyYy9leHByZXNzaW9ucy9pc19mYWxzZS5ycw==) | `0.00% <0.00%> (ø)` | |
| ... and [10 more](https://codecov.io/gh/apache/arrow-datafusion/pull/3189/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950520533
##########
datafusion/physical-expr/src/expressions/is_true.rs:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS TRUE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS TRUE expression
+#[derive(Debug)]
+pub struct IsTrueExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsTrueExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsTrueExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS TRUE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsTrueExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ let array_len = array.len();
+ let my_array = ColumnarValue::Array(array).into_array(array_len);
+ let true_array = BooleanArray::from(vec![Some(true)]);
+ let false_array = BooleanArray::from(vec![Some(false)]);
+ let null_array = BooleanArray::from(vec![None]);
+ let mut result_vec = vec![];
+ for i in 0..array_len {
+ let current = (*my_array).slice(i, 1);
+ if (*current).eq(&true_array) {
+ result_vec.push(Some(true));
+ } else if (*current).eq(&false_array) || (*current).eq(&null_array) {
+ result_vec.push(Some(false));
+ } else {
+ return Err(DataFusionError::Execution(format!("Cannot apply 'IS TRUE' to arguments of type '<{:?}> IS TRUE'. Supported form(s): '<BOOLEAN> IS TRUE'", current.data_type())))
+ }
+ }
+
+ let return_array = BooleanArray::from(result_vec);
+ Ok(ColumnarValue::Array(Arc::new(return_array)))
+ }
+ ColumnarValue::Scalar(scalar) => Ok(ColumnarValue::Scalar(
+ ScalarValue::Boolean(Some(scalar.is_true().unwrap())),
Review Comment:
We want to return an `Err` result rather than panic if `is_true` returns an error.
```suggestion
ScalarValue::Boolean(Some(scalar.is_true()?)),
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950516458
##########
datafusion/physical-expr/src/expressions/is_true.rs:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS TRUE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS TRUE expression
+#[derive(Debug)]
+pub struct IsTrueExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsTrueExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsTrueExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS TRUE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsTrueExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
Review Comment:
I am reviewing this part now and will post some suggested changes later today
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950380329
##########
datafusion/physical-expr/src/expressions/is_false.rs:
##########
@@ -0,0 +1,143 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS FALSE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS FALSE expression
+#[derive(Debug)]
+pub struct IsFalseExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsFalseExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsFalseExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS FALSE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsFalseExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ let array_len = array.len();
+ let my_array = ColumnarValue::Array(array).into_array(array_len);
+ let true_array = BooleanArray::from(vec![Some(true)]);
+ let false_array = BooleanArray::from(vec![Some(false)]);
+ let null_array = BooleanArray::from(vec![None]);
+ let mut result_vec = vec![];
+ for i in 0..array_len {
+ let current = (*my_array).slice(i, 1);
+ if (*current).eq(&false_array) {
+ result_vec.push(Some(true));
+ } else if (*current).eq(&true_array) || (*current).eq(&null_array) {
+ result_vec.push(Some(false));
+ } else {
+ return Err(DataFusionError::Execution(format!("Cannot apply 'IS FALSE' to arguments of type '<{:?}> IS FALSE'. Supported form(s): '<BOOLEAN> IS FALSE'", current.data_type())))
+ }
+ }
+
+ let return_array = BooleanArray::from(result_vec);
+ Ok(ColumnarValue::Array(Arc::new(return_array)))
+ }
+ ColumnarValue::Scalar(scalar) => {
+ match scalar {
+ ScalarValue::Boolean(Some(false)) => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(true)))),
+ ScalarValue::Null => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(false)))),
+ e => return Err(DataFusionError::Execution(format!("Cannot apply 'IS FALSE' to arguments of type '<{}> IS FALSE'. Supported form(s): '<BOOLEAN> IS FALSE'", e.get_datatype())))
+ }
Review Comment:
Ok cool, I've moved it back to `scalar.rs`. One quick question - would it be better to have the return type a `Result<bool>` or a `Result<Option<bool>>`? Right now I'm doing the former.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950380329
##########
datafusion/physical-expr/src/expressions/is_false.rs:
##########
@@ -0,0 +1,143 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS FALSE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS FALSE expression
+#[derive(Debug)]
+pub struct IsFalseExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsFalseExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsFalseExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS FALSE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsFalseExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ let array_len = array.len();
+ let my_array = ColumnarValue::Array(array).into_array(array_len);
+ let true_array = BooleanArray::from(vec![Some(true)]);
+ let false_array = BooleanArray::from(vec![Some(false)]);
+ let null_array = BooleanArray::from(vec![None]);
+ let mut result_vec = vec![];
+ for i in 0..array_len {
+ let current = (*my_array).slice(i, 1);
+ if (*current).eq(&false_array) {
+ result_vec.push(Some(true));
+ } else if (*current).eq(&true_array) || (*current).eq(&null_array) {
+ result_vec.push(Some(false));
+ } else {
+ return Err(DataFusionError::Execution(format!("Cannot apply 'IS FALSE' to arguments of type '<{:?}> IS FALSE'. Supported form(s): '<BOOLEAN> IS FALSE'", current.data_type())))
+ }
+ }
+
+ let return_array = BooleanArray::from(result_vec);
+ Ok(ColumnarValue::Array(Arc::new(return_array)))
+ }
+ ColumnarValue::Scalar(scalar) => {
+ match scalar {
+ ScalarValue::Boolean(Some(false)) => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(true)))),
+ ScalarValue::Null => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(false)))),
+ e => return Err(DataFusionError::Execution(format!("Cannot apply 'IS FALSE' to arguments of type '<{}> IS FALSE'. Supported form(s): '<BOOLEAN> IS FALSE'", e.get_datatype())))
+ }
Review Comment:
Ok cool, I've moved it back to `scalar.rs`. One quick question - would it be better to have the return type a `Result<bool>` or a `Result<Option<bool>>` (or something else)? Right now I'm doing the former.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r949623739
##########
datafusion/physical-expr/src/expressions/is_false.rs:
##########
@@ -0,0 +1,110 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS FALSE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS FALSE expression
+#[derive(Debug)]
+pub struct IsFalseExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsFalseExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsFalseExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS FALSE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsFalseExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ let array_len = array.len();
+ let my_array = ColumnarValue::Array(array).into_array(array_len);
+ let true_array = BooleanArray::from(vec![Some(true)]);
+ let false_array = BooleanArray::from(vec![Some(false)]);
+ let null_array = BooleanArray::from(vec![None]);
+ let mut result_vec = vec![];
+ for i in 0..array_len {
+ let current = (*my_array).slice(i, 1);
+ if (*current).eq(&false_array) {
+ result_vec.push(Some(true));
+ } else if (*current).eq(&true_array) || (*current).eq(&null_array) {
+ result_vec.push(Some(false));
+ } else {
+ return Err(DataFusionError::Execution("Cannot apply 'IS FALSE' to argument type. Supported form(s): '<BOOLEAN> IS FALSE'".to_string()))
+ }
+ }
+
+ let return_array = BooleanArray::from(result_vec);
+ Ok(ColumnarValue::Array(Arc::new(return_array)))
+ }
+ ColumnarValue::Scalar(scalar) => {
+ match scalar {
+ ScalarValue::Boolean(Some(false)) => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(true)))),
+ ScalarValue::Null => Ok(ColumnarValue::Scalar(ScalarValue::Boolean(Some(false)))),
+ _ => return Err(DataFusionError::Execution("Cannot apply 'IS FALSE' to argument type. Supported form(s): '<BOOLEAN> IS FALSE'".to_string()))
Review Comment:
Instead of `panic!`
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950519839
##########
datafusion/physical-expr/src/expressions/is_true.rs:
##########
@@ -0,0 +1,139 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! IS TRUE expression
+
+use std::{any::Any, sync::Arc};
+
+use crate::PhysicalExpr;
+use arrow::{
+ array::{Array, BooleanArray},
+ datatypes::{DataType, Schema},
+ record_batch::RecordBatch,
+};
+use datafusion_common::DataFusionError;
+use datafusion_common::Result;
+use datafusion_common::ScalarValue;
+use datafusion_expr::ColumnarValue;
+
+/// IS TRUE expression
+#[derive(Debug)]
+pub struct IsTrueExpr {
+ /// The input expression
+ arg: Arc<dyn PhysicalExpr>,
+}
+
+impl IsTrueExpr {
+ /// Create new not expression
+ pub fn new(arg: Arc<dyn PhysicalExpr>) -> Self {
+ Self { arg }
+ }
+
+ /// Get the input expression
+ pub fn arg(&self) -> &Arc<dyn PhysicalExpr> {
+ &self.arg
+ }
+}
+
+impl std::fmt::Display for IsTrueExpr {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ write!(f, "{} IS TRUE", self.arg)
+ }
+}
+
+impl PhysicalExpr for IsTrueExpr {
+ /// Return a reference to Any that can be used for downcasting
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+
+ fn data_type(&self, _input_schema: &Schema) -> Result<DataType> {
+ Ok(DataType::Boolean)
+ }
+
+ fn nullable(&self, _input_schema: &Schema) -> Result<bool> {
+ Ok(false)
+ }
+
+ fn evaluate(&self, batch: &RecordBatch) -> Result<ColumnarValue> {
+ let arg = self.arg.evaluate(batch)?;
+ match arg {
+ ColumnarValue::Array(array) => {
+ let array_len = array.len();
+ let my_array = ColumnarValue::Array(array).into_array(array_len);
+ let true_array = BooleanArray::from(vec![Some(true)]);
+ let false_array = BooleanArray::from(vec![Some(false)]);
+ let null_array = BooleanArray::from(vec![None]);
+ let mut result_vec = vec![];
+ for i in 0..array_len {
+ let current = (*my_array).slice(i, 1);
+ if (*current).eq(&true_array) {
+ result_vec.push(Some(true));
+ } else if (*current).eq(&false_array) || (*current).eq(&null_array) {
+ result_vec.push(Some(false));
+ } else {
+ return Err(DataFusionError::Execution(format!("Cannot apply 'IS TRUE' to arguments of type '<{:?}> IS TRUE'. Supported form(s): '<BOOLEAN> IS TRUE'", current.data_type())))
+ }
+ }
+
+ let return_array = BooleanArray::from(result_vec);
+ Ok(ColumnarValue::Array(Arc::new(return_array)))
Review Comment:
Here is my suggested approach. The key changes are:
- It is more efficient to use an Arrow array builder, rather than build a vec and then convert it into an Arrow array
- We can just use `arary.value(i)` to get individual values now that we are downcasting to a `BinaryArray`
```suggestion
match array.as_any().downcast_ref::<BooleanArray>() {
Some(bool_array) => {
let array_len = array.len();
let mut result_builder = BooleanBuilder::new(array_len);
for i in 0..array_len {
result_builder.append_value(!bool_array.is_null(i) && bool_array.value(i));
}
Ok(ColumnarValue::Array(Arc::new(result_builder.finish())))
}
_ => Err(DataFusionError::Execution(format!("Cannot apply 'IS FALSE' to arguments of type '<{:?}> IS FALSE'. Supported form(s): '<BOOLEAN> IS FALSE'", array.data_type())))
}
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r949574542
##########
datafusion/common/src/scalar.rs:
##########
@@ -726,6 +726,24 @@ impl ScalarValue {
}
}
+ /// whether this value is true or not.
+ pub fn is_true(&self) -> bool {
+ match self {
+ ScalarValue::Boolean(v) => v.eq(&Some(true)),
Review Comment:
We can make more use of pattern matching here:
```suggestion
ScalarValue::Boolean(Some(true)) => true,
```
##########
datafusion/common/src/scalar.rs:
##########
@@ -726,6 +726,24 @@ impl ScalarValue {
}
}
+ /// whether this value is true or not.
+ pub fn is_true(&self) -> bool {
+ match self {
+ ScalarValue::Boolean(v) => v.eq(&Some(true)),
+ ScalarValue::Null => false,
+ other => panic!("Cannot apply 'IS TRUE' to arguments of type '{} IS TRUE'. Supported form(s): '<BOOLEAN> IS TRUE'", other)
+ }
+ }
+
+ /// whether this value is false or not.
+ pub fn is_false(&self) -> bool {
+ match self {
+ ScalarValue::Boolean(v) => v.eq(&Some(false)),
Review Comment:
```suggestion
ScalarValue::Boolean(Some(false)) => true,
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r949575221
##########
datafusion/common/src/scalar.rs:
##########
@@ -726,6 +726,24 @@ impl ScalarValue {
}
}
+ /// whether this value is true or not.
+ pub fn is_true(&self) -> bool {
+ match self {
+ ScalarValue::Boolean(v) => v.eq(&Some(true)),
+ ScalarValue::Null => false,
+ other => panic!("Cannot apply 'IS TRUE' to arguments of type '{} IS TRUE'. Supported form(s): '<BOOLEAN> IS TRUE'", other)
+ }
+ }
+
+ /// whether this value is false or not.
+ pub fn is_false(&self) -> bool {
+ match self {
+ ScalarValue::Boolean(v) => v.eq(&Some(false)),
Review Comment:
```suggestion
ScalarValue::Boolean(&Some(false)) => true,
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] sarahyurick commented on pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
sarahyurick commented on PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#issuecomment-1220080662
Should I also add tests in `proto/src/lib.rs`? I'm not quite sure what those would look like.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r950827446
##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -183,7 +185,11 @@ impl ExprSchemable for Expr {
| Expr::WindowFunction { .. }
| Expr::AggregateFunction { .. }
| Expr::AggregateUDF { .. } => Ok(true),
- Expr::IsNull(_) | Expr::IsNotNull(_) | Expr::Exists { .. } => Ok(false),
+ Expr::IsNull(_)
+ | Expr::IsNotNull(_)
+ | Expr::IsTrue(_)
+ | Expr::IsFalse(_)
+ | Expr::Exists { .. } => Ok(false),
Review Comment:
👍
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] andygrove commented on a diff in pull request #3189: Support "IS TRUE/FALSE" syntax
Posted by GitBox <gi...@apache.org>.
andygrove commented on code in PR #3189:
URL: https://github.com/apache/arrow-datafusion/pull/3189#discussion_r951814941
##########
datafusion/sql/src/planner.rs:
##########
@@ -1882,6 +1882,14 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
self.sql_expr_to_logical_expr(*expr, schema, ctes)?,
))),
+ SQLExpr::IsTrue(expr) => Ok(Expr::IsTrue(Box::new(
+ self.sql_expr_to_logical_expr(*expr, schema, ctes)?,
+ ))),
Review Comment:
Yes, that is related to the work I am starting in https://github.com/apache/arrow-datafusion/pull/3222. I think we need to do some type validation here so that if we see `<expr> IS TRUE` where `expr` is not boolean, then we either add a cast or throw an error if it cannot be cast to boolean.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org