Skip to content

Commit

Permalink
fix: bugs when having and group by are all false
Browse files Browse the repository at this point in the history
  • Loading branch information
Lordworms committed Aug 9, 2024
1 parent 786f353 commit a1205aa
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 5 deletions.
12 changes: 7 additions & 5 deletions datafusion/sql/src/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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)?
Expand Down Expand Up @@ -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()],
Expand Down
17 changes: 17 additions & 0 deletions datafusion/sqllogictest/test_files/aggregate.slt
Original file line number Diff line number Diff line change
Expand Up @@ -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;

0 comments on commit a1205aa

Please sign in to comment.