diff --git a/rust/cubesql/cubesql/src/compile/mod.rs b/rust/cubesql/cubesql/src/compile/mod.rs index 6c2f06bb86798..e46131f080d10 100644 --- a/rust/cubesql/cubesql/src/compile/mod.rs +++ b/rust/cubesql/cubesql/src/compile/mod.rs @@ -3878,6 +3878,38 @@ ORDER BY \"COUNT(count)\" DESC" ); } + #[tokio::test] + async fn tableau_mul_null_by_timestamp() { + init_logger(); + + let query_plan = convert_select_to_query_plan( + "SELECT ((CAST('1900-01-01 00:00:00' AS TIMESTAMP) + NULL * INTERVAL '1 DAY') + 1 * INTERVAL '1 DAY') AS \"TEMP(Test)(4169571243)(0)\" FROM \"public\".\"KibanaSampleDataEcommerce\" \"KibanaSampleDataEcommerce\" HAVING (COUNT(1) > 0)".to_string(), + DatabaseProtocol::PostgreSQL, + ).await; + + let logical_plan = query_plan.as_logical_plan(); + assert_eq!( + logical_plan.find_cube_scan().request, + V1LoadRequestQuery { + measures: Some(vec!["KibanaSampleDataEcommerce.count".to_string()]), + segments: Some(vec![]), + dimensions: Some(vec![]), + time_dimensions: None, + order: None, + limit: None, + offset: None, + filters: Some(vec![V1LoadRequestQueryFilterItem { + member: Some("KibanaSampleDataEcommerce.count".to_string()), + operator: Some("gt".to_string()), + values: Some(vec!["0".to_string()]), + or: None, + and: None, + }]), + ungrouped: None, + } + ); + } + #[tokio::test] async fn measure_used_on_dimension() { init_logger(); diff --git a/rust/cubesql/cubesql/src/compile/rewrite/analysis.rs b/rust/cubesql/cubesql/src/compile/rewrite/analysis.rs index 65be174319ca5..e7d83dc5dc094 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/analysis.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/analysis.rs @@ -33,7 +33,7 @@ use std::{fmt::Debug, ops::Index, sync::Arc}; #[derive(Clone, Debug)] pub struct LogicalPlanData { - pub original_expr: Option, + pub original_expr: Option, pub member_name_to_expr: Option, Member, Expr)>>, pub trivial_push_down: Option, pub column: Option, @@ -46,6 +46,12 @@ pub struct LogicalPlanData { pub is_empty_list: Option, } +#[derive(Debug, Clone)] +pub enum OriginalExpr { + Expr(Expr), + List(Vec), +} + #[derive(Debug, Clone)] pub enum ConstantFolding { Scalar(ScalarValue), @@ -230,7 +236,13 @@ impl<'a> Index for SingleNodeIndex<'a> { // "Single node expected but {:?} found", // self.egraph.index(index).nodes // ); - &self.egraph.index(index).nodes[0] + &self + .egraph + .index(index) + .nodes + .iter() + .find(|n| !matches!(n, LogicalPlanLanguage::QueryParam(_))) + .unwrap_or(&self.egraph.index(index).nodes[0]) } } @@ -245,8 +257,8 @@ impl LogicalPlanAnalysis { fn make_original_expr( egraph: &EGraph, enode: &LogicalPlanLanguage, - ) -> Option { - let id_to_expr = |id| { + ) -> Option { + let id_to_original_expr = |id| { egraph[id].data.original_expr.clone().ok_or_else(|| { CubeError::internal(format!( "Original expr wasn't prepared for {:?}", @@ -254,6 +266,15 @@ impl LogicalPlanAnalysis { )) }) }; + let id_to_expr = |id| { + id_to_original_expr(id).and_then(|e| match e { + OriginalExpr::Expr(expr) => Ok(expr), + OriginalExpr::List(_) => Err(CubeError::internal(format!( + "Original expr list can't be used in expr eval {:?}", + egraph[id] + ))), + }) + }; let original_expr = if is_expr_node(enode) { node_to_expr( enode, @@ -262,8 +283,29 @@ impl LogicalPlanAnalysis { &SingleNodeIndex { egraph }, ) .ok() + .map(|expr| OriginalExpr::Expr(expr)) } else { - None + // While not used directly in expression evaluation OriginalExpr::List is used to trigger parent data invalidation + // Going forward should be used for expression evaluation as well + match enode { + LogicalPlanLanguage::CaseExprWhenThenExpr(params) + | LogicalPlanLanguage::CaseExprElseExpr(params) + | LogicalPlanLanguage::CaseExprExpr(params) + | LogicalPlanLanguage::AggregateFunctionExprArgs(params) + | LogicalPlanLanguage::AggregateUDFExprArgs(params) + | LogicalPlanLanguage::ScalarFunctionExprArgs(params) + | LogicalPlanLanguage::ScalarUDFExprArgs(params) => { + let mut list = Vec::new(); + for id in params { + match id_to_original_expr(*id).ok()? { + OriginalExpr::Expr(expr) => list.push(expr), + OriginalExpr::List(exprs) => list.extend(exprs), + } + } + Some(OriginalExpr::List(list)) + } + _ => None, + } }; original_expr } @@ -276,6 +318,7 @@ impl LogicalPlanAnalysis { match enode { LogicalPlanLanguage::ColumnExpr(_) => Some(0), LogicalPlanLanguage::LiteralExpr(_) => Some(0), + LogicalPlanLanguage::QueryParam(_) => Some(0), LogicalPlanLanguage::AliasExpr(params) => trivial_push_down(params[0]), LogicalPlanLanguage::ProjectionExpr(params) | LogicalPlanLanguage::AggregateAggrExpr(params) @@ -333,7 +376,17 @@ impl LogicalPlanAnalysis { ) -> Option, Member, Expr)>> { let column_name = |id| egraph.index(id).data.column.clone(); let id_to_column_name_to_expr = |id| egraph.index(id).data.member_name_to_expr.clone(); - let original_expr = |id| egraph.index(id).data.original_expr.clone(); + let original_expr = |id| { + egraph + .index(id) + .data + .original_expr + .clone() + .and_then(|e| match e { + OriginalExpr::Expr(expr) => Some(expr), + OriginalExpr::List(_) => None, + }) + }; let literal_member_relation = |id| { egraph .index(id) @@ -641,7 +694,17 @@ impl LogicalPlanAnalysis { egraph: &EGraph, enode: &LogicalPlanLanguage, ) -> Option)>> { - let original_expr = |id| egraph.index(id).data.original_expr.clone(); + let original_expr = |id| { + egraph + .index(id) + .data + .original_expr + .clone() + .and_then(|e| match e { + OriginalExpr::Expr(expr) => Some(expr), + OriginalExpr::List(_) => None, + }) + }; let id_to_column_name = |id| egraph.index(id).data.column.clone(); let column_name_to_alias = |id| egraph.index(id).data.expr_to_alias.clone(); let mut map = Vec::new(); @@ -686,7 +749,17 @@ impl LogicalPlanAnalysis { enode: &LogicalPlanLanguage, ) -> Option> { let referenced_columns = |id| egraph.index(id).data.referenced_expr.clone(); - let original_expr = |id| egraph.index(id).data.original_expr.clone(); + let original_expr = |id| { + egraph + .index(id) + .data + .original_expr + .clone() + .and_then(|e| match e { + OriginalExpr::Expr(expr) => Some(expr), + OriginalExpr::List(_) => None, + }) + }; let column_name = |id| egraph.index(id).data.column.clone(); let push_referenced_columns = |id, columns: &mut Vec| -> Option<()> { if let Some(col) = column_name(id) { @@ -769,6 +842,7 @@ impl LogicalPlanAnalysis { Some(vec) } LogicalPlanLanguage::LiteralExpr(_) => Some(vec), + LogicalPlanLanguage::QueryParam(_) => Some(vec), LogicalPlanLanguage::SortExpr(params) => { if column_name(params[0]).is_some() { let expr = original_expr(params[0])?; @@ -821,13 +895,13 @@ impl LogicalPlanAnalysis { }; match enode { LogicalPlanLanguage::LiteralExpr(_) => { - let expr = node_to_expr( + let result = node_to_expr( enode, &egraph.analysis.cube_context, &constant_expr, &SingleNodeIndex { egraph }, - ) - .ok()?; + ); + let expr = result.ok()?; match expr { Expr::Literal(value) => Some(ConstantFolding::Scalar(value)), _ => panic!("Expected Literal but got: {:?}", expr), @@ -1126,7 +1200,7 @@ impl LogicalPlanAnalysis { } } - fn merge_option_field( + fn merge_option_field( &mut self, a: &mut LogicalPlanData, mut b: LogicalPlanData, @@ -1193,7 +1267,9 @@ impl Analysis for LogicalPlanAnalysis { if let Some(ConstantFolding::Scalar(c)) = &egraph[id].data.constant { // As ConstantFolding goes through Alias we can't add LiteralExpr at this level otherwise it gets dropped. // In case there's wrapping node on top of Alias that can be evaluated to LiteralExpr further it gets replaced instead. - if let Some(Expr::Alias(_, _)) = egraph[id].data.original_expr.as_ref() { + if let Some(OriginalExpr::Expr(Expr::Alias(_, _))) = + egraph[id].data.original_expr.as_ref() + { return; } // TODO: ideally all constants should be aliased, but this requires @@ -1210,6 +1286,10 @@ impl Analysis for LogicalPlanAnalysis { .data .original_expr .as_ref() + .and_then(|e| match e { + OriginalExpr::Expr(expr) => Some(expr), + OriginalExpr::List(_) => None, + }) .map(|expr| expr.name(&DFSchema::empty()).unwrap()) } else { None diff --git a/rust/cubesql/cubesql/src/compile/rewrite/mod.rs b/rust/cubesql/cubesql/src/compile/rewrite/mod.rs index 7d680faa3606e..f30282d69b647 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/mod.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/mod.rs @@ -6,7 +6,7 @@ pub mod rewriter; pub mod rules; use crate::{ - compile::rewrite::analysis::{LogicalPlanAnalysis, Member}, + compile::rewrite::analysis::{LogicalPlanAnalysis, Member, OriginalExpr}, CubeError, }; use datafusion::{ @@ -1361,10 +1361,18 @@ pub fn original_expr_name( egraph: &EGraph, id: Id, ) -> Option { - egraph[id].data.original_expr.as_ref().map(|e| match e { - Expr::Column(c) => c.name.to_string(), - _ => e.name(&DFSchema::empty()).unwrap(), - }) + egraph[id] + .data + .original_expr + .as_ref() + .and_then(|e| match e { + OriginalExpr::Expr(e) => Some(e), + _ => None, + }) + .map(|e| match e { + Expr::Column(c) => c.name.to_string(), + _ => e.name(&DFSchema::empty()).unwrap(), + }) } fn search_match_chained<'a>( diff --git a/rust/cubesql/cubesql/src/compile/rewrite/rewriter.rs b/rust/cubesql/cubesql/src/compile/rewrite/rewriter.rs index 467d757911a7e..74af6421c027f 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/rewriter.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/rewriter.rs @@ -245,7 +245,7 @@ impl Rewriter { let (plan, qtrace_egraph_iterations) = tokio::task::spawn_blocking(move || { let (runner, qtrace_egraph_iterations) = - Self::run_rewrites(&cube_context, egraph, rules)?; + Self::run_rewrites(&cube_context, egraph, rules, "intermediate")?; Ok::<_, CubeError>((runner.egraph, qtrace_egraph_iterations)) }) @@ -329,7 +329,7 @@ impl Rewriter { let (plan, qtrace_egraph_iterations, qtrace_best_graph) = tokio::task::spawn_blocking(move || { let (runner, qtrace_egraph_iterations) = - Self::run_rewrites(&cube_context, egraph, rules)?; + Self::run_rewrites(&cube_context, egraph, rules, "final")?; let extractor = Extractor::new(&runner.egraph, BestCubePlan); let (best_cost, best) = extractor.find_best(root); @@ -374,6 +374,7 @@ impl Rewriter { cube_context: &Arc, egraph: EGraph, rules: Arc>>, + stage: &str, ) -> Result<(CubeRunner, Vec), CubeError> { let runner = Self::rewrite_runner(cube_context.clone(), egraph); let runner = runner.run(rules.iter()); @@ -394,20 +395,21 @@ impl Rewriter { } }; if IterInfo::egraph_debug_enabled() { - let _ = fs::create_dir_all("egraph-debug"); - let _ = fs::create_dir_all("egraph-debug/public"); - let _ = fs::create_dir_all("egraph-debug/src"); + let dir = format!("egraph-debug-{}", stage); + let _ = fs::create_dir_all(dir.clone()); + let _ = fs::create_dir_all(format!("{}/public", dir)); + let _ = fs::create_dir_all(format!("{}/src", dir)); fs::copy( "egraph-debug-template/public/index.html", - "egraph-debug/public/index.html", + format!("{}/public/index.html", dir), )?; fs::copy( "egraph-debug-template/package.json", - "egraph-debug/package.json", + format!("{}/package.json", dir), )?; fs::copy( "egraph-debug-template/src/index.js", - "egraph-debug/src/index.js", + format!("{}/src/index.js", dir), )?; let mut iterations = Vec::new(); @@ -451,7 +453,7 @@ impl Rewriter { last_debug_data = Some(debug_data_clone); } fs::write( - "egraph-debug/src/iterations.js", + format!("{}/src/iterations.js", dir), &format!( "export const iterations = {};", serde_json::to_string_pretty(&iterations)? diff --git a/rust/cubesql/cubesql/src/compile/rewrite/rules/dates.rs b/rust/cubesql/cubesql/src/compile/rewrite/rules/dates.rs index 9a1cd58dd79de..5c2d2daf9860a 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/rules/dates.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/rules/dates.rs @@ -2,7 +2,7 @@ use super::utils; use crate::{ compile::rewrite::{ agg_fun_expr, alias_expr, - analysis::{ConstantFolding, LogicalPlanAnalysis}, + analysis::{ConstantFolding, LogicalPlanAnalysis, OriginalExpr}, binary_expr, cast_expr, cast_expr_explicit, column_expr, fun_expr, literal_expr, literal_int, literal_string, negative_expr, rewrite, rewriter::RewriteRules, @@ -611,7 +611,9 @@ impl DateRules { let alias_var = var!(alias_var); move |egraph, root, subst| { for data_type in var_iter!(egraph[subst[data_type_var]], CastExprDataType) { - if let Some(original_expr) = egraph[root].data.original_expr.as_ref() { + if let Some(OriginalExpr::Expr(original_expr)) = + egraph[root].data.original_expr.as_ref() + { let alias = original_expr.name(&DFSchema::empty()).unwrap(); match data_type { DataType::Timestamp(TimeUnit::Nanosecond, None) => { @@ -663,7 +665,9 @@ impl DateRules { { let alias_var = var!(alias_var); move |egraph, root, subst| { - if let Some(original_expr) = egraph[root].data.original_expr.as_ref() { + if let Some(OriginalExpr::Expr(original_expr)) = + egraph[root].data.original_expr.as_ref() + { let alias = original_expr.name(&DFSchema::empty()).unwrap(); subst.insert( alias_var, diff --git a/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs b/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs index 126ef18fab7c8..6c6956a88d2fe 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/rules/filters.rs @@ -2,7 +2,7 @@ use super::utils; use crate::{ compile::rewrite::{ alias_expr, - analysis::{ConstantFolding, LogicalPlanAnalysis}, + analysis::{ConstantFolding, LogicalPlanAnalysis, OriginalExpr}, between_expr, binary_expr, case_expr, case_expr_var_arg, cast_expr, change_user_member, column_expr, cube_scan, cube_scan_filters, cube_scan_filters_empty_tail, cube_scan_members, dimension_expr, expr_column_name, filter, filter_member, filter_op, filter_op_filters, @@ -4436,7 +4436,9 @@ impl FilterRules { let expr_var = var!(expr_var); let data_type_var = var!(data_type_var); move |egraph, subst| { - if let Some(expr) = egraph[subst[expr_var]].data.original_expr.clone() { + if let Some(OriginalExpr::Expr(expr)) = + egraph[subst[expr_var]].data.original_expr.clone() + { for data_type in var_iter!(egraph[subst[data_type_var]], CastExprDataType).cloned() { return match data_type { diff --git a/rust/cubesql/cubesql/src/compile/rewrite/rules/members.rs b/rust/cubesql/cubesql/src/compile/rewrite/rules/members.rs index e236b954381bd..a6999413b4bf1 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/rules/members.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/rules/members.rs @@ -1,7 +1,7 @@ use crate::{ compile::rewrite::{ agg_fun_expr, aggregate, alias_expr, all_members, - analysis::LogicalPlanAnalysis, + analysis::{LogicalPlanAnalysis, OriginalExpr}, binary_expr, cast_expr, change_user_expr, column_expr, column_name_to_member_def_vec, column_name_to_member_to_aliases, column_name_to_member_vec, cross_join, cube_scan, cube_scan_filters_empty_tail, cube_scan_members, cube_scan_members_empty_tail, @@ -1232,7 +1232,7 @@ impl MemberRules { None => continue, }; - if let Ok(expr) = res { + if let Ok(OriginalExpr::Expr(expr)) = res { // TODO unwrap let name = expr.name(&DFSchema::empty()).unwrap(); let suffix_alias = format!("{}_{}", name, granularity); diff --git a/rust/cubesql/cubesql/src/compile/rewrite/rules/order.rs b/rust/cubesql/cubesql/src/compile/rewrite/rules/order.rs index 8db4f15e4b5d5..1401bf361b12c 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/rules/order.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/rules/order.rs @@ -1,10 +1,11 @@ use crate::{ compile::rewrite::{ - analysis::LogicalPlanAnalysis, column_name_to_member_vec, cube_scan, cube_scan_order, - cube_scan_order_empty_tail, expr_column_name, order, order_replacer, referenced_columns, - rewrite, rewriter::RewriteRules, sort, sort_exp, sort_exp_empty_tail, sort_expr, - transforming_rewrite, LogicalPlanLanguage, OrderAsc, OrderMember, - OrderReplacerColumnNameToMember, SortExprAsc, + analysis::{LogicalPlanAnalysis, OriginalExpr}, + column_name_to_member_vec, cube_scan, cube_scan_order, cube_scan_order_empty_tail, + expr_column_name, order, order_replacer, referenced_columns, rewrite, + rewriter::RewriteRules, + sort, sort_exp, sort_exp_empty_tail, sort_expr, transforming_rewrite, LogicalPlanLanguage, + OrderAsc, OrderMember, OrderReplacerColumnNameToMember, SortExprAsc, }, var, var_iter, }; @@ -127,38 +128,34 @@ impl OrderRules { let order_member_var = order_member_var.parse().unwrap(); let order_asc_var = order_asc_var.parse().unwrap(); move |egraph, subst| { - let expr = egraph[subst[expr_var]] - .data - .original_expr - .as_ref() - .expect(&format!( - "Original expr wasn't prepared for {:?}", - egraph[subst[expr_var]] - )); - let column_name = expr_column_name(expr.clone(), &None); - for asc in var_iter!(egraph[subst[asc_var]], SortExprAsc) { - let asc = *asc; - for column_name_to_member in var_iter!( - egraph[subst[column_name_to_member_var]], - OrderReplacerColumnNameToMember - ) { - if let Some((_, Some(member_name))) = column_name_to_member - .iter() - .find(|(c, _)| c == &column_name) - { - let member_name = member_name.to_string(); - subst.insert( - order_member_var, - egraph.add(LogicalPlanLanguage::OrderMember(OrderMember( - member_name.to_string(), - ))), - ); + if let Some(OriginalExpr::Expr(expr)) = + egraph[subst[expr_var]].data.original_expr.clone() + { + let column_name = expr_column_name(expr.clone(), &None); + for asc in var_iter!(egraph[subst[asc_var]], SortExprAsc) { + let asc = *asc; + for column_name_to_member in var_iter!( + egraph[subst[column_name_to_member_var]], + OrderReplacerColumnNameToMember + ) { + if let Some((_, Some(member_name))) = column_name_to_member + .iter() + .find(|(c, _)| c == &column_name) + { + let member_name = member_name.to_string(); + subst.insert( + order_member_var, + egraph.add(LogicalPlanLanguage::OrderMember(OrderMember( + member_name.to_string(), + ))), + ); - subst.insert( - order_asc_var, - egraph.add(LogicalPlanLanguage::OrderAsc(OrderAsc(asc))), - ); - return true; + subst.insert( + order_asc_var, + egraph.add(LogicalPlanLanguage::OrderAsc(OrderAsc(asc))), + ); + return true; + } } } } diff --git a/rust/cubesql/cubesql/src/compile/rewrite/rules/split.rs b/rust/cubesql/cubesql/src/compile/rewrite/rules/split.rs index 594119e624e9f..b621d745f5a8a 100644 --- a/rust/cubesql/cubesql/src/compile/rewrite/rules/split.rs +++ b/rust/cubesql/cubesql/src/compile/rewrite/rules/split.rs @@ -3,7 +3,7 @@ use crate::{ compile::rewrite::{ agg_fun_expr, aggr_aggr_expr, aggr_aggr_expr_empty_tail, aggr_group_expr, aggr_group_expr_empty_tail, aggregate, alias_expr, - analysis::{ConstantFolding, LogicalPlanAnalysis}, + analysis::{ConstantFolding, LogicalPlanAnalysis, OriginalExpr}, binary_expr, cast_expr, cast_expr_explicit, column_expr, cube_scan, event_notification, fun_expr, group_aggregate_split_replacer, group_expr_split_replacer, inner_aggregate_split_replacer, is_not_null_expr, is_null_expr, literal_expr, @@ -4867,7 +4867,7 @@ impl SplitRules { original_expr_id ))); - if let Ok(expr) = res { + if let Ok(OriginalExpr::Expr(expr)) = res { // TODO unwrap let name = expr.name(&DFSchema::empty()).unwrap(); let column = Column::from_name(name.to_string()); @@ -4925,7 +4925,7 @@ impl SplitRules { "Original expr wasn't prepared for {:?}", original_expr_id ))); - if let Ok(expr) = res { + if let Ok(OriginalExpr::Expr(expr)) = res { for alias_to_cube in alias_to_cube_fn(egraph, subst[alias_to_cube_var]) { for column in var_iter!(egraph[subst[column_var]], ColumnExprColumn).cloned() { if let Some((alias, _)) = @@ -6215,7 +6215,7 @@ impl SplitRules { "Original expr wasn't prepared for {:?}", original_expr_id ))); - if let Ok(expr) = res { + if let Ok(OriginalExpr::Expr(expr)) = res { let name = expr.name(&DFSchema::empty()).unwrap(); subst.insert( alias_val, @@ -6355,7 +6355,7 @@ impl SplitRules { expr_id ))); - if let Ok(expr) = res { + if let Ok(OriginalExpr::Expr(expr)) = res { let inner_expr_id = subst[inner_expr_var]; let res = egraph[inner_expr_id] @@ -6367,7 +6367,7 @@ impl SplitRules { inner_expr_id ))); - if let Ok(inner_expr) = res { + if let Ok(OriginalExpr::Expr(inner_expr)) = res { match inner_expr { Expr::Column(_) => { for data_type in @@ -6462,7 +6462,7 @@ impl SplitRules { expr_id ))); - if let Ok(expr) = res { + if let Ok(OriginalExpr::Expr(expr)) = res { let inner_expr_id = subst[inner_expr_var]; let res = egraph[inner_expr_id] @@ -6474,7 +6474,7 @@ impl SplitRules { inner_expr_id ))); - if let Ok(inner_expr) = res { + if let Ok(OriginalExpr::Expr(inner_expr)) = res { match inner_expr { Expr::Column(_) => { for data_type in