mingmwang commented on code in PR #5509:
URL: https://github.com/apache/arrow-datafusion/pull/5509#discussion_r1133495404
##########
datafusion/expr/src/utils.rs:
##########
@@ -96,41 +96,7 @@ pub fn expr_to_columns(expr: &Expr, accum: &mut
HashSet<Column>) -> Result<()> {
Expr::ScalarVariable(_, var_names) => {
accum.insert(Column::from_name(var_names.join(".")));
}
- Expr::Alias(_, _)
- | Expr::Literal(_)
- | Expr::BinaryExpr { .. }
- | Expr::Like { .. }
- | Expr::ILike { .. }
- | Expr::SimilarTo { .. }
- | Expr::Not(_)
- | Expr::IsNotNull(_)
- | Expr::IsNull(_)
- | Expr::IsTrue(_)
- | Expr::IsFalse(_)
- | Expr::IsUnknown(_)
- | Expr::IsNotTrue(_)
- | Expr::IsNotFalse(_)
- | Expr::IsNotUnknown(_)
- | Expr::Negative(_)
- | Expr::Between { .. }
- | Expr::Case { .. }
- | Expr::Cast { .. }
- | Expr::TryCast { .. }
- | Expr::Sort { .. }
- | Expr::ScalarFunction { .. }
- | Expr::ScalarUDF { .. }
- | Expr::WindowFunction { .. }
- | Expr::AggregateFunction { .. }
- | Expr::GroupingSet(_)
- | Expr::AggregateUDF { .. }
- | Expr::InList { .. }
- | Expr::Exists { .. }
- | Expr::InSubquery { .. }
- | Expr::ScalarSubquery(_)
- | Expr::Wildcard
- | Expr::QualifiedWildcard { .. }
- | Expr::GetIndexedField { .. }
- | Expr::Placeholder { .. } => {}
+ _ => {}
}
Review Comment:
I would suggest keeping the explicit pattern matching instead of a default
implementation. In future someone might add new Expr types and it is safe to
use explicit pattern matching and force the committer to check all the places.
--
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]
