Skip to content

Commit 948f6b8

Browse files
Col-Waltzalamb
andauthored
Fix: common_sub_expression_eliminate optimizer rule failed (#16066)
Common_sub_expression_eliminate rule failed with error: `SchemaError(FieldNotFound {field: <name>}, valid_fields: []})` due to the schema being changed by the second application of `find_common_exprs` As I understood the source of the problem was in sequential call of `find_common_exprs`. First call returned original names as `aggr_expr` and changed names as `new_aggr_expr`. Second call takes into account only `new_aggr_expr` and if names was already changed by first call will return changed names as `aggr_expr`(original ones) and put them into Projection logic. I used NamePreserver mechanism to restore original schema names and generate Projection with original name at the end of aggregate optimization. Co-authored-by: Andrew Lamb <[email protected]>
1 parent 2987e80 commit 948f6b8

File tree

2 files changed

+49
-2
lines changed

2 files changed

+49
-2
lines changed

datafusion/optimizer/src/common_subexpr_eliminate.rs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,19 @@ impl CommonSubexprEliminate {
316316
} => {
317317
let rewritten_aggr_expr = new_exprs_list.pop().unwrap();
318318
let new_aggr_expr = original_exprs_list.pop().unwrap();
319+
let saved_names = if let Some(aggr_expr) = aggr_expr {
320+
let name_preserver = NamePreserver::new_for_projection();
321+
aggr_expr
322+
.iter()
323+
.map(|expr| Some(name_preserver.save(expr)))
324+
.collect::<Vec<_>>()
325+
} else {
326+
new_aggr_expr
327+
.clone()
328+
.into_iter()
329+
.map(|_| None)
330+
.collect::<Vec<_>>()
331+
};
319332

320333
let mut agg_exprs = common_exprs
321334
.into_iter()
@@ -326,10 +339,19 @@ impl CommonSubexprEliminate {
326339
for expr in &new_group_expr {
327340
extract_expressions(expr, &mut proj_exprs)
328341
}
329-
for (expr_rewritten, expr_orig) in
330-
rewritten_aggr_expr.into_iter().zip(new_aggr_expr)
342+
for ((expr_rewritten, expr_orig), saved_name) in
343+
rewritten_aggr_expr
344+
.into_iter()
345+
.zip(new_aggr_expr)
346+
.zip(saved_names)
331347
{
332348
if expr_rewritten == expr_orig {
349+
let expr_rewritten = if let Some(saved_name) = saved_name
350+
{
351+
saved_name.restore(expr_rewritten)
352+
} else {
353+
expr_rewritten
354+
};
333355
if let Expr::Alias(Alias { expr, name, .. }) =
334356
expr_rewritten
335357
{

datafusion/sqllogictest/test_files/aggregate.slt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,31 @@ SELECT c2, var_samp(c12) FILTER (WHERE c12 > 0.95) FROM aggregate_test_100 GROUP
739739
4 NULL
740740
5 NULL
741741

742+
statement ok
743+
CREATE TABLE t (
744+
a DOUBLE,
745+
b BIGINT,
746+
c INT
747+
) AS VALUES
748+
(1.0, 10, -5),
749+
(2.0, 20, -5),
750+
(3.0, 20, 4);
751+
752+
# https://github.com/apache/datafusion/issues/15291
753+
query III
754+
WITH s AS (
755+
SELECT
756+
COUNT(a) FILTER (WHERE (b * b) - 3600 <= b),
757+
COUNT(a) FILTER (WHERE (b * b) - 3000 <= b AND (c >= 0)),
758+
COUNT(a) FILTER (WHERE (b * b) - 3000 <= b AND (c >= 0) AND (c >= 0))
759+
FROM t
760+
) SELECT * FROM s
761+
----
762+
3 1 1
763+
764+
statement ok
765+
DROP TABLE t
766+
742767
# Restore the default dialect
743768
statement ok
744769
set datafusion.sql_parser.dialect = 'Generic';

0 commit comments

Comments
 (0)