andygrove commented on code in PR #3796:
URL: https://github.com/apache/arrow-datafusion/pull/3796#discussion_r992388547
##########
datafusion/expr/src/logical_plan/plan.rs:
##########
@@ -1148,18 +1150,78 @@ pub struct SubqueryAlias {
#[derive(Clone)]
pub struct Filter {
/// The predicate expression, which must have Boolean type.
- pub predicate: Expr,
+ pub(crate) predicate: Expr,
/// The incoming logical plan
- pub input: Arc<LogicalPlan>,
+ pub(crate) 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
+ match predicate.get_type(input.schema()) {
+ Ok(predicate_type) => {
+ if predicate_type != DataType::Boolean {
+ return Err(DataFusionError::Plan(format!(
+ "Cannot create filter with non-boolean predicate '{}'
returning {}",
+ predicate, predicate_type
+ )));
+ }
+ }
+ Err(e) => {
+ // We shouldn't really have to deal with an error case here
but it looks
+ // like we sometimes have plans that are not valid until they
are optimized.
+ // In this case, the optimizer rules will eventually recreate
the
+ // filter expression and we'll do the validation at that time.
+ debug!("Could not determine type of filter predicate: {}", e);
+ }
+ }
+
+ // 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]
