Skip to content

Commit 8826ccc

Browse files
committed
fix(cubesql): Disable filter pushdown over Filter(CrossJoin)
This should help with rewrites filters on top of complex ungrouped-grouped joins as a subquery joins
1 parent 4b634b3 commit 8826ccc

File tree

3 files changed

+75
-6
lines changed

3 files changed

+75
-6
lines changed

rust/cubesql/cubesql/src/compile/engine/df/optimizers/filter_push_down.rs

+9
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,15 @@ fn filter_push_down(
111111
)
112112
}
113113
LogicalPlan::Filter(Filter { predicate, input }) => {
114+
// Current DataFusion version plans complex joins as Filter(CrossJoin)
115+
// So for query like `SELECT ... FROM ... JOIN ... ON complex_condition WHERE predicate`
116+
// Plan can look like Filter(predicate, Filter(join_condition, CrossJoin))
117+
// This optimizer can mess with filter predicates, and break join detection later in rewrites
118+
// So, for now, it just completely pessimizes plans like Filter(CrossJoin)
119+
if let LogicalPlan::CrossJoin(_) = input.as_ref() {
120+
return issue_filter(predicates, plan.clone());
121+
}
122+
114123
// When encountering a filter, collect it to our list of predicates,
115124
// remove the filter from the plan and continue down the plan.
116125

rust/cubesql/cubesql/src/compile/engine/df/optimizers/snapshots/cubesql__compile__engine__df__optimizers__filter_push_down__tests__filter_down_cross_join_right_one_row.snap

+6-6
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ source: cubesql/src/compile/engine/df/optimizers/filter_push_down.rs
33
expression: optimize(&plan)
44
---
55
Filter: #j2.c2 = Int32(10)
6-
CrossJoin:
7-
Filter: #j1.c1 = Int32(5)
6+
Filter: #j1.c1 = Int32(5)
7+
CrossJoin:
88
Projection: #j1.c1
99
TableScan: j1 projection=None
10-
Projection: #COUNT(UInt8(1)) AS c2, alias=j2
11-
Aggregate: groupBy=[[]], aggr=[[COUNT(UInt8(1))]]
12-
Projection: #j2.c2
13-
TableScan: j2 projection=None
10+
Projection: #COUNT(UInt8(1)) AS c2, alias=j2
11+
Aggregate: groupBy=[[]], aggr=[[COUNT(UInt8(1))]]
12+
Projection: #j2.c2
13+
TableScan: j2 projection=None

rust/cubesql/cubesql/src/compile/test/test_cube_join_grouped.rs

+60
Original file line numberDiff line numberDiff line change
@@ -899,3 +899,63 @@ LIMIT 1
899899
.on
900900
.contains(r#"${MultiTypeCube.dim_str0} IS NOT DISTINCT FROM \"t0\".\"dim_str0\""#));
901901
}
902+
903+
/// Filter on top of ungrouped-grouped join with complex condition should be rewritten as well
904+
#[tokio::test]
905+
async fn test_join_ungrouped_grouped_with_filter_and_measure() {
906+
if !Rewriter::sql_push_down_enabled() {
907+
return;
908+
}
909+
init_testing_logger();
910+
911+
let query_plan = convert_select_to_query_plan(
912+
// language=PostgreSQL
913+
r#"
914+
SELECT "t0"."measure"
915+
FROM
916+
MultiTypeCube
917+
INNER JOIN (
918+
SELECT
919+
dim_str0,
920+
AVG(avgPrice) AS "measure"
921+
FROM
922+
MultiTypeCube
923+
GROUP BY 1
924+
) "t0"
925+
ON (MultiTypeCube.dim_str0 IS NOT DISTINCT FROM "t0".dim_str0)
926+
WHERE ("t0"."measure" IS NULL)
927+
LIMIT 1
928+
;
929+
"#
930+
.to_string(),
931+
DatabaseProtocol::PostgreSQL,
932+
)
933+
.await;
934+
935+
let physical_plan = query_plan.as_physical_plan().await.unwrap();
936+
println!(
937+
"Physical plan: {}",
938+
displayable(physical_plan.as_ref()).indent()
939+
);
940+
941+
let request = query_plan
942+
.as_logical_plan()
943+
.find_cube_scan_wrapped_sql()
944+
.request;
945+
946+
assert_eq!(request.ungrouped, Some(true));
947+
948+
assert_eq!(request.subquery_joins.as_ref().unwrap().len(), 1);
949+
950+
let subquery = &request.subquery_joins.unwrap()[0];
951+
952+
assert!(!subquery.sql.contains("ungrouped"));
953+
assert_eq!(subquery.join_type, "INNER");
954+
assert!(subquery
955+
.on
956+
.contains(r#"${MultiTypeCube.dim_str0} IS NOT DISTINCT FROM \"t0\".\"dim_str0\""#));
957+
958+
// Outer filter
959+
assert_eq!(request.segments.as_ref().unwrap().len(), 1);
960+
assert!(request.segments.as_ref().unwrap()[0].contains(r#"\"t0\".\"measure\" IS NULL"#));
961+
}

0 commit comments

Comments
 (0)