Skip to content

Commit d509852

Browse files
committed
fix(cubesql): Break cost symmetry for (non)-push-to-Cube WrappedSelect
Without this two different representations can have zero members, different push-to-Cube and same cost. It could be done in a single cost component, like zero_members_wrapper, but would require to have a complex dispatch instead of add_child
1 parent 57bcbc4 commit d509852

File tree

2 files changed

+50
-1
lines changed

2 files changed

+50
-1
lines changed

rust/cubesql/cubesql/src/compile/rewrite/cost.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::{
44
compile::rewrite::{
55
rules::utils::granularity_str_to_int_order, CubeScanUngrouped, CubeScanWrapped,
66
DimensionName, LogicalPlanLanguage, MemberErrorPriority, ScalarUDFExprFun,
7-
TimeDimensionGranularity, WrappedSelectUngroupedScan,
7+
TimeDimensionGranularity, WrappedSelectPushToCube, WrappedSelectUngroupedScan,
88
},
99
transport::{MetaContext, V1CubeMetaDimensionExt},
1010
};
@@ -186,6 +186,11 @@ impl BestCubePlan {
186186
_ => 0,
187187
};
188188

189+
let wrapped_select_non_push_to_cube = match enode {
190+
LogicalPlanLanguage::WrappedSelectPushToCube(WrappedSelectPushToCube(false)) => 1,
191+
_ => 0,
192+
};
193+
189194
let wrapped_select_ungrouped_scan = match enode {
190195
LogicalPlanLanguage::WrappedSelectUngroupedScan(WrappedSelectUngroupedScan(true)) => 1,
191196
_ => 0,
@@ -215,6 +220,7 @@ impl BestCubePlan {
215220
ungrouped_aggregates: 0,
216221
wrapper_nodes,
217222
joins,
223+
wrapped_select_non_push_to_cube,
218224
wrapped_select_ungrouped_scan,
219225
empty_wrappers: 0,
220226
ast_size_outside_wrapper: 0,
@@ -239,6 +245,7 @@ impl BestCubePlan {
239245
/// - `member_errors` > `wrapper_nodes` - use SQL push down where possible if cube scan can't be detected
240246
/// - `non_pushed_down_window` > `wrapper_nodes` - prefer to always push down window functions
241247
/// - `non_pushed_down_limit_sort` > `wrapper_nodes` - prefer to always push down limit-sort expressions
248+
/// - `wrapped_select_non_push_to_cube` > `wrapped_select_ungrouped_scan` - otherwise cost would prefer any aggregation, even non-push-to-Cube
242249
/// - match errors by priority - optimize for more specific errors
243250
#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq)]
244251
pub struct CubePlanCost {
@@ -256,6 +263,7 @@ pub struct CubePlanCost {
256263
joins: usize,
257264
wrapper_nodes: i64,
258265
ast_size_outside_wrapper: usize,
266+
wrapped_select_non_push_to_cube: usize,
259267
wrapped_select_ungrouped_scan: usize,
260268
filters: i64,
261269
structure_points: i64,
@@ -382,6 +390,8 @@ impl CubePlanCost {
382390
+ other.ast_size_outside_wrapper,
383391
ungrouped_aggregates: self.ungrouped_aggregates + other.ungrouped_aggregates,
384392
wrapper_nodes: self.wrapper_nodes + other.wrapper_nodes,
393+
wrapped_select_non_push_to_cube: self.wrapped_select_non_push_to_cube
394+
+ other.wrapped_select_non_push_to_cube,
385395
wrapped_select_ungrouped_scan: self.wrapped_select_ungrouped_scan
386396
+ other.wrapped_select_ungrouped_scan,
387397
cube_scan_nodes: self.cube_scan_nodes + other.cube_scan_nodes,
@@ -468,6 +478,7 @@ impl CubePlanCost {
468478
} + self.ungrouped_aggregates,
469479
unwrapped_subqueries: self.unwrapped_subqueries,
470480
wrapper_nodes: self.wrapper_nodes,
481+
wrapped_select_non_push_to_cube: self.wrapped_select_non_push_to_cube,
471482
wrapped_select_ungrouped_scan: self.wrapped_select_ungrouped_scan,
472483
cube_scan_nodes: self.cube_scan_nodes,
473484
ast_size_without_alias: self.ast_size_without_alias,

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1417,3 +1417,41 @@ async fn wrapper_agg_dimension_over_limit() {
14171417
.sql
14181418
.contains("\"ungrouped\": true"));
14191419
}
1420+
1421+
/// Simple wrapper with cast should have explicit members, not zero
1422+
#[tokio::test]
1423+
async fn wrapper_cast_limit_explicit_members() {
1424+
if !Rewriter::sql_push_down_enabled() {
1425+
return;
1426+
}
1427+
init_testing_logger();
1428+
1429+
let query_plan = convert_select_to_query_plan(
1430+
// language=PostgreSQL
1431+
r#"
1432+
SELECT
1433+
CAST(dim_date0 AS DATE) AS "dim_date0"
1434+
FROM
1435+
MultiTypeCube
1436+
LIMIT 10
1437+
;
1438+
"#
1439+
.to_string(),
1440+
DatabaseProtocol::PostgreSQL,
1441+
)
1442+
.await;
1443+
1444+
let physical_plan = query_plan.as_physical_plan().await.unwrap();
1445+
println!(
1446+
"Physical plan: {}",
1447+
displayable(physical_plan.as_ref()).indent()
1448+
);
1449+
1450+
// Query should mention just a single member
1451+
let request = query_plan
1452+
.as_logical_plan()
1453+
.find_cube_scan_wrapped_sql()
1454+
.request;
1455+
assert_eq!(request.measures.unwrap().len(), 1);
1456+
assert_eq!(request.dimensions.unwrap().len(), 0);
1457+
}

0 commit comments

Comments
 (0)