Skip to content

Commit

Permalink
tmp: if set short_circuit to only be on divide, not boolean operators
Browse files Browse the repository at this point in the history
  • Loading branch information
wiedld committed Mar 19, 2024
1 parent a88ddc4 commit 256a701
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 35 deletions.
2 changes: 1 addition & 1 deletion datafusion/expr/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1297,7 +1297,7 @@ impl Expr {
matches!(func_def, ScalarFunctionDefinition::BuiltIn(fun) if *fun == BuiltinScalarFunction::Coalesce)
}
Expr::BinaryExpr(BinaryExpr { op, .. }) => {
matches!(op, Operator::And | Operator::Or)
matches!(op, Operator::Divide)
}
Expr::Case { .. } => true,
// Use explicit pattern match instead of a default
Expand Down
24 changes: 2 additions & 22 deletions datafusion/sqllogictest/test_files/common_subexpr_eliminate.slt
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,9 @@ D 1 true
E 0 false
E 1 false

# BUG: fix this
# added to boolean expr in the projections `AND c2 % 2 = 1`
# `Error during planning: No function matches the given name and argument types 'chr(Utf8)'`
#
query error DataFusion error: Optimizer rule 'common_sub_expression_eliminate' failed
SELECT DISTINCT ON (chr(ascii(c1) + 3), c2 % 2) chr(ascii(upper(c1)) + 3), c2 % 2, c3 > 80 AND c2 % 2 = 1
FROM aggregate_test_100
WHERE c1 IN ('a', 'b')
ORDER BY chr(ascii(c1) + 3), c2 % 2, c3 DESC;

# another boolean is fine. It's the subexpr `c2 % 2 = 1`.
# handles starting complex expression, with AND'ed expressions
query TIB
SELECT DISTINCT ON (chr(ascii(c1) + 3), c2 % 2) chr(ascii(upper(c1)) + 3), c2 % 2, c3 > 80 AND true
SELECT DISTINCT ON (chr(ascii(c1) + 3), c2 % 2) chr(ascii(upper(c1)) + 3), c2 % 2, c3 > 80 AND c2 % 2 = 1
FROM aggregate_test_100
WHERE c1 IN ('a', 'b')
ORDER BY chr(ascii(c1) + 3), c2 % 2, c3 DESC;
Expand All @@ -104,13 +94,3 @@ D 0 false
D 1 true
E 0 false
E 1 false

# The projection `c2 % 2 = 1` is fine.
query B
SELECT c2 % 2 = 1
FROM aggregate_test_100
WHERE c1 IN ('a', 'b')
ORDER BY chr(ascii(c1) + 3), c2 % 2, c3 DESC
LIMIT 1;
----
false
10 changes: 4 additions & 6 deletions datafusion/sqllogictest/test_files/select.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1383,13 +1383,11 @@ query TT
EXPLAIN SELECT x/2, x/2+1 FROM t;
----
logical_plan
Projection: t.x / Int64(2)Int64(2)t.x AS t.x / Int64(2), t.x / Int64(2)Int64(2)t.x AS t.x / Int64(2) + Int64(1)
--Projection: t.x / Int64(2) AS t.x / Int64(2)Int64(2)t.x
----TableScan: t projection=[x]
Projection: t.x / Int64(2), t.x / Int64(2) + t.x / Int64(2) AS t.x / Int64(2) + Int64(1)
--TableScan: t projection=[x]
physical_plan
ProjectionExec: expr=[t.x / Int64(2)Int64(2)t.x@0 as t.x / Int64(2), t.x / Int64(2)Int64(2)t.x@0 + 1 as t.x / Int64(2) + Int64(1)]
--ProjectionExec: expr=[x@0 / 2 as t.x / Int64(2)Int64(2)t.x]
----MemoryExec: partitions=1, partition_sizes=[1]
ProjectionExec: expr=[x@0 / 2 as t.x / Int64(2), x@0 / 2 + x@0 / 2 as t.x / Int64(2) + Int64(1)]
--MemoryExec: partitions=1, partition_sizes=[1]

query II
SELECT x/2, x/2+1 FROM t;
Expand Down
10 changes: 4 additions & 6 deletions datafusion/sqllogictest/test_files/subquery.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1046,9 +1046,8 @@ query TT
explain select a/2, a/2 + 1 from t
----
logical_plan
Projection: t.a / Int64(2)Int64(2)t.a AS t.a / Int64(2), t.a / Int64(2)Int64(2)t.a AS t.a / Int64(2) + Int64(1)
--Projection: t.a / Int64(2) AS t.a / Int64(2)Int64(2)t.a
----TableScan: t projection=[a]
Projection: t.a / Int64(2), t.a / Int64(2) + t.a / Int64(2) AS t.a / Int64(2) + Int64(1)
--TableScan: t projection=[a]

statement ok
set datafusion.optimizer.max_passes = 3;
Expand All @@ -1057,6 +1056,5 @@ query TT
explain select a/2, a/2 + 1 from t
----
logical_plan
Projection: t.a / Int64(2)Int64(2)t.a AS t.a / Int64(2), t.a / Int64(2)Int64(2)t.a AS t.a / Int64(2) + Int64(1)
--Projection: t.a / Int64(2) AS t.a / Int64(2)Int64(2)t.a
----TableScan: t projection=[a]
Projection: t.a / Int64(2), t.a / Int64(2) + t.a / Int64(2) AS t.a / Int64(2) + Int64(1)
--TableScan: t projection=[a]

0 comments on commit 256a701

Please sign in to comment.