Skip to content

Commit

Permalink
remove Expr::Nop, define Expr::Literal(ScalarValue::Null) as defa…
Browse files Browse the repository at this point in the history
…ult for `Expr`
  • Loading branch information
peter-toth committed Dec 20, 2023
1 parent aa333d1 commit 6cd5d39
Show file tree
Hide file tree
Showing 9 changed files with 11 additions and 26 deletions.
3 changes: 1 addition & 2 deletions datafusion/core/src/datasource/listing/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ pub fn expr_applicable_for_cols(col_names: &[String], expr: &Expr) -> bool {
Ok(TreeNodeRecursion::Stop)
}
}
Expr::Nop
| Expr::Literal(_)
Expr::Literal(_)
| Expr::Alias(_)
| Expr::OuterReferenceColumn(_, _)
| Expr::ScalarVariable(_, _)
Expand Down
3 changes: 0 additions & 3 deletions datafusion/core/src/physical_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,9 +381,6 @@ fn create_physical_name(e: &Expr, is_first_expr: bool) -> Result<String> {
Expr::OuterReferenceColumn(_, _) => {
internal_err!("Create physical name does not support OuterReferenceColumn")
}
Expr::Nop => {
internal_err!("Create physical name does not support Nop expression")
}
}
}

Expand Down
13 changes: 7 additions & 6 deletions datafusion/expr/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,8 @@ use std::{fmt, mem};
/// assert_eq!(binary_expr.op, Operator::Eq);
/// }
/// ```
#[derive(Clone, PartialEq, Eq, Hash, Debug, Default)]
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub enum Expr {
#[default]
Nop,
/// An expression with a specific name.
Alias(Alias),
/// A named reference to a qualified filed in a schema.
Expand Down Expand Up @@ -181,6 +179,12 @@ pub enum Expr {
OuterReferenceColumn(DataType, Column),
}

impl Default for Expr {
fn default() -> Self {
Expr::Literal(ScalarValue::Null)
}
}

/// Alias expression
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
pub struct Alias {
Expand Down Expand Up @@ -786,7 +790,6 @@ impl Expr {
/// Useful for non-rust based bindings
pub fn variant_name(&self) -> &str {
match self {
Expr::Nop { .. } => "Nop",
Expr::AggregateFunction { .. } => "AggregateFunction",
Expr::Alias(..) => "Alias",
Expr::Between { .. } => "Between",
Expand Down Expand Up @@ -1207,7 +1210,6 @@ macro_rules! expr_vec_fmt {
impl fmt::Display for Expr {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Expr::Nop => write!(f, "NOP"),
Expr::Alias(Alias { expr, name, .. }) => write!(f, "{expr} AS {name}"),
Expr::Column(c) => write!(f, "{c}"),
Expr::OuterReferenceColumn(_, c) => write!(f, "outer_ref({c})"),
Expand Down Expand Up @@ -1450,7 +1452,6 @@ fn create_function_name(fun: &str, distinct: bool, args: &[Expr]) -> Result<Stri
/// This function recursively transverses the expression for names such as "CAST(a > 2)".
fn create_name(e: &Expr) -> Result<String> {
match e {
Expr::Nop => Ok("NOP".to_string()),
Expr::Alias(Alias { name, .. }) => Ok(name.clone()),
Expr::Column(c) => Ok(c.flat_name()),
Expr::OuterReferenceColumn(_, c) => Ok(format!("outer_ref({})", c.flat_name())),
Expand Down
4 changes: 1 addition & 3 deletions datafusion/expr/src/expr_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ impl ExprSchemable for Expr {
/// (e.g. `[utf8] + [bool]`).
fn get_type<S: ExprSchema>(&self, schema: &S) -> Result<DataType> {
match self {
Expr::Nop => Ok(DataType::Null),
Expr::Alias(Alias { expr, name, .. }) => match &**expr {
Expr::Placeholder(Placeholder { data_type, .. }) => match &data_type {
None => schema.data_type(&Column::from_name(name)).cloned(),
Expand Down Expand Up @@ -252,8 +251,7 @@ impl ExprSchemable for Expr {
| Expr::WindowFunction { .. }
| Expr::AggregateFunction { .. }
| Expr::Placeholder(_) => Ok(true),
Expr::Nop
| Expr::IsNull(_)
Expr::IsNull(_)
| Expr::IsNotNull(_)
| Expr::IsTrue(_)
| Expr::IsFalse(_)
Expand Down
3 changes: 0 additions & 3 deletions datafusion/expr/src/tree_node/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ impl TreeNode for Expr {
Expr::GroupingSet(GroupingSet::GroupingSets(lists_of_exprs)) => {
lists_of_exprs.iter().flatten().for_each_till_continue(f)
}
Expr::Nop
| Expr::Column(_)
// Treat OuterReferenceColumn as a leaf expression
| Expr::OuterReferenceColumn(_, _)
Expand Down Expand Up @@ -120,7 +119,6 @@ impl TreeNode for Expr {
let mut transform = transform;

Ok(match self {
Expr::Nop => self,
Expr::Alias(Alias {
expr,
relation,
Expand Down Expand Up @@ -381,7 +379,6 @@ impl TreeNode for Expr {
Expr::GroupingSet(GroupingSet::GroupingSets(lists_of_exprs)) => {
lists_of_exprs.iter_mut().flatten().for_each_till_continue(f)
}
Expr::Nop
| Expr::Column(_)
// Treat OuterReferenceColumn as a leaf expression
| Expr::OuterReferenceColumn(_, _)
Expand Down
3 changes: 1 addition & 2 deletions datafusion/expr/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,7 @@ pub fn expr_to_columns(expr: &Expr, accum: &mut HashSet<Column>) -> Result<()> {
// Use explicit pattern match instead of a default
// implementation, so that in the future if someone adds
// new Expr types, they will check here as well
Expr::Nop
| Expr::ScalarVariable(_, _)
Expr::ScalarVariable(_, _)
| Expr::Alias(_)
| Expr::Literal(_)
| Expr::BinaryExpr { .. }
Expand Down
1 change: 0 additions & 1 deletion datafusion/optimizer/src/push_down_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ fn can_evaluate_as_join_condition(predicate: &Expr) -> Result<bool> {
| Expr::ScalarFunction(..)
| Expr::InList { .. } => Ok(TreeNodeRecursion::Continue),
Expr::Sort(_)
| Expr::Nop
| Expr::AggregateFunction(_)
| Expr::WindowFunction(_)
| Expr::Wildcard { .. }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,7 @@ impl<'a> ConstEvaluator<'a> {
// at plan time
match expr {
// Has no runtime cost, but needed during planning
Expr::Nop
| Expr::Alias(..)
Expr::Alias(..)
| Expr::AggregateFunction { .. }
| Expr::ScalarVariable(_, _)
| Expr::Column(_)
Expand Down
4 changes: 0 additions & 4 deletions datafusion/proto/src/logical_plan/to_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,10 +484,6 @@ impl TryFrom<&Expr> for protobuf::LogicalExprNode {
use protobuf::logical_expr_node::ExprType;

let expr_node = match expr {
Expr::Nop => Err(Error::NotImplemented(
"Proto serialization error: Trying to serialize a Nop expression"
.to_string(),
))?,
Expr::Column(c) => Self {
expr_type: Some(ExprType::Column(c.into())),
},
Expand Down

0 comments on commit 6cd5d39

Please sign in to comment.