Skip to content

Commit 778e16c

Browse files
committed
fix(cubesql): ORDER BY references outer alias instead of inner expression for SQL push down
1 parent da24afe commit 778e16c

File tree

2 files changed

+60
-3
lines changed

2 files changed

+60
-3
lines changed

rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ use cubeclient::models::V1LoadRequestQuery;
1313
use datafusion::{
1414
error::{DataFusionError, Result},
1515
logical_plan::{
16-
plan::Extension, replace_col, Column, DFSchema, DFSchemaRef, Expr, LogicalPlan,
17-
UserDefinedLogicalNode,
16+
plan::Extension, replace_col, replace_col_to_expr, Column, DFSchema, DFSchemaRef, Expr,
17+
LogicalPlan, UserDefinedLogicalNode,
1818
},
1919
physical_plan::aggregates::AggregateFunction,
2020
scalar::ScalarValue,
@@ -405,10 +405,37 @@ impl CubeScanWrapperNode {
405405
ungrouped_scan_node.clone(),
406406
)
407407
.await?;
408+
// Sort node always comes on top and pushed down to select so we need to replace columns here by appropriate column definitions
409+
let order_replace_map = projection_expr
410+
.iter()
411+
.chain(group_expr.iter())
412+
.chain(aggr_expr.iter())
413+
.map(|e| {
414+
Ok((
415+
Column {
416+
relation: alias.clone(),
417+
name: expr_name(&e, &schema)?,
418+
},
419+
e.clone(),
420+
))
421+
})
422+
.collect::<Result<HashMap<_, _>>>()?;
423+
408424
let (order, mut sql) = Self::generate_column_expr(
409425
plan.clone(),
410426
schema.clone(),
411-
order_expr.clone(),
427+
order_expr
428+
.iter()
429+
.map(|o| {
430+
replace_col_to_expr(
431+
o.clone(),
432+
&order_replace_map
433+
.iter()
434+
.map(|(k, v)| (k, v))
435+
.collect(),
436+
)
437+
})
438+
.collect::<Result<Vec<_>>>()?,
412439
sql,
413440
generator.clone(),
414441
&column_remapping,

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18438,6 +18438,36 @@ ORDER BY \"COUNT(count)\" DESC"
1843818438
.contains("ORDER BY"));
1843918439
}
1844018440

18441+
#[tokio::test]
18442+
async fn test_case_wrapper_ungrouped_sorted_aliased() {
18443+
if !Rewriter::sql_push_down_enabled() {
18444+
return;
18445+
}
18446+
init_logger();
18447+
18448+
let query_plan = convert_select_to_query_plan(
18449+
"SELECT x FROM (SELECT CASE WHEN customer_gender = 'female' THEN 'f' ELSE 'm' END x, AVG(avgPrice) mp FROM KibanaSampleDataEcommerce a GROUP BY 1 ORDER BY 1 DESC) b"
18450+
.to_string(),
18451+
DatabaseProtocol::PostgreSQL,
18452+
)
18453+
.await;
18454+
18455+
let physical_plan = query_plan.as_physical_plan().await.unwrap();
18456+
println!(
18457+
"Physical plan: {}",
18458+
displayable(physical_plan.as_ref()).indent()
18459+
);
18460+
18461+
let logical_plan = query_plan.as_logical_plan();
18462+
assert!(logical_plan
18463+
.find_cube_scan_wrapper()
18464+
.wrapped_sql
18465+
.unwrap()
18466+
.sql
18467+
// TODO test without depend on column name
18468+
.contains("ORDER BY \"case_when"));
18469+
}
18470+
1844118471
#[tokio::test]
1844218472
async fn test_case_wrapper_with_limit() {
1844318473
if !Rewriter::sql_push_down_enabled() {

0 commit comments

Comments
 (0)