diff --git a/docs-mintlify/docs/explore-analyze/dashboards/widgets/index.mdx b/docs-mintlify/docs/explore-analyze/dashboards/widgets/index.mdx index afeac4e2a1ebc..ca5fd69109586 100644 --- a/docs-mintlify/docs/explore-analyze/dashboards/widgets/index.mdx +++ b/docs-mintlify/docs/explore-analyze/dashboards/widgets/index.mdx @@ -13,7 +13,7 @@ The dashboard builder supports the following widget types: - [Text](/docs/explore-analyze/dashboards/widgets/text) — Add titles, descriptions, and rich formatting in Markdown - [Controls](/docs/explore-analyze/dashboards/widgets/controls) — Let viewers filter the data, switch the time granularity, or drive several controls at once - [AI summary](/docs/explore-analyze/dashboards/widgets/ai-summary) — Generate narrative summaries of dashboard data on demand -- [Spacer & Divider](/docs/explore-analyze/dashboards/widgets/layout) — Non-data layout elements for whitespace and section breaks (in preview) +- [Spacer & Divider](/docs/explore-analyze/dashboards/widgets/layout) — Non-data layout elements for whitespace and section breaks ## Adding widgets @@ -26,22 +26,10 @@ Each item — a toolbar button, or an option inside the **Add Widgets** / **Add Starting a drag from a menu option closes the menu, so it doesn't cover the canvas while you place the widget. - - -Dragging a toolbar item to place it exactly (drag-to-place) is currently in preview, and the behavior may still change. Reach out to the [Cube support team](/admin/account-billing/support) to activate it for your account. Clicking to add a widget is available to everyone. - - - ## Arranging widgets Drag any widget to move it, and use the handle in its bottom-right corner to resize it — the surrounding widgets shift to make room. - - -Selecting multiple widgets and moving or deleting them as a group is currently in preview, and the behavior may still change. Reach out to the [Cube support team](/admin/account-billing/support) to activate it for your account. - - - To work with several widgets at once, select them first: - **Click** a widget to select it. diff --git a/docs-mintlify/docs/explore-analyze/dashboards/widgets/layout.mdx b/docs-mintlify/docs/explore-analyze/dashboards/widgets/layout.mdx index ccf61535a675b..21864897470ab 100644 --- a/docs-mintlify/docs/explore-analyze/dashboards/widgets/layout.mdx +++ b/docs-mintlify/docs/explore-analyze/dashboards/widgets/layout.mdx @@ -3,12 +3,6 @@ title: Spacer & Divider description: Non-data layout elements — a spacer for whitespace and a divider line — that help you structure a dashboard. --- - - -Spacer and divider widgets are currently in preview, and their behavior may still change. Reach out to the [Cube support team](/admin/account-billing/support) to activate them for your account. - - - Spacer and divider are non-data **layout** widgets. They carry no data of their own; you place them on the canvas alongside charts, text, and controls to add whitespace and visual structure to a dashboard. ## Spacer diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 5649c9bc9554a..77a772f98fd57 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -4521,8 +4521,27 @@ export class BaseQuery { FLOOR: 'FLOOR({{ args_concat }})', CEIL: 'CEIL({{ args_concat }})', TRUNC: 'TRUNC({{ args_concat }})', + // Window functions. The SQL API resolves a window function to `functions/` + // just like any other function, so a built-in without an entry here is never + // pushed down: the window, and everything computed on top of it, is left to + // post processing over a row-capped result. These are the SQL:2003 window + // functions, supported by every dialect that supports windowing at all, so they + // live in the base and a dialect missing one deletes it. LAG: 'LAG({{ args_concat }})', LEAD: 'LEAD({{ args_concat }})', + ROW_NUMBER: 'ROW_NUMBER({{ args_concat }})', + RANK: 'RANK({{ args_concat }})', + DENSE_RANK: 'DENSE_RANK({{ args_concat }})', + PERCENT_RANK: 'PERCENT_RANK({{ args_concat }})', + CUME_DIST: 'CUME_DIST({{ args_concat }})', + // Unreachable from the SQL API until CORE-831: DataFusion types NTILE's argument + // `Exact([UInt64])` and will not coerce an integer literal to it, so the query + // fails to plan. Kept because the template itself is right - once the fork's + // signature is relaxed, NTILE pushes down with no change here. + NTILE: 'NTILE({{ args_concat }})', + FIRST_VALUE: 'FIRST_VALUE({{ args_concat }})', + LAST_VALUE: 'LAST_VALUE({{ args_concat }})', + NTH_VALUE: 'NTH_VALUE({{ args_concat }})', // There is a difference in behaviour of these function processing in different DBs and DWHs. // The SQL standard requires greatest and least to return null in case one argument is null. diff --git a/packages/cubejs-schema-compiler/src/adapter/MongoBiQuery.ts b/packages/cubejs-schema-compiler/src/adapter/MongoBiQuery.ts index 497ddcc4df59a..c443d7f4ecee4 100644 --- a/packages/cubejs-schema-compiler/src/adapter/MongoBiQuery.ts +++ b/packages/cubejs-schema-compiler/src/adapter/MongoBiQuery.ts @@ -21,4 +21,23 @@ export class MongoBiQuery extends MysqlQuery { public timeStampCast(value: string): string { return `TIMESTAMP(${value})`; } + + public sqlTemplates() { + const templates = super.sqlTemplates(); + // The BI Connector speaks a MySQL 5.7-era dialect and documents no OVER clause at all, + // so none of the base window functions can render SQL it will run. Leaving them defined + // sends it a syntax error; removing them computes the window in Cube instead. + delete templates.functions.LAG; + delete templates.functions.LEAD; + delete templates.functions.ROW_NUMBER; + delete templates.functions.RANK; + delete templates.functions.DENSE_RANK; + delete templates.functions.PERCENT_RANK; + delete templates.functions.CUME_DIST; + delete templates.functions.NTILE; + delete templates.functions.FIRST_VALUE; + delete templates.functions.LAST_VALUE; + delete templates.functions.NTH_VALUE; + return templates; + } } diff --git a/packages/cubejs-schema-compiler/src/adapter/MssqlQuery.ts b/packages/cubejs-schema-compiler/src/adapter/MssqlQuery.ts index 0eedd8690482d..fb3481b23c80c 100644 --- a/packages/cubejs-schema-compiler/src/adapter/MssqlQuery.ts +++ b/packages/cubejs-schema-compiler/src/adapter/MssqlQuery.ts @@ -278,6 +278,8 @@ export class MssqlQuery extends BaseQuery { // PERCENTILE_CONT works but requires PARTITION BY delete templates.functions.PERCENTILECONT; delete templates.functions.WIDTH_BUCKET; + // T-SQL has every other SQL:2003 window function, but no NTH_VALUE + delete templates.functions.NTH_VALUE; templates.expressions.like = '{{ expr }} {% if negated %}NOT {% endif %}LIKE {{ pattern }}{% if default_escape %} ESCAPE \'\\\'{% endif %}'; delete templates.expressions.ilike; // MSSQL uses + for string concatenation instead of || diff --git a/packages/cubejs-schema-compiler/src/adapter/PrestodbQuery.ts b/packages/cubejs-schema-compiler/src/adapter/PrestodbQuery.ts index 64e9d0c551ec0..0915353387bc1 100644 --- a/packages/cubejs-schema-compiler/src/adapter/PrestodbQuery.ts +++ b/packages/cubejs-schema-compiler/src/adapter/PrestodbQuery.ts @@ -169,7 +169,12 @@ export class PrestodbQuery extends BaseQuery { templates.functions.UTCTIMESTAMP = 'CAST(NOW() AT TIME ZONE \'UTC\' AS TIMESTAMP)'; templates.functions.TRUNC = 'TRUNCATE({{ args_concat }})'; templates.functions.STRING_AGG = 'ARRAY_JOIN(ARRAY_AGG({% if distinct %}DISTINCT {% endif %}{{ args[0] }}), COALESCE({{ args[1] }}, \'\'))'; + // Presto has no exact percentile aggregate, so PERCENTILE_CONT cannot be pushed + // down. APPROX_PERCENTILE is the approximate one it does have, and it is the only + // way to evaluate a median here - including the one DataFusion rewrites + // APPROX_MEDIAN(expr) into, APPROXPERCENTILECONT(expr, 0.5). delete templates.functions.PERCENTILECONT; + templates.functions.APPROXPERCENTILECONT = 'APPROX_PERCENTILE({{ args_concat }})'; templates.statements.select = '{% if ctes %} WITH \n' + '{{ ctes | join(\',\n\') }}\n' + '{% endif %}' + diff --git a/packages/cubejs-testing-shared/src/db-container-runners/mysql.ts b/packages/cubejs-testing-shared/src/db-container-runners/mysql.ts index 0bb6e9b41fdf2..c61e60c39cede 100644 --- a/packages/cubejs-testing-shared/src/db-container-runners/mysql.ts +++ b/packages/cubejs-testing-shared/src/db-container-runners/mysql.ts @@ -10,14 +10,34 @@ export class MysqlDBRunner extends DbRunnerAbstract { .withEnvironment({ MYSQL_ROOT_PASSWORD: process.env.TEST_DB_PASSWORD || 'Test1test', }) + // On a fresh volume MySQL initializes the data directory before it starts + // listening, which on a loaded CI runner takes well over the 20s the old + // budget (10s start period + 3 retries * 5s) allowed - Docker then marks + // the container unhealthy and testcontainers gives up at once, regardless + // of the startup timeout. Probes that fail inside the start period do not + // count against `retries`, and the container is reported healthy as soon + // as one passes, so a generous start period costs nothing when MySQL comes + // up quickly and only buys time when it does not. + // + // The probe must go over TCP, hence `-h 127.0.0.1` rather than `-h + // localhost`, which the MySQL client resolves to the Unix socket. The + // entrypoint initializes the data directory by running a temporary server + // with `--skip-networking` on that socket, and `mysqladmin ping` exits 0 + // even on `Access denied` - the server answered - so a socket probe can + // report healthy mid-initialization, only for that temporary server to be + // stopped underneath the connecting test. Over TCP the temporary server + // cannot answer, so healthy means the real one is up on the mapped port. .withHealthCheck({ - test: ['CMD-SHELL', 'mysqladmin ping -h localhost'], + test: ['CMD-SHELL', 'mysqladmin ping -h 127.0.0.1'], interval: 5 * 1000, - timeout: 2 * 1000, + timeout: 5 * 1000, retries: 3, - startPeriod: 10 * 1000, + startPeriod: 60 * 1000, }) .withWaitStrategy(Wait.forHealthCheck()) + // Must outlast the health check budget above, otherwise testcontainers' + // 60s default would cut the wait short before Docker has given up. + .withStartupTimeout(120 * 1000) .withExposedPorts(3306); if (options.volumes) { diff --git a/rust/cubesql/cubesql/src/compile/mod.rs b/rust/cubesql/cubesql/src/compile/mod.rs index 19e2d39b878fb..a66370f23d007 100644 --- a/rust/cubesql/cubesql/src/compile/mod.rs +++ b/rust/cubesql/cubesql/src/compile/mod.rs @@ -9110,23 +9110,27 @@ ORDER BY "source"."str0" ASC .await .as_logical_plan(); + // QuickSight's $RANK_1 is a window function over an unlimited query, so the whole + // statement is pushed to the data source rather than ranking a row-capped result + let request = logical_plan.find_cube_scan_wrapped_sql().request; assert_eq!( - logical_plan.find_cube_scan().request, - V1LoadRequestQuery { - measures: Some(vec!["KibanaSampleDataEcommerce.count".to_string()]), - dimensions: Some(vec!["KibanaSampleDataEcommerce.customer_gender".to_string()]), - segments: Some(vec![]), - order: Some(vec![]), - filters: Some(vec![V1LoadRequestQueryFilterItem { - member: Some("KibanaSampleDataEcommerce.customer_gender".to_string()), - operator: Some("startsWith".to_string()), - values: Some(vec!["f".to_string()]), - or: None, - and: None, - }]), - ..Default::default() - } - ) + member_expression_sql(&request.measures), + vec![ + "${KibanaSampleDataEcommerce.count}", + "DENSE_RANK() OVER (ORDER BY ${KibanaSampleDataEcommerce.customer_gender} DESC)", + ] + ); + assert_eq!( + member_expression_sql(&request.dimensions), + vec!["${KibanaSampleDataEcommerce.customer_gender}"] + ); + // LEFT(...) = 'f' is only recognised as a `startsWith` filter on the member query + // path; pushed down it stays the expression QuickSight wrote + assert_eq!( + member_expression_sql(&request.segments), + vec!["(LEFT(${KibanaSampleDataEcommerce.customer_gender}, 1) = $0$)"] + ); + assert_eq!(request.filters, None); } #[tokio::test] @@ -9149,23 +9153,25 @@ ORDER BY "source"."str0" ASC .await .as_logical_plan(); + // QuickSight's $RANK_1 is a window function over an unlimited query, so the whole + // statement is pushed to the data source rather than ranking a row-capped result + let request = logical_plan.find_cube_scan_wrapped_sql().request; assert_eq!( - logical_plan.find_cube_scan().request, - V1LoadRequestQuery { - measures: Some(vec!["KibanaSampleDataEcommerce.count".to_string()]), - dimensions: Some(vec!["KibanaSampleDataEcommerce.customer_gender".to_string()]), - segments: Some(vec![]), - order: Some(vec![]), - filters: Some(vec![V1LoadRequestQueryFilterItem { - member: Some("KibanaSampleDataEcommerce.customer_gender".to_string()), - operator: Some("endsWith".to_string()), - values: Some(vec!["le".to_string()]), - or: None, - and: None, - }]), - ..Default::default() - } - ) + member_expression_sql(&request.measures), + vec![ + "${KibanaSampleDataEcommerce.count}", + "DENSE_RANK() OVER (ORDER BY ${KibanaSampleDataEcommerce.customer_gender} DESC)", + ] + ); + assert_eq!( + member_expression_sql(&request.dimensions), + vec!["${KibanaSampleDataEcommerce.customer_gender}"] + ); + assert_eq!( + member_expression_sql(&request.segments), + vec!["(RIGHT(${KibanaSampleDataEcommerce.customer_gender}, 2) = $0$)"] + ); + assert_eq!(request.filters, None); } #[tokio::test] @@ -9192,23 +9198,36 @@ ORDER BY "source"."str0" ASC .await .as_logical_plan(); + // QuickSight's $RANK_1 is a window function over an unlimited query, so the whole + // statement is pushed to the data source rather than ranking a row-capped result + let request = logical_plan.find_cube_scan_wrapped_sql().request; assert_eq!( - logical_plan.find_cube_scan().request, - V1LoadRequestQuery { - measures: Some(vec!["KibanaSampleDataEcommerce.count".to_string()]), - dimensions: Some(vec!["KibanaSampleDataEcommerce.customer_gender".to_string()]), - segments: Some(vec![]), - order: Some(vec![]), - filters: Some(vec![V1LoadRequestQueryFilterItem { - member: Some("KibanaSampleDataEcommerce.customer_gender".to_string()), - operator: Some("contains".to_string()), - values: Some(vec!["al".to_string()]), - or: None, - and: None, - }]), - ..Default::default() - } - ) + member_expression_sql(&request.measures), + vec![ + "${KibanaSampleDataEcommerce.count}", + "DENSE_RANK() OVER (ORDER BY ${KibanaSampleDataEcommerce.customer_gender} DESC)", + ] + ); + assert_eq!( + member_expression_sql(&request.dimensions), + vec!["${KibanaSampleDataEcommerce.customer_gender}"] + ); + // The strpos(...) shape is still recognised as a member filter, so it survives the + // push down as one + assert_eq!( + member_expression_sql(&request.segments), + Vec::::new() + ); + assert_eq!( + request.filters, + Some(vec![V1LoadRequestQueryFilterItem { + member: Some("KibanaSampleDataEcommerce.customer_gender".to_string()), + operator: Some("contains".to_string()), + values: Some(vec!["al".to_string()]), + or: None, + and: None, + }]) + ); } #[tokio::test] @@ -9236,32 +9255,43 @@ ORDER BY "source"."str0" ASC .await .as_logical_plan(); + // QuickSight's $RANK_1 is a window function over an unlimited query, so the whole + // statement is pushed to the data source rather than ranking a row-capped result + let request = logical_plan.find_cube_scan_wrapped_sql().request; assert_eq!( - logical_plan.find_cube_scan().request, - V1LoadRequestQuery { - measures: Some(vec!["KibanaSampleDataEcommerce.count".to_string()]), - dimensions: Some(vec!["KibanaSampleDataEcommerce.customer_gender".to_string()]), - segments: Some(vec![]), - order: Some(vec![]), - filters: Some(vec![ - V1LoadRequestQueryFilterItem { - member: Some("KibanaSampleDataEcommerce.customer_gender".to_string()), - operator: Some("notContains".to_string()), - values: Some(vec!["al".to_string()]), - or: None, - and: None, - }, - V1LoadRequestQueryFilterItem { - member: Some("KibanaSampleDataEcommerce.customer_gender".to_string()), - operator: Some("set".to_string()), - values: None, - or: None, - and: None, - }, - ]), - ..Default::default() - } - ) + member_expression_sql(&request.measures), + vec![ + "${KibanaSampleDataEcommerce.count}", + "DENSE_RANK() OVER (ORDER BY ${KibanaSampleDataEcommerce.customer_gender} DESC)", + ] + ); + assert_eq!( + member_expression_sql(&request.dimensions), + vec!["${KibanaSampleDataEcommerce.customer_gender}"] + ); + assert_eq!( + member_expression_sql(&request.segments), + Vec::::new() + ); + assert_eq!( + request.filters, + Some(vec![ + V1LoadRequestQueryFilterItem { + member: Some("KibanaSampleDataEcommerce.customer_gender".to_string()), + operator: Some("notContains".to_string()), + values: Some(vec!["al".to_string()]), + or: None, + and: None, + }, + V1LoadRequestQueryFilterItem { + member: Some("KibanaSampleDataEcommerce.customer_gender".to_string()), + operator: Some("set".to_string()), + values: None, + or: None, + and: None, + }, + ]) + ); } #[tokio::test] @@ -19133,12 +19163,14 @@ LIMIT {{ limit }}{% endif %}"#.to_string(), "KibanaSampleDataEcommerce.taxful_total_price".to_string(), ]) ); - // The dedupe must be present in the plan; previously DISTINCT ON was - // silently dropped + // The dedupe must be present; previously DISTINCT ON was silently dropped. The + // ranking window is pushed to the data source, so it shows up in the generated + // SQL rather than as a DataFusion node + let sql = logical_plan.find_cube_scan_wrapped_sql().wrapped_sql.sql; assert!( - format!("{:?}", logical_plan).contains("ROW_NUMBER() PARTITION BY"), - "plan must contain the DISTINCT ON window: {:?}", - logical_plan + sql.contains("ROW_NUMBER() OVER (PARTITION BY"), + "generated SQL must contain the DISTINCT ON window: {}", + sql ); } diff --git a/rust/cubesql/cubesql/src/compile/rewrite/converter.rs b/rust/cubesql/cubesql/src/compile/rewrite/converter.rs index 2d7e53d3e5ac7..5210fd031bc25 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/converter.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/converter.rs @@ -2323,15 +2323,26 @@ impl LanguageToLogicalPlanConverter { without_window_fields.clone(), HashMap::new(), )?); + // The name DataFusion derived for a window expression is what a filter or a + // projection above this select refers to it by, and it is built out of the + // expression as it was written. Take it before flattening qualified columns: + // that rewrite aliases a column back to its unqualified name, which renames + // the window column - `LAG(ta_3.ca_1) OVER (...)` becomes `LAG(ca_1) OVER + // (...)` - and leaves everything above pointing at a field that is no longer + // in the schema. + let window_expr_names = window_expr + .iter() + .map(|e| e.name(&without_window_fields_schema)) + .collect::, _>>()?; let window_expr_rebased = replace_qualified_col_with_flat_name_if_missing( window_expr, &without_window_fields_schema, true, )? - .iter() - .map(|e| { - let original_expr_name = e.name(&without_window_fields_schema)?; - let new_expr = match replace_col_to_expr(e.clone(), &replace_map)? { + .into_iter() + .zip(window_expr_names) + .map(|(e, original_expr_name)| { + let new_expr = match replace_col_to_expr(e, &replace_map)? { Expr::Alias(expr, _) => Expr::Alias(expr, original_expr_name), expr => Expr::Alias(Box::new(expr), original_expr_name), }; diff --git a/rust/cubesql/cubesql/src/compile/rewrite/mod.rs b/rust/cubesql/cubesql/src/compile/rewrite/mod.rs index e0b50cd040c38..e860285ae4c40 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/mod.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/mod.rs @@ -1520,6 +1520,20 @@ fn agg_fun_expr( args: Vec, distinct: impl Display, within_group: impl Display, +) -> String { + agg_fun_expr_var_arg( + fun_name, + list_expr("AggregateFunctionExprArgs", args), + distinct, + within_group, + ) +} + +fn agg_fun_expr_var_arg( + fun_name: impl Display, + arg_list: impl Display, + distinct: impl Display, + within_group: impl Display, ) -> String { let prefix = if fun_name.to_string().starts_with("?") { "" @@ -1528,14 +1542,18 @@ fn agg_fun_expr( }; format!( "(AggregateFunctionExpr {}{} {} {} {})", - prefix, - fun_name, - list_expr("AggregateFunctionExprArgs", args), - distinct, - within_group, + prefix, fun_name, arg_list, distinct, within_group, ) } +fn agg_fun_expr_args(left: impl Display, right: impl Display) -> String { + format!("(AggregateFunctionExprArgs {} {})", left, right) +} + +fn agg_fun_expr_args_empty_tail() -> String { + "AggregateFunctionExprArgs".to_string() +} + fn agg_fun_expr_within_group(left: impl Display, right: impl Display) -> String { format!("(AggregateFunctionExprWithinGroup {} {})", left, right) } diff --git a/rust/cubesql/cubesql/src/compile/rewrite/rules/wrapper/aggregate_function.rs b/rust/cubesql/cubesql/src/compile/rewrite/rules/wrapper/aggregate_function.rs index b44331214dcbf..35b7f4517b685 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/rules/wrapper/aggregate_function.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/rules/wrapper/aggregate_function.rs @@ -1,6 +1,7 @@ use crate::{ compile::rewrite::{ - agg_fun_expr, agg_fun_expr_within_group, agg_fun_expr_within_group_empty_tail, rewrite, + agg_fun_expr_args, agg_fun_expr_args_empty_tail, agg_fun_expr_var_arg, + agg_fun_expr_within_group, agg_fun_expr_within_group_empty_tail, rewrite, rewriter::{CubeEGraph, CubeRewrite}, rules::wrapper::WrapperRules, transforming_rewrite, wrapper_pullup_replacer, wrapper_pushdown_replacer, @@ -15,25 +16,28 @@ use egg::Subst; impl WrapperRules { pub fn aggregate_function_rules(&self, rules: &mut Vec) { rules.extend(vec![ + // Matched over the whole argument list rather than a single argument: an + // aggregate can take more than one, as `APPROX_PERCENTILE_CONT(expr, 0.5)` + // does, and a one-argument pattern silently leaves those behind rewrite( "wrapper-push-down-aggregate-function", wrapper_pushdown_replacer( - agg_fun_expr("?fun", vec!["?expr"], "?distinct", "?within_group"), + agg_fun_expr_var_arg("?fun", "?args", "?distinct", "?within_group"), "?context", ), - agg_fun_expr( + agg_fun_expr_var_arg( "?fun", - vec![wrapper_pushdown_replacer("?expr", "?context")], + wrapper_pushdown_replacer("?args", "?context"), "?distinct", wrapper_pushdown_replacer("?within_group", "?context"), ), ), transforming_rewrite( "wrapper-pull-up-aggregate-function", - agg_fun_expr( + agg_fun_expr_var_arg( "?fun", - vec![wrapper_pullup_replacer( - "?expr", + wrapper_pullup_replacer( + "?args", wrapper_replacer_context( "?alias_to_cube", "?push_to_cube", @@ -43,7 +47,7 @@ impl WrapperRules { "?ungrouped_scan", "?input_data_source", ), - )], + ), "?distinct", wrapper_pullup_replacer( "?within_group", @@ -59,7 +63,7 @@ impl WrapperRules { ), ), wrapper_pullup_replacer( - agg_fun_expr("?fun", vec!["?expr"], "?distinct", "?within_group"), + agg_fun_expr_var_arg("?fun", "?args", "?distinct", "?within_group"), wrapper_replacer_context( "?alias_to_cube", "?push_to_cube", @@ -98,6 +102,27 @@ impl WrapperRules { ), wrapper_pullup_replacer(agg_fun_expr_within_group("?left", "?right"), "?context"), ), + rewrite( + "wrapper-push-down-aggregate-function-args", + wrapper_pushdown_replacer(agg_fun_expr_args("?left", "?right"), "?context"), + agg_fun_expr_args( + wrapper_pushdown_replacer("?left", "?context"), + wrapper_pushdown_replacer("?right", "?context"), + ), + ), + rewrite( + "wrapper-pull-up-aggregate-function-args", + agg_fun_expr_args( + wrapper_pullup_replacer("?left", "?context"), + wrapper_pullup_replacer("?right", "?context"), + ), + wrapper_pullup_replacer(agg_fun_expr_args("?left", "?right"), "?context"), + ), + rewrite( + "wrapper-push-down-aggregate-function-args-empty-tail", + wrapper_pushdown_replacer(agg_fun_expr_args_empty_tail(), "?context"), + wrapper_pullup_replacer(agg_fun_expr_args_empty_tail(), "?context"), + ), ]); } diff --git a/rust/cubesql/cubesql/src/compile/test/mod.rs b/rust/cubesql/cubesql/src/compile/test/mod.rs index ecf1f645c10ae..0c3953ec05260 100644 --- a/rust/cubesql/cubesql/src/compile/test/mod.rs +++ b/rust/cubesql/cubesql/src/compile/test/mod.rs @@ -693,6 +693,15 @@ pub fn sql_generator( ("functions/TRUNC".to_string(), "TRUNC({{ args_concat }})".to_string()), ("functions/LAG".to_string(), "LAG({{ args_concat }})".to_string()), ("functions/LEAD".to_string(), "LEAD({{ args_concat }})".to_string()), + ("functions/ROW_NUMBER".to_string(), "ROW_NUMBER({{ args_concat }})".to_string()), + ("functions/RANK".to_string(), "RANK({{ args_concat }})".to_string()), + ("functions/DENSE_RANK".to_string(), "DENSE_RANK({{ args_concat }})".to_string()), + ("functions/PERCENT_RANK".to_string(), "PERCENT_RANK({{ args_concat }})".to_string()), + ("functions/CUME_DIST".to_string(), "CUME_DIST({{ args_concat }})".to_string()), + ("functions/NTILE".to_string(), "NTILE({{ args_concat }})".to_string()), + ("functions/FIRST_VALUE".to_string(), "FIRST_VALUE({{ args_concat }})".to_string()), + ("functions/LAST_VALUE".to_string(), "LAST_VALUE({{ args_concat }})".to_string()), + ("functions/NTH_VALUE".to_string(), "NTH_VALUE({{ args_concat }})".to_string()), ("functions/LEAST".to_string(), "LEAST({{ args_concat }})".to_string()), ("functions/DATEDIFF".to_string(), "DATEDIFF({{ date_part }}, {{ args[1] }}, {{ args[2] }})".to_string()), ("functions/CURRENTDATE".to_string(), "CURRENT_DATE({{ args_concat }})".to_string()), diff --git a/rust/cubesql/cubesql/src/compile/test/test_wrapper.rs b/rust/cubesql/cubesql/src/compile/test/test_wrapper.rs index c03599644a7d6..21a002f84c03b 100644 --- a/rust/cubesql/cubesql/src/compile/test/test_wrapper.rs +++ b/rust/cubesql/cubesql/src/compile/test/test_wrapper.rs @@ -15,9 +15,10 @@ use crate::{ rewrite::rewriter::Rewriter, test::{ convert_select_to_query_plan, convert_select_to_query_plan_customized, - convert_select_to_query_plan_with_config, convert_sql_to_cube_query, - get_test_session_with_config, get_test_tenant_ctx_with_cube_data_sources, - init_testing_logger, member_expression_sql, LogicalPlanTestUtils, TestContext, + convert_select_to_query_plan_with_config, convert_sql_to_cube_query, get_test_session, + get_test_session_with_config, get_test_tenant_ctx, + get_test_tenant_ctx_with_cube_data_sources, init_testing_logger, member_expression_sql, + LogicalPlanTestUtils, TestContext, }, DatabaseProtocol, }, @@ -3853,3 +3854,351 @@ async fn test_wrapper_union_in_join_push_down() { sql ); } + +/// A window function is looked up as `functions/` like any other function, so a +/// built-in without a template is never pushed down - and because the window sits under +/// everything else in the query, that leaves the whole plan above it to post processing. +/// `LAG` and `LEAD` were once the only built-ins with a template, which left a query as +/// ordinary as `ROW_NUMBER() OVER (...)` reading a row-capped scan. +#[tokio::test] +async fn test_wrapper_built_in_window_functions() { + if !Rewriter::sql_push_down_enabled() { + return; + } + init_testing_logger(); + + for (call, expected) in [ + ("ROW_NUMBER()", "ROW_NUMBER()"), + ("RANK()", "RANK()"), + ("DENSE_RANK()", "DENSE_RANK()"), + ("PERCENT_RANK()", "PERCENT_RANK()"), + ("CUME_DIST()", "CUME_DIST()"), + // NTILE is left out - see test_wrapper_ntile_does_not_plan, which pins why and + // goes red when it can be added back here as ("NTILE(4)", "NTILE(4)") + ("FIRST_VALUE(notes)", "FIRST_VALUE("), + ("LAST_VALUE(notes)", "LAST_VALUE("), + ("NTH_VALUE(notes, 2)", "NTH_VALUE("), + ("LAG(notes)", "LAG("), + ("LEAD(notes)", "LEAD("), + ] { + let query_plan = convert_select_to_query_plan( + format!( + r#" + SELECT + customer_gender, + {call} OVER ( + PARTITION BY customer_gender + ORDER BY notes + ) AS w + FROM ( + SELECT customer_gender, notes + FROM KibanaSampleDataEcommerce + GROUP BY 1, 2 + ) t + "# + ), + DatabaseProtocol::PostgreSQL, + ) + .await; + + let logical_plan = query_plan.as_logical_plan(); + let sql = logical_plan.find_cube_scan_wrapped_sql().wrapped_sql.sql; + assert!( + sql.contains(expected), + "{} is not pushed down, generated SQL: {}", + call, + sql + ); + } +} + +/// The reported shape: a CTE that sequences rows with `LAG` and `ROW_NUMBER`, a second CTE +/// deriving values from them, and an aggregation over the result. Without a `ROW_NUMBER` +/// template the window blocked the push down of everything above it, and the final +/// aggregate ran in DataFusion over the capped rows of a raw scan. +#[tokio::test] +async fn test_wrapper_window_sequenced_cte_aggregation() { + if !Rewriter::sql_push_down_enabled() { + return; + } + init_testing_logger(); + + let query_plan = convert_select_to_query_plan( + // language=PostgreSQL + r#" + WITH sequenced AS ( + SELECT + customer_gender, + notes, + CAST(order_date AS DATE) AS order_date, + CAST(taxful_total_price AS DECIMAL(18,2)) AS price, + LAG(CAST(order_date AS DATE)) OVER ( + PARTITION BY customer_gender + ORDER BY CAST(order_date AS DATE), notes + ) AS prev_order_date, + ROW_NUMBER() OVER ( + PARTITION BY customer_gender + ORDER BY CAST(order_date AS DATE), notes + ) AS order_seq + FROM KibanaSampleDataEcommerce + WHERE customer_gender = 'female' + ), + gaps AS ( + SELECT + *, + DATEDIFF('day', prev_order_date, order_date) AS days_since, + CASE + WHEN prev_order_date IS NULL THEN '1. first' + WHEN DATEDIFF('day', prev_order_date, order_date) > 180 THEN '3. gap' + ELSE '2. continuous' + END AS category + FROM sequenced + ) + SELECT + customer_gender, + category, + COUNT(DISTINCT notes) AS notes_count, + ROUND(AVG(days_since), 2) AS avg_gap, + SUM(price) AS total_price + FROM gaps + GROUP BY customer_gender, category + ORDER BY customer_gender, notes_count DESC + "# + .to_string(), + DatabaseProtocol::PostgreSQL, + ) + .await; + + // The whole query reaches the data source: nothing is left above the wrapper + let logical_plan = query_plan.as_logical_plan(); + let sql = logical_plan + .find_cube_scan_wrapped_sql_deep() + .wrapped_sql + .sql; + for expected in [ + "ROW_NUMBER() OVER", + "LAG(", + "DATEDIFF(", + "COUNT(DISTINCT", + "GROUP BY", + "ORDER BY", + ] { + assert!( + sql.contains(expected), + "no {} in generated SQL: {}", + expected, + sql + ); + } + + // Only the projection renaming the pushed down columns is left on top of the wrapper: + // nothing that would read the row-capped result of an unlimited query + let plan = format!("{:?}", logical_plan); + for unexpected in ["WindowAggr:", "Aggregate:", "Sort:", "Filter:"] { + assert!( + !plan.contains(unexpected), + "{} left in plan: {}", + unexpected, + plan + ); + } +} + +/// A window column is referred to by the name DataFusion derived for the window expression, +/// so the wrapped select has to keep that name when it takes the window over. Flattening a +/// qualified column inside the expression renames it - `LAG(ta_3.ca_1) OVER (...)` becomes +/// `LAG(ca_1) OVER (...)` - and a filter above is then left pointing at a field that is not +/// in the schema, which fails the whole plan rather than falling back to post processing. +#[tokio::test] +async fn test_wrapper_filter_on_window_over_grouped_join() { + if !Rewriter::sql_push_down_enabled() { + return; + } + init_testing_logger(); + + let query_plan = convert_select_to_query_plan( + // language=PostgreSQL + r#" + WITH + "qt_0" AS ( + SELECT + "ta_1".content "ca_1", + DATE_TRUNC('month', "ta_2".order_date) "ca_2", + CASE WHEN sum("ta_2"."sumPrice") IS NOT NULL THEN sum("ta_2"."sumPrice") ELSE 0 END "ca_3" + FROM KibanaSampleDataEcommerce "ta_2" + JOIN Logs "ta_1" ON "ta_2".__cubeJoinField = "ta_1".__cubeJoinField + GROUP BY "ca_1", "ca_2" + ), + "qt_1" AS ( + SELECT + LAG("ta_3"."ca_1") OVER ( + PARTITION BY DATE_TRUNC('month', "ta_3"."ca_2") + ORDER BY DATE_TRUNC('month', "ta_3"."ca_2"), "ta_3"."ca_1" + ) "ca_4", + DATE_TRUNC('month', "ta_3"."ca_2") "ca_5", + "ta_3"."ca_1" "ca_6" + FROM "qt_0" "ta_3" + GROUP BY "ca_5", "ca_6" + ) + SELECT "ta_4"."ca_5" "ca_7", "ta_4"."ca_6" "ca_8" + FROM "qt_1" "ta_4" + WHERE "ta_4"."ca_4" <= 'x' + "# + .to_string(), + DatabaseProtocol::PostgreSQL, + ) + .await; + + let sql = query_plan + .as_logical_plan() + .find_cube_scan_wrapped_sql() + .wrapped_sql + .sql; + assert!(sql.contains("LAG("), "generated SQL: {}", sql); +} + +/// An aggregate can take more than one argument. `APPROX_PERCENTILE_CONT(expr, 0.5)` is the +/// only way to get a median out of a dialect without an exact percentile aggregate - Presto, +/// Trino and Athena among them - and it is also what DataFusion rewrites `APPROX_MEDIAN(expr)` +/// into, so leaving multi-argument aggregates unpushable left those dialects with no median +/// at all. +#[tokio::test] +async fn test_wrapper_multi_arg_aggregate_function() { + if !Rewriter::sql_push_down_enabled() { + return; + } + init_testing_logger(); + + // Not a base template: only dialects with an approximate percentile define it + let approx_percentile = vec![( + "functions/APPROXPERCENTILECONT".to_string(), + "APPROX_PERCENTILE({{ args_concat }})".to_string(), + )]; + + for call in [ + "APPROX_PERCENTILE_CONT(taxful_total_price, 0.5)", + "APPROX_MEDIAN(taxful_total_price)", + ] { + let query_plan = convert_select_to_query_plan_customized( + format!("SELECT customer_gender, {call} FROM KibanaSampleDataEcommerce GROUP BY 1"), + DatabaseProtocol::PostgreSQL, + approx_percentile.clone(), + ) + .await; + + assert_eq!( + member_expression_sql( + &query_plan + .as_logical_plan() + .find_cube_scan_wrapped_sql() + .request + .measures + ), + vec!["APPROX_PERCENTILE(${KibanaSampleDataEcommerce.taxful_total_price}, 0.5)"], + "{} is not pushed down", + call + ); + } +} + +/// NTILE is the one built-in window function that cannot be pushed down. DataFusion types +/// its argument `Exact([UInt64])` and will not coerce the `Int64` an integer literal plans +/// as, so `NTILE(4)` fails the whole query before rewriting ever sees it. Its SQL template +/// is correct, so the fix belongs in the fork's signature rather than in a cast bolted onto +/// the statement - CORE-831. +/// +/// When this test starts failing, that fix has landed: delete it and add +/// `("NTILE(4)", "NTILE(4)")` to the case list in `test_wrapper_built_in_window_functions`. +/// +/// One other thing can turn it red. `Exact([UInt64])` is the `Debug` rendering of a +/// `TypeSignature`, not a stable string, so a DataFusion bump that reformats the coercion +/// error fails this assertion while NTILE still does not plan. The error text says which +/// happened: if it no longer mentions a coercion at all, the signature was relaxed and the +/// case can move back; if it still refuses the argument in different words, only the +/// assertion needs updating. +#[tokio::test] +async fn test_wrapper_ntile_does_not_plan() { + if !Rewriter::sql_push_down_enabled() { + return; + } + init_testing_logger(); + + let error = convert_sql_to_cube_query( + &r#" + SELECT customer_gender, NTILE(4) OVER (ORDER BY notes) AS w + FROM ( + SELECT customer_gender, notes + FROM KibanaSampleDataEcommerce + GROUP BY 1, 2 + ) t + "# + .to_string(), + get_test_tenant_ctx(), + get_test_session(DatabaseProtocol::PostgreSQL, get_test_tenant_ctx()).await, + ) + .await + .expect_err("NTILE should not plan until CORE-831 lands"); + + assert!( + error.to_string().contains("Exact([UInt64])"), + "unexpected error: {}", + error + ); +} + +/// The two functions disagree about a third argument - DataFusion's weighted variant is +/// `approx_percentile_cont_with_weight(x, w, percentile)` while Presto reads +/// `approx_percentile(x, w, percentage)` - so a template rendering every argument would +/// mis-map one against the other. It cannot: `APPROXPERCENTILECONT` is two arguments here +/// and a third is rejected before rewriting, and the weighted variant is a different +/// function with a template name of its own, which no dialect defines. +#[tokio::test] +async fn test_wrapper_approx_percentile_cont_is_binary() { + if !Rewriter::sql_push_down_enabled() { + return; + } + init_testing_logger(); + + let error = convert_sql_to_cube_query( + &"SELECT customer_gender, APPROX_PERCENTILE_CONT(taxful_total_price, 0.5, 100) FROM KibanaSampleDataEcommerce GROUP BY 1".to_string(), + get_test_tenant_ctx(), + get_test_session(DatabaseProtocol::PostgreSQL, get_test_tenant_ctx()).await, + ) + .await + .expect_err("a third argument should not plan"); + + assert!( + error + .to_string() + .contains("does not accept 3 function arguments"), + "unexpected error: {}", + error + ); +} + +/// A dialect without the template does not get the aggregate. It does not fall back either: +/// nothing rewrites the expression, and an approximate percentile over a dimension is not a +/// Cube measure, so the query ends up with no plan at all. That hard failure - rather than +/// post processing - is what a user on such a dialect hits, so pin the error it fails with +/// instead of accepting any failure at all. +#[tokio::test] +async fn test_wrapper_multi_arg_aggregate_function_without_template() { + if !Rewriter::sql_push_down_enabled() { + return; + } + init_testing_logger(); + + let error = convert_sql_to_cube_query( + &"SELECT customer_gender, APPROX_PERCENTILE_CONT(taxful_total_price, 0.5) FROM KibanaSampleDataEcommerce GROUP BY 1".to_string(), + get_test_tenant_ctx(), + get_test_session(DatabaseProtocol::PostgreSQL, get_test_tenant_ctx()).await, + ) + .await + .expect_err("aggregate without a template should not be pushed down"); + + assert!( + error.to_string().contains("Can't detect Cube query"), + "unexpected error: {}", + error + ); +}