Skip to content

Commit bd20e65

Browse files
authored
fix(cubesql): Fix push Limit-Sort down Projection recursion (#9986)
1 parent 1490598 commit bd20e65

File tree

2 files changed

+56
-16
lines changed

2 files changed

+56
-16
lines changed

rust/cubesql/cubesql/src/compile/mod.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17737,4 +17737,45 @@ LIMIT {{ limit }}{% endif %}"#.to_string(),
1773717737
);
1773817738
Ok(())
1773917739
}
17740+
17741+
#[tokio::test]
17742+
async fn test_push_down_limit_sort_projection_recursion() {
17743+
if !Rewriter::sql_push_down_enabled() {
17744+
return;
17745+
}
17746+
init_testing_logger();
17747+
17748+
let logical_plan = convert_select_to_query_plan(
17749+
r#"
17750+
SELECT
17751+
customer_gender AS "customer_gender",
17752+
SUM(sumPrice) AS "SUM(KibanaSampleDataEcommerce.sumPrice)"
17753+
FROM KibanaSampleDataEcommerce
17754+
GROUP BY 1
17755+
ORDER BY 2 DESC
17756+
LIMIT 3
17757+
"#
17758+
.to_string(),
17759+
DatabaseProtocol::PostgreSQL,
17760+
)
17761+
.await
17762+
.as_logical_plan();
17763+
17764+
assert_eq!(
17765+
logical_plan.find_cube_scan().request,
17766+
V1LoadRequestQuery {
17767+
measures: Some(vec!["KibanaSampleDataEcommerce.sumPrice".to_string()]),
17768+
dimensions: Some(vec![
17769+
"KibanaSampleDataEcommerce.customer_gender".to_string(),
17770+
]),
17771+
segments: Some(vec![]),
17772+
order: Some(vec![vec![
17773+
"KibanaSampleDataEcommerce.sumPrice".to_string(),
17774+
"desc".to_string(),
17775+
]]),
17776+
limit: Some(3),
17777+
..Default::default()
17778+
}
17779+
)
17780+
}
1774017781
}

rust/cubesql/cubesql/src/compile/rewrite/rules/order.rs

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@ use crate::{
1616
order_replacer, projection, referenced_columns, rewrite,
1717
rewriter::{CubeEGraph, CubeRewrite, RewriteRules},
1818
sort, sort_exp, sort_exp_empty_tail, sort_expr, sort_projection_pullup_replacer,
19-
sort_projection_pushdown_replacer, transforming_rewrite, ColumnExprColumn,
20-
LogicalPlanLanguage, OrderAsc, OrderMember, OrderReplacerColumnNameToMember,
21-
ProjectionAlias, SortExprAsc, SortProjectionPushdownReplacerColumnToExpr,
19+
sort_projection_pushdown_replacer, transforming_chain_rewrite, transforming_rewrite,
20+
ColumnExprColumn, LogicalPlanLanguage, OrderAsc, OrderMember,
21+
OrderReplacerColumnNameToMember, ProjectionAlias, SortExprAsc,
22+
SortProjectionPushdownReplacerColumnToExpr,
2223
},
2324
},
2425
config::ConfigObj,
@@ -87,21 +88,19 @@ impl RewriteRules for OrderRules {
8788
),
8889
// TODO: refactor this rule to `push-down-sort-projection`,
8990
// possibly adjust cost function to penalize Limit-...-Sort plan
90-
transforming_rewrite(
91+
transforming_chain_rewrite(
9192
"push-down-limit-sort-projection",
92-
limit(
93-
"?skip",
94-
"?fetch",
95-
sort(
96-
"?sort_expr",
97-
projection(
98-
"?projection_expr",
99-
"?input",
100-
"?projection_alias",
101-
"?projection_split",
102-
),
93+
limit("?skip", "?fetch", sort("?sort_expr", "?projection")),
94+
vec![(
95+
// Avoid recursion when projection input is self
96+
"?projection",
97+
projection(
98+
"?projection_expr",
99+
"?input",
100+
"?projection_alias",
101+
"?projection_split",
103102
),
104-
),
103+
)],
105104
projection(
106105
"?projection_expr",
107106
limit(

0 commit comments

Comments
 (0)