From a1205aa06835e0eff20c05e791fa687869659d13 Mon Sep 17 00:00:00 2001 From: Lordworms Date: Thu, 8 Aug 2024 18:40:14 -0700 Subject: [PATCH] fix: bugs when having and group by are all false --- datafusion/sql/src/select.rs | 12 +++++++----- .../sqllogictest/test_files/aggregate.slt | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/datafusion/sql/src/select.rs b/datafusion/sql/src/select.rs index 95a44dace31a..0b220d5b0980 100644 --- a/datafusion/sql/src/select.rs +++ b/datafusion/sql/src/select.rs @@ -27,7 +27,7 @@ use crate::utils::{ }; use datafusion_common::tree_node::{TreeNode, TreeNodeRecursion}; -use datafusion_common::{not_impl_err, plan_err, DataFusionError, Result}; +use datafusion_common::{not_impl_err, plan_err, DataFusionError, Result, ScalarValue}; use datafusion_common::{Column, UnnestOptions}; use datafusion_expr::expr::Alias; use datafusion_expr::expr_rewriter::{ @@ -211,7 +211,6 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { None => (base_plan.clone(), select_exprs.clone(), having_expr_opt) } }; - let plan = if let Some(having_expr_post_aggr) = having_expr_post_aggr { LogicalPlanBuilder::from(plan) .filter(having_expr_post_aggr)? @@ -778,9 +777,12 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> { // Rewrite the HAVING expression to use the columns produced by the // aggregation. let having_expr_post_aggr = if let Some(having_expr) = having_expr_opt { - let having_expr_post_aggr = - rebase_expr(having_expr, &aggr_projection_exprs, input)?; - + let having_expr_post_aggr = match having_expr { + Expr::Literal(ScalarValue::Boolean(Some(false))) => { + having_expr.to_owned() + } + _ => rebase_expr(having_expr, &aggr_projection_exprs, input)?, + }; check_columns_satisfy_exprs( &column_exprs_post_aggr, &[having_expr_post_aggr.clone()], diff --git a/datafusion/sqllogictest/test_files/aggregate.slt b/datafusion/sqllogictest/test_files/aggregate.slt index 8a5222143356..d05fa5c8548f 100644 --- a/datafusion/sqllogictest/test_files/aggregate.slt +++ b/datafusion/sqllogictest/test_files/aggregate.slt @@ -5548,3 +5548,20 @@ set datafusion.explain.logical_plan_only = false; statement ok drop table employee_csv; + +statement ok +create table t1(v1 int); + +query R +SELECT AVG(v1) FROM t1 having false; +---- + +query TT +explain SELECT AVG(v1) FROM t1 GROUP BY false having false; +---- +logical_plan EmptyRelation +physical_plan EmptyExec + + +statement ok +drop table t1;