andygrove commented on code in PR #3796:
URL: https://github.com/apache/arrow-datafusion/pull/3796#discussion_r992505272


##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1148,18 +1150,72 @@ pub struct SubqueryAlias {
 #[derive(Clone)]
 pub struct Filter {
     /// The predicate expression, which must have Boolean type.
-    pub predicate: Expr,
+    predicate: Expr,
     /// The incoming logical plan
-    pub input: Arc<LogicalPlan>,
+    input: Arc<LogicalPlan>,
 }
 
 impl Filter {
+    /// Create a new filter operator.
+    pub fn try_new(
+        predicate: Expr,
+        input: Arc<LogicalPlan>,
+    ) -> datafusion_common::Result<Self> {
+        // Filter predicates must return a boolean value so we try and 
validate that here.
+        // Note that it is not always possible to resolve the predicate 
expression during plan
+        // construction (such as with correlated subqueries) so we make a best 
effort here and
+        // ignore errors resolving the expression against the schema.
+        if let Ok(predicate_type) = predicate.get_type(input.schema()) {
+            if predicate_type != DataType::Boolean {
+                return Err(DataFusionError::Plan(format!(
+                    "Cannot create filter with non-boolean predicate '{}' 
returning {}",
+                    predicate, predicate_type
+                )));
+            }
+        }
+
+        // filter predicates should not contain aliased expressions so we 
remove any aliases here
+        // but perhaps we should be failing here instead?
+        struct RemoveAliases {}
+
+        impl ExprRewriter for RemoveAliases {
+            fn mutate(&mut self, expr: Expr) -> 
datafusion_common::Result<Expr> {
+                match expr {
+                    Expr::Alias(expr, alias) => {
+                        debug!(
+                            "Attempted to create Filter predicate containing 
aliased \
+                            expression `{}` AS '{}'",
+                            expr, alias
+                        );
+                        Ok(expr.as_ref().clone())
+                    }
+                    _ => Ok(expr.clone()),
+                }
+            }
+        }
+
+        let mut remove_aliases = RemoveAliases {};
+        let predicate = predicate.rewrite(&mut remove_aliases)?;
+
+        Ok(Self { predicate, input })
+    }

Review Comment:
   This is the main point of this PR - implementing some validation when 
creating a new filter operator.



-- 
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]

Reply via email to