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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
