From 12aa82c7454e9d5fdbc637a4dbeb7b7a4fc459e8 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Fri, 9 Aug 2024 22:12:30 -0400 Subject: [PATCH] Minor: use `lit(true)` and `lit(false)` more (#11904) --- datafusion/optimizer/src/decorrelate.rs | 6 ++-- datafusion/optimizer/src/eliminate_filter.rs | 8 ++--- datafusion/optimizer/src/eliminate_join.rs | 8 ++--- .../optimizer/src/propagate_empty_relation.rs | 34 +++++++++---------- .../simplify_expressions/expr_simplifier.rs | 8 ++--- datafusion/physical-expr/src/planner.rs | 22 +++++------- 6 files changed, 40 insertions(+), 46 deletions(-) diff --git a/datafusion/optimizer/src/decorrelate.rs b/datafusion/optimizer/src/decorrelate.rs index 16b4e43abcd5..4d0770ccbbfb 100644 --- a/datafusion/optimizer/src/decorrelate.rs +++ b/datafusion/optimizer/src/decorrelate.rs @@ -31,7 +31,7 @@ use datafusion_common::{plan_err, Column, DFSchemaRef, Result, ScalarValue}; use datafusion_expr::expr::Alias; use datafusion_expr::simplify::SimplifyContext; use datafusion_expr::utils::{conjunction, find_join_exprs, split_conjunction}; -use datafusion_expr::{expr, EmptyRelation, Expr, LogicalPlan, LogicalPlanBuilder}; +use datafusion_expr::{expr, lit, EmptyRelation, Expr, LogicalPlan, LogicalPlanBuilder}; use datafusion_physical_expr::execution_props::ExecutionProps; /// This struct rewrite the sub query plan by pull up the correlated @@ -282,9 +282,7 @@ impl TreeNodeRewriter for PullUpCorrelatedExpr { )?; if !expr_result_map_for_count_bug.is_empty() { // has count bug - let un_matched_row = - Expr::Literal(ScalarValue::Boolean(Some(true))) - .alias(UN_MATCHED_ROW_INDICATOR); + let un_matched_row = lit(true).alias(UN_MATCHED_ROW_INDICATOR); // add the unmatched rows indicator to the Aggregation's group expressions missing_exprs.push(un_matched_row); } diff --git a/datafusion/optimizer/src/eliminate_filter.rs b/datafusion/optimizer/src/eliminate_filter.rs index 2d8d77b89ddc..84bb8e782142 100644 --- a/datafusion/optimizer/src/eliminate_filter.rs +++ b/datafusion/optimizer/src/eliminate_filter.rs @@ -97,7 +97,7 @@ mod tests { #[test] fn filter_false() -> Result<()> { - let filter_expr = Expr::Literal(ScalarValue::Boolean(Some(false))); + let filter_expr = lit(false); let table_scan = test_table_scan().unwrap(); let plan = LogicalPlanBuilder::from(table_scan) @@ -127,7 +127,7 @@ mod tests { #[test] fn filter_false_nested() -> Result<()> { - let filter_expr = Expr::Literal(ScalarValue::Boolean(Some(false))); + let filter_expr = lit(false); let table_scan = test_table_scan()?; let plan1 = LogicalPlanBuilder::from(table_scan.clone()) @@ -149,7 +149,7 @@ mod tests { #[test] fn filter_true() -> Result<()> { - let filter_expr = Expr::Literal(ScalarValue::Boolean(Some(true))); + let filter_expr = lit(true); let table_scan = test_table_scan()?; let plan = LogicalPlanBuilder::from(table_scan) @@ -164,7 +164,7 @@ mod tests { #[test] fn filter_true_nested() -> Result<()> { - let filter_expr = Expr::Literal(ScalarValue::Boolean(Some(true))); + let filter_expr = lit(true); let table_scan = test_table_scan()?; let plan1 = LogicalPlanBuilder::from(table_scan.clone()) diff --git a/datafusion/optimizer/src/eliminate_join.rs b/datafusion/optimizer/src/eliminate_join.rs index c5115c87a0ed..b15d981d1180 100644 --- a/datafusion/optimizer/src/eliminate_join.rs +++ b/datafusion/optimizer/src/eliminate_join.rs @@ -83,9 +83,9 @@ impl OptimizerRule for EliminateJoin { mod tests { use crate::eliminate_join::EliminateJoin; use crate::test::*; - use datafusion_common::{Result, ScalarValue}; + use datafusion_common::Result; use datafusion_expr::JoinType::Inner; - use datafusion_expr::{logical_plan::builder::LogicalPlanBuilder, Expr, LogicalPlan}; + use datafusion_expr::{lit, logical_plan::builder::LogicalPlanBuilder, LogicalPlan}; use std::sync::Arc; fn assert_optimized_plan_equal(plan: LogicalPlan, expected: &str) -> Result<()> { @@ -98,7 +98,7 @@ mod tests { .join_on( LogicalPlanBuilder::empty(false).build()?, Inner, - Some(Expr::Literal(ScalarValue::Boolean(Some(false)))), + Some(lit(false)), )? .build()?; @@ -112,7 +112,7 @@ mod tests { .join_on( LogicalPlanBuilder::empty(false).build()?, Inner, - Some(Expr::Literal(ScalarValue::Boolean(Some(true)))), + Some(lit(true)), )? .build()?; diff --git a/datafusion/optimizer/src/propagate_empty_relation.rs b/datafusion/optimizer/src/propagate_empty_relation.rs index 91044207c4e1..6bf878ab3df8 100644 --- a/datafusion/optimizer/src/propagate_empty_relation.rs +++ b/datafusion/optimizer/src/propagate_empty_relation.rs @@ -250,10 +250,10 @@ mod tests { use arrow::datatypes::{DataType, Field, Schema}; - use datafusion_common::{Column, DFSchema, JoinType, ScalarValue}; + use datafusion_common::{Column, DFSchema, JoinType}; use datafusion_expr::logical_plan::table_scan; use datafusion_expr::{ - binary_expr, col, lit, logical_plan::builder::LogicalPlanBuilder, Expr, Operator, + binary_expr, col, lit, logical_plan::builder::LogicalPlanBuilder, Operator, }; use crate::eliminate_filter::EliminateFilter; @@ -289,7 +289,7 @@ mod tests { #[test] fn propagate_empty() -> Result<()> { let plan = LogicalPlanBuilder::empty(false) - .filter(Expr::Literal(ScalarValue::Boolean(Some(true))))? + .filter(lit(true))? .limit(10, None)? .project(vec![binary_expr(lit(1), Operator::Plus, lit(1))])? .build()?; @@ -305,7 +305,7 @@ mod tests { let right_table_scan = test_table_scan_with_name("test2")?; let right = LogicalPlanBuilder::from(right_table_scan) .project(vec![col("a")])? - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let plan = LogicalPlanBuilder::from(left) @@ -325,7 +325,7 @@ mod tests { fn propagate_union_empty() -> Result<()> { let left = LogicalPlanBuilder::from(test_table_scan()?).build()?; let right = LogicalPlanBuilder::from(test_table_scan_with_name("test2")?) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let plan = LogicalPlanBuilder::from(left).union(right)?.build()?; @@ -339,10 +339,10 @@ mod tests { let one = LogicalPlanBuilder::from(test_table_scan_with_name("test1")?).build()?; let two = LogicalPlanBuilder::from(test_table_scan_with_name("test2")?) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let three = LogicalPlanBuilder::from(test_table_scan_with_name("test3")?) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let four = LogicalPlanBuilder::from(test_table_scan_with_name("test4")?).build()?; @@ -362,16 +362,16 @@ mod tests { #[test] fn propagate_union_all_empty() -> Result<()> { let one = LogicalPlanBuilder::from(test_table_scan_with_name("test1")?) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let two = LogicalPlanBuilder::from(test_table_scan_with_name("test2")?) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let three = LogicalPlanBuilder::from(test_table_scan_with_name("test3")?) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let four = LogicalPlanBuilder::from(test_table_scan_with_name("test4")?) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let plan = LogicalPlanBuilder::from(one) @@ -389,7 +389,7 @@ mod tests { let one_schema = Schema::new(vec![Field::new("t1a", DataType::UInt32, false)]); let t1_scan = table_scan(Some("test1"), &one_schema, None)?.build()?; let one = LogicalPlanBuilder::from(t1_scan) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let two_schema = Schema::new(vec![Field::new("t2a", DataType::UInt32, false)]); @@ -415,7 +415,7 @@ mod tests { fn propagate_union_alias() -> Result<()> { let left = LogicalPlanBuilder::from(test_table_scan()?).build()?; let right = LogicalPlanBuilder::from(test_table_scan_with_name("test2")?) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let plan = LogicalPlanBuilder::from(left).union(right)?.build()?; @@ -449,7 +449,7 @@ mod tests { let left_table_scan = test_table_scan()?; LogicalPlanBuilder::from(left_table_scan) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build() } else { let scan = test_table_scan_with_name("left").unwrap(); @@ -460,7 +460,7 @@ mod tests { let right_table_scan = test_table_scan_with_name("right")?; LogicalPlanBuilder::from(right_table_scan) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build() } else { let scan = test_table_scan_with_name("right").unwrap(); @@ -487,14 +487,14 @@ mod tests { let (left, right, join_type, expected) = if anti_left_join { let left = test_table_scan()?; let right = LogicalPlanBuilder::from(test_table_scan()?) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let expected = left.display_indent().to_string(); (left, right, JoinType::LeftAnti, expected) } else { let right = test_table_scan()?; let left = LogicalPlanBuilder::from(test_table_scan()?) - .filter(Expr::Literal(ScalarValue::Boolean(Some(false))))? + .filter(lit(false))? .build()?; let expected = right.display_indent().to_string(); (left, right, JoinType::RightAnti, expected) diff --git a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs index 979a1499d0de..c45df74a564d 100644 --- a/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs +++ b/datafusion/optimizer/src/simplify_expressions/expr_simplifier.rs @@ -817,7 +817,7 @@ impl<'a, S: SimplifyInfo> TreeNodeRewriter for Simplifier<'a, S> { op: Or, right, }) if is_not_of(&right, &left) && !info.nullable(&left)? => { - Transformed::yes(Expr::Literal(ScalarValue::Boolean(Some(true)))) + Transformed::yes(lit(true)) } // !A OR A ---> true (if A not nullable) Expr::BinaryExpr(BinaryExpr { @@ -825,7 +825,7 @@ impl<'a, S: SimplifyInfo> TreeNodeRewriter for Simplifier<'a, S> { op: Or, right, }) if is_not_of(&left, &right) && !info.nullable(&right)? => { - Transformed::yes(Expr::Literal(ScalarValue::Boolean(Some(true)))) + Transformed::yes(lit(true)) } // (..A..) OR A --> (..A..) Expr::BinaryExpr(BinaryExpr { @@ -890,7 +890,7 @@ impl<'a, S: SimplifyInfo> TreeNodeRewriter for Simplifier<'a, S> { op: And, right, }) if is_not_of(&right, &left) && !info.nullable(&left)? => { - Transformed::yes(Expr::Literal(ScalarValue::Boolean(Some(false)))) + Transformed::yes(lit(false)) } // !A AND A ---> false (if A not nullable) Expr::BinaryExpr(BinaryExpr { @@ -898,7 +898,7 @@ impl<'a, S: SimplifyInfo> TreeNodeRewriter for Simplifier<'a, S> { op: And, right, }) if is_not_of(&left, &right) && !info.nullable(&right)? => { - Transformed::yes(Expr::Literal(ScalarValue::Boolean(Some(false)))) + Transformed::yes(lit(false)) } // (..A..) AND A --> (..A..) Expr::BinaryExpr(BinaryExpr { diff --git a/datafusion/physical-expr/src/planner.rs b/datafusion/physical-expr/src/planner.rs index a975f0c6ef83..d015f545bf9d 100644 --- a/datafusion/physical-expr/src/planner.rs +++ b/datafusion/physical-expr/src/planner.rs @@ -31,7 +31,9 @@ use datafusion_expr::execution_props::ExecutionProps; use datafusion_expr::expr::{Alias, Cast, InList, ScalarFunction}; use datafusion_expr::var_provider::is_system_variables; use datafusion_expr::var_provider::VarType; -use datafusion_expr::{binary_expr, Between, BinaryExpr, Expr, Like, Operator, TryCast}; +use datafusion_expr::{ + binary_expr, lit, Between, BinaryExpr, Expr, Like, Operator, TryCast, +}; /// [PhysicalExpr] evaluate DataFusion expressions such as `A + 1`, or `CAST(c1 /// AS int)`. @@ -140,32 +142,26 @@ pub fn create_physical_expr( let binary_op = binary_expr( expr.as_ref().clone(), Operator::IsNotDistinctFrom, - Expr::Literal(ScalarValue::Boolean(Some(true))), + lit(true), ); create_physical_expr(&binary_op, input_dfschema, execution_props) } Expr::IsNotTrue(expr) => { - let binary_op = binary_expr( - expr.as_ref().clone(), - Operator::IsDistinctFrom, - Expr::Literal(ScalarValue::Boolean(Some(true))), - ); + let binary_op = + binary_expr(expr.as_ref().clone(), Operator::IsDistinctFrom, lit(true)); create_physical_expr(&binary_op, input_dfschema, execution_props) } Expr::IsFalse(expr) => { let binary_op = binary_expr( expr.as_ref().clone(), Operator::IsNotDistinctFrom, - Expr::Literal(ScalarValue::Boolean(Some(false))), + lit(false), ); create_physical_expr(&binary_op, input_dfschema, execution_props) } Expr::IsNotFalse(expr) => { - let binary_op = binary_expr( - expr.as_ref().clone(), - Operator::IsDistinctFrom, - Expr::Literal(ScalarValue::Boolean(Some(false))), - ); + let binary_op = + binary_expr(expr.as_ref().clone(), Operator::IsDistinctFrom, lit(false)); create_physical_expr(&binary_op, input_dfschema, execution_props) } Expr::IsUnknown(expr) => {