diff --git a/datafusion/common/src/display/graphviz.rs b/datafusion/common/src/display/graphviz.rs index f84490cd3ea4e..3fc9909c60c12 100644 --- a/datafusion/common/src/display/graphviz.rs +++ b/datafusion/common/src/display/graphviz.rs @@ -73,22 +73,17 @@ impl GraphvizBuilder { label: &str, tooltip: Option<&str>, ) -> fmt::Result { - if let Some(tooltip) = tooltip { - writeln!( - f, - " {}[shape=box label={}, tooltip={}]", - id, - GraphvizBuilder::quoted(label), - GraphvizBuilder::quoted(tooltip), - ) - } else { - writeln!( - f, - " {}[shape=box label={}]", - id, - GraphvizBuilder::quoted(label), - ) - } + let tooltip = tooltip + .map(|tooltip| format!(", tooltip={}", GraphvizBuilder::quoted(tooltip))) + .unwrap_or_default(); + + writeln!( + f, + " {}[shape=box label={}{}]", + id, + GraphvizBuilder::quoted(label), + tooltip, + ) } pub fn add_edge( diff --git a/datafusion/expr/src/expr.rs b/datafusion/expr/src/expr.rs index f9c0662e682e8..e403a33e35148 100644 --- a/datafusion/expr/src/expr.rs +++ b/datafusion/expr/src/expr.rs @@ -3023,23 +3023,14 @@ impl Display for SchemaDisplay<'_> { low, high, }) => { - if *negated { - write!( - f, - "{} NOT BETWEEN {} AND {}", - SchemaDisplay(expr), - SchemaDisplay(low), - SchemaDisplay(high), - ) - } else { - write!( - f, - "{} BETWEEN {} AND {}", - SchemaDisplay(expr), - SchemaDisplay(low), - SchemaDisplay(high), - ) - } + let not = if *negated { "NOT " } else { "" }; + write!( + f, + "{} {not}BETWEEN {} AND {}", + SchemaDisplay(expr), + SchemaDisplay(low), + SchemaDisplay(high), + ) } Expr::BinaryExpr(BinaryExpr { left, op, right }) => { write!(f, "{} {op} {}", SchemaDisplay(left), SchemaDisplay(right),) @@ -3305,23 +3296,14 @@ impl Display for SqlDisplay<'_> { low, high, }) => { - if *negated { - write!( - f, - "{} NOT BETWEEN {} AND {}", - SqlDisplay(expr), - SqlDisplay(low), - SqlDisplay(high), - ) - } else { - write!( - f, - "{} BETWEEN {} AND {}", - SqlDisplay(expr), - SqlDisplay(low), - SqlDisplay(high), - ) - } + let not = if *negated { "NOT " } else { "" }; + write!( + f, + "{} {not}BETWEEN {} AND {}", + SqlDisplay(expr), + SqlDisplay(low), + SqlDisplay(high), + ) } Expr::BinaryExpr(BinaryExpr { left, op, right }) => { write!(f, "{} {op} {}", SqlDisplay(left), SqlDisplay(right),) diff --git a/datafusion/expr/src/logical_plan/builder.rs b/datafusion/expr/src/logical_plan/builder.rs index ef5e496b0d7de..d8b74fa82aa99 100644 --- a/datafusion/expr/src/logical_plan/builder.rs +++ b/datafusion/expr/src/logical_plan/builder.rs @@ -1423,28 +1423,19 @@ impl LogicalPlanBuilder { ) }) .unzip(); - if is_all { - LogicalPlanBuilder::from(left_plan) - .join_detailed( - right_plan, - join_type, - join_keys, - None, - NullEquality::NullEqualsNull, - )? - .build() - } else { - LogicalPlanBuilder::from(left_plan) - .distinct()? - .join_detailed( - right_plan, - join_type, - join_keys, - None, - NullEquality::NullEqualsNull, - )? - .build() + let mut left_builder = LogicalPlanBuilder::from(left_plan); + if !is_all { + left_builder = left_builder.distinct()?; } + left_builder + .join_detailed( + right_plan, + join_type, + join_keys, + None, + NullEquality::NullEqualsNull, + )? + .build() } /// Build the plan diff --git a/datafusion/functions-aggregate/src/sum.rs b/datafusion/functions-aggregate/src/sum.rs index 1999f68d4cdd7..1b9660dc89b7e 100644 --- a/datafusion/functions-aggregate/src/sum.rs +++ b/datafusion/functions-aggregate/src/sum.rs @@ -261,21 +261,18 @@ impl AggregateUDFImpl for Sum { } fn accumulator(&self, args: AccumulatorArgs) -> Result> { - if args.is_distinct { - macro_rules! helper { - ($t:ty, $dt:expr) => { - Ok(Box::new(DistinctSumAccumulator::<$t>::new($dt))) - }; - } - downcast_sum!(args, helper) - } else { - macro_rules! helper { - ($t:ty, $dt:expr) => { - Ok(Box::new(SumAccumulator::<$t>::new($dt))) - }; - } - downcast_sum!(args, helper) + let is_distinct = args.is_distinct; + macro_rules! helper { + ($t:ty, $dt:expr) => { + if is_distinct { + Ok(Box::new(DistinctSumAccumulator::<$t>::new($dt)) + as Box) + } else { + Ok(Box::new(SumAccumulator::<$t>::new($dt)) as Box) + } + }; } + downcast_sum!(args, helper) } fn state_fields(&self, args: StateFieldsArgs) -> Result> { diff --git a/datafusion/physical-plan/src/joins/hash_join/exec.rs b/datafusion/physical-plan/src/joins/hash_join/exec.rs index 1b36dabebc564..054877cfd63ff 100644 --- a/datafusion/physical-plan/src/joins/hash_join/exec.rs +++ b/datafusion/physical-plan/src/joins/hash_join/exec.rs @@ -2964,25 +2964,16 @@ mod tests { ) -> Arc { let mut session_config = SessionConfig::default().with_batch_size(batch_size); - if use_perfect_hash_join_as_possible { - session_config - .options_mut() - .execution - .perfect_hash_join_small_build_threshold = 819200; - session_config - .options_mut() - .execution - .perfect_hash_join_min_key_density = 0.0; - } else { - session_config - .options_mut() - .execution - .perfect_hash_join_small_build_threshold = 0; - session_config - .options_mut() - .execution - .perfect_hash_join_min_key_density = f64::INFINITY; - } + // Either always take the perfect hash join path, or never take it. + let (small_build_threshold, min_key_density) = + if use_perfect_hash_join_as_possible { + (819200, 0.0) + } else { + (0, f64::INFINITY) + }; + let execution = &mut session_config.options_mut().execution; + execution.perfect_hash_join_small_build_threshold = small_build_threshold; + execution.perfect_hash_join_min_key_density = min_key_density; Arc::new(TaskContext::default().with_session_config(session_config)) } diff --git a/datafusion/proto/src/logical_plan/to_proto.rs b/datafusion/proto/src/logical_plan/to_proto.rs index 16c3468465541..ef49b46d1a04e 100644 --- a/datafusion/proto/src/logical_plan/to_proto.rs +++ b/datafusion/proto/src/logical_plan/to_proto.rs @@ -126,28 +126,29 @@ pub fn serialize_expr( escape_char, case_insensitive, }) => { - if *case_insensitive { - let pb = Box::new(protobuf::ILikeNode { - negated: *negated, - expr: Some(Box::new(serialize_expr(expr.as_ref(), codec)?)), - pattern: Some(Box::new(serialize_expr(pattern.as_ref(), codec)?)), - escape_char: escape_char.map(|ch| ch.to_string()).unwrap_or_default(), - }); + let negated = *negated; + let expr = Some(Box::new(serialize_expr(expr.as_ref(), codec)?)); + let pattern = Some(Box::new(serialize_expr(pattern.as_ref(), codec)?)); + let escape_char = escape_char.map(|ch| ch.to_string()).unwrap_or_default(); - protobuf::LogicalExprNode { - expr_type: Some(ExprType::Ilike(pb)), - } + let expr_type = if *case_insensitive { + ExprType::Ilike(Box::new(protobuf::ILikeNode { + negated, + expr, + pattern, + escape_char, + })) } else { - let pb = Box::new(protobuf::LikeNode { - negated: *negated, - expr: Some(Box::new(serialize_expr(expr.as_ref(), codec)?)), - pattern: Some(Box::new(serialize_expr(pattern.as_ref(), codec)?)), - escape_char: escape_char.map(|ch| ch.to_string()).unwrap_or_default(), - }); + ExprType::Like(Box::new(protobuf::LikeNode { + negated, + expr, + pattern, + escape_char, + })) + }; - protobuf::LogicalExprNode { - expr_type: Some(ExprType::Like(pb)), - } + protobuf::LogicalExprNode { + expr_type: Some(expr_type), } } Expr::SimilarTo(Like { diff --git a/datafusion/sql/src/unparser/expr.rs b/datafusion/sql/src/unparser/expr.rs index 898330018c708..baa64da7733de 100644 --- a/datafusion/sql/src/unparser/expr.rs +++ b/datafusion/sql/src/unparser/expr.rs @@ -371,22 +371,26 @@ impl Unparser<'_> { escape_char, case_insensitive, }) => { + let negated = *negated; + let expr = Box::new(self.expr_to_sql_inner(expr)?); + let pattern = Box::new(self.expr_to_sql_inner(pattern)?); + let escape_char = + escape_char.map(|c| SingleQuotedString(c.to_string()).into()); + if *case_insensitive { Ok(ast::Expr::ILike { - negated: *negated, - expr: Box::new(self.expr_to_sql_inner(expr)?), - pattern: Box::new(self.expr_to_sql_inner(pattern)?), - escape_char: escape_char - .map(|c| SingleQuotedString(c.to_string()).into()), + negated, + expr, + pattern, + escape_char, any: false, }) } else { Ok(ast::Expr::Like { - negated: *negated, - expr: Box::new(self.expr_to_sql_inner(expr)?), - pattern: Box::new(self.expr_to_sql_inner(pattern)?), - escape_char: escape_char - .map(|c| SingleQuotedString(c.to_string()).into()), + negated, + expr, + pattern, + escape_char, any: false, }) } diff --git a/datafusion/sql/src/unparser/plan.rs b/datafusion/sql/src/unparser/plan.rs index 9922509a0e609..d38b1432913b1 100644 --- a/datafusion/sql/src/unparser/plan.rs +++ b/datafusion/sql/src/unparser/plan.rs @@ -1675,21 +1675,13 @@ impl Unparser<'_> { Ok(()) } LogicalPlan::Extension(extension) => { - if let Some(query) = query.as_mut() { - self.extension_to_sql( - extension.node.as_ref(), - &mut Some(query), - &mut Some(select), - &mut Some(relation), - ) - } else { - self.extension_to_sql( - extension.node.as_ref(), - &mut None, - &mut Some(select), - &mut Some(relation), - ) - } + let mut query = query.as_mut(); + self.extension_to_sql( + extension.node.as_ref(), + &mut query, + &mut Some(select), + &mut Some(relation), + ) } LogicalPlan::Unnest(unnest) => { if !unnest.struct_type_columns.is_empty() {