diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 20381e39bbbf6..7adad3e85ec0d 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -951,6 +951,7 @@ export class BaseQuery { joinHints: this.options.joinHints, cubestoreSupportMultistage: this.options.cubestoreSupportMultistage ?? getEnv('cubeStoreRollingWindowJoin'), disableExternalPreAggregations: !!this.options.disableExternalPreAggregations, + convertTzForRawTimeDimension: !!this.options.convertTzForRawTimeDimension, maskedMembers: this.options.maskedMembers, memberToAlias: this.options.memberToAlias, }; diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts index dbda56eafe09b..3efee71060690 100644 --- a/packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts +++ b/packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts @@ -5067,6 +5067,47 @@ SELECT 1 AS revenue, cast('2024-01-01' AS timestamp) as time UNION ALL }] )); + it('raw time dimension with timezone', async () => runQueryTest( + { + measures: [ + 'visitors.visitor_revenue', + ], + dimensions: ['visitors.created_at'], + timeDimensions: [{ + dimension: 'visitors.created_at', + granularity: 'day', + dateRange: ['2017-01-01', '2017-01-30'] + }], + timezone: 'America/Los_Angeles', + convertTzForRawTimeDimension: true, + order: [{ + id: 'visitors.created_at' + }] + }, + [ + { + visitors__created_at: '2017-01-02T16:00:00.000Z', + visitors__created_at_day: '2017-01-02T00:00:00.000Z', + visitors__visitor_revenue: '100' + }, + { + visitors__created_at: '2017-01-04T16:00:00.000Z', + visitors__created_at_day: '2017-01-04T00:00:00.000Z', + visitors__visitor_revenue: '200' + }, + { + visitors__created_at: '2017-01-05T16:00:00.000Z', + visitors__created_at_day: '2017-01-05T00:00:00.000Z', + visitors__visitor_revenue: null + }, + { + visitors__created_at: '2017-01-06T16:00:00.000Z', + visitors__created_at_day: '2017-01-06T00:00:00.000Z', + visitors__visitor_revenue: null + } + ] + )); + it('simple join with segment', async () => runQueryTest( { measures: [ diff --git a/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_query_options.rs b/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_query_options.rs index 1d58958f32e20..92b6759a52d4a 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_query_options.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/cube_bridge/base_query_options.rs @@ -74,6 +74,8 @@ pub struct BaseQueryOptionsStatic { pub disable_external_pre_aggregations: bool, #[serde(rename = "preAggregationId")] pub pre_aggregation_id: Option, + #[serde(rename = "convertTzForRawTimeDimension")] + pub convert_tz_for_raw_time_dimension: Option, #[serde(rename = "maskedMembers")] pub masked_members: Option>, #[serde(rename = "memberToAlias", default)] diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/base_query.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/base_query.rs index 27651cc87a0ef..92ef1b9a824ba 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/base_query.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/base_query.rs @@ -34,6 +34,10 @@ impl BaseQuery { options.join_graph()?, options.static_data().timezone.clone(), options.static_data().export_annotated_sql, + options + .static_data() + .convert_tz_for_raw_time_dimension + .unwrap_or(false), options.static_data().masked_members.clone(), options.static_data().member_to_alias.clone(), )?; diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs index 5642a274e830e..eb75f26bdefac 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/filter/typed_filter.rs @@ -2,7 +2,8 @@ use crate::cube_bridge::member_sql::FilterParamsColumn; use crate::planner::query_tools::QueryTools; use crate::planner::sql_evaluator::MemberSymbol; use crate::planner::sql_templates::PlanSqlTemplates; -use crate::planner::{evaluate_with_context, FiltersContext, VisitorContext}; +use crate::planner::visitor_context::evaluate_filter_with_context; +use crate::planner::{FiltersContext, VisitorContext}; use cubenativeutils::CubeError; use std::rc::Rc; @@ -109,7 +110,7 @@ impl TypedFilter { } let resolved = resolve_base_symbol(&self.member_evaluator); - let member_sql = evaluate_with_context(&resolved, context.clone(), plan_templates)?; + let member_sql = evaluate_filter_with_context(&resolved, context.clone(), plan_templates)?; let filters_context = context.filters_context(); let ctx = FilterSqlContext { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs index 74fc677a7129f..377a3b78a068e 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/query_tools.rs @@ -30,6 +30,7 @@ pub struct QueryTools { params_allocator: Rc>, evaluator_compiler: Rc>, timezone: Tz, + convert_tz_for_raw_time_dimension: bool, masked_members: HashSet, } @@ -41,6 +42,7 @@ impl QueryTools { join_graph: Rc, timezone_name: Option, export_annotated_sql: bool, + convert_tz_for_raw_time_dimension: bool, masked_members: Option>, member_to_alias: Option>, ) -> Result, CubeError> { @@ -67,6 +69,7 @@ impl QueryTools { params_allocator: Rc::new(RefCell::new(ParamsAllocator::new(export_annotated_sql))), evaluator_compiler, timezone, + convert_tz_for_raw_time_dimension, masked_members: masked_members.unwrap_or_default().into_iter().collect(), })) } @@ -96,6 +99,10 @@ impl QueryTools { self.timezone } + pub fn convert_tz_for_raw_time_dimension(&self) -> bool { + self.convert_tz_for_raw_time_dimension + } + pub fn join_for_hints( &self, hints: &JoinHints, diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/evaluate_sql.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/evaluate_sql.rs index 9a96dc43739c6..95066c71e9523 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/evaluate_sql.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/evaluate_sql.rs @@ -35,6 +35,7 @@ impl SqlNode for EvaluateSqlNode { Ok(res) } MemberSymbol::TimeDimension(ev) => { + let visitor = visitor.with_ignore_tz_convert(); let res = visitor.apply(&ev.base_symbol(), node_processor.clone(), templates)?; Ok(res) } diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/factory.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/factory.rs index 95833fc3211f8..5d15962841284 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/factory.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/factory.rs @@ -257,12 +257,13 @@ impl SqlNodesFactory { let input: Rc = CaseSqlNode::new(input); input }; - let input: Rc = - TimeDimensionNode::new(self.dimensions_with_ignored_timezone.clone(), input); let input: Rc = AutoPrefixSqlNode::new(input, self.cube_name_references.clone()); + let input: Rc = + TimeDimensionNode::new(self.dimensions_with_ignored_timezone.clone(), input); + let input = if !self.calendar_time_shifts.is_empty() { CalendarTimeShiftSqlNode::new(self.calendar_time_shifts.clone(), input) } else { diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/time_dimension.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/time_dimension.rs index b24c59e22ee1a..861fd3ba9ee1c 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/time_dimension.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_nodes/time_dimension.rs @@ -53,10 +53,11 @@ impl SqlNode for TimeDimensionNode { ); } - let converted_tz = if self + let skip_convert_tz = self .dimensions_with_ignored_timezone - .contains(&ev.full_name()) - { + .contains(&ev.full_name()); + + let converted_tz = if skip_convert_tz { input_sql } else { templates.convert_tz(input_sql)? @@ -68,6 +69,16 @@ impl SqlNode for TimeDimensionNode { }; Ok(res) } + MemberSymbol::Dimension(ev) => { + if !visitor.ignore_tz_convert() + && query_tools.convert_tz_for_raw_time_dimension() + && ev.dimension_type() == "time" + { + Ok(templates.convert_tz(input_sql)?) + } else { + Ok(input_sql) + } + } _ => Ok(input_sql), } } diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_visitor.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_visitor.rs index eb9def9ce21aa..25e14ab148337 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_visitor.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/sql_evaluator/sql_visitor.rs @@ -13,6 +13,7 @@ pub struct SqlEvaluatorVisitor { query_tools: Rc, cube_ref_evaluator: Rc, all_filters: Option, //To pass to FILTER_PARAMS and FILTER_GROUP + ignore_tz_convert: bool, } impl SqlEvaluatorVisitor { @@ -25,9 +26,16 @@ impl SqlEvaluatorVisitor { query_tools, cube_ref_evaluator, all_filters, + ignore_tz_convert: false, } } + pub fn with_ignore_tz_convert(&self) -> Self { + let mut self_copy = self.clone(); + self_copy.ignore_tz_convert = true; + self_copy + } + pub fn all_filters(&self) -> Option { self.all_filters.clone() } @@ -48,6 +56,10 @@ impl SqlEvaluatorVisitor { Ok(result) } + pub fn ignore_tz_convert(&self) -> bool { + self.ignore_tz_convert + } + pub fn evaluate_cube_ref( &self, cube_ref: &CubeRef, diff --git a/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs b/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs index ce61b26692ad8..793321ec6ac51 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs @@ -92,6 +92,19 @@ pub fn evaluate_with_context( visitor.apply(node, node_processor, templates) } +pub fn evaluate_filter_with_context( + node: &Rc, + context: Rc, + templates: &PlanSqlTemplates, +) -> Result { + let visitor = context + .make_visitor(context.query_tools()) + .with_ignore_tz_convert(); + let node_processor = context.node_processor(); + + visitor.apply(node, node_processor, templates) +} + pub fn evaluate_sql_call_with_context( sql_call: &Rc, context: Rc, diff --git a/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/base_query_options.rs b/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/base_query_options.rs index 0443af98aef5b..96a1ce29cf8da 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/base_query_options.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/base_query_options.rs @@ -69,6 +69,8 @@ pub struct MockBaseQueryOptions { #[builder(default)] pre_aggregation_id: Option, #[builder(default)] + convert_tz_for_raw_time_dimension: Option, + #[builder(default)] masked_members: Option>, #[builder(default)] member_to_alias: Option>, @@ -91,6 +93,7 @@ impl_static_data!( cubestore_support_multistage, disable_external_pre_aggregations, pre_aggregation_id, + convert_tz_for_raw_time_dimension, masked_members, member_to_alias ); diff --git a/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/base_query_options.rs b/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/base_query_options.rs index fd184e2954631..0de5d60d4efb1 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/base_query_options.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/yaml/base_query_options.rs @@ -35,6 +35,8 @@ pub struct YamlBaseQueryOptions { pub disable_external_pre_aggregations: Option, #[serde(default)] pub pre_aggregation_id: Option, + #[serde(default)] + pub convert_tz_for_raw_time_dimension: Option, #[serde(default, rename = "joinHints")] pub join_hints: Option>>, #[serde(default, rename = "memberToAlias")] diff --git a/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/test_utils/test_context.rs b/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/test_utils/test_context.rs index 7d641011dc515..a03b9223f72b7 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/test_utils/test_context.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/test_fixtures/test_utils/test_context.rs @@ -28,7 +28,7 @@ pub struct TestContext { impl TestContext { pub fn new(schema: MockSchema) -> Result { - Self::new_with_options(schema, Tz::UTC, None, None, false) + Self::new_with_options(schema, Tz::UTC, None, None, false, false) } #[allow(dead_code)] @@ -48,6 +48,7 @@ impl TestContext { join_graph, Some(Tz::UTC.to_string()), false, + false, None, None, )?; @@ -70,14 +71,14 @@ impl TestContext { #[allow(dead_code)] pub fn new_with_timezone(schema: MockSchema, timezone: Tz) -> Result { - Self::new_with_options(schema, timezone, None, None, false) + Self::new_with_options(schema, timezone, None, None, false, false) } pub fn new_with_masked_members( schema: MockSchema, masked_members: Vec, ) -> Result { - Self::new_with_options(schema, Tz::UTC, Some(masked_members), None, false) + Self::new_with_options(schema, Tz::UTC, Some(masked_members), None, false, false) } fn for_options(&self, options: &dyn BaseQueryOptions) -> Result { @@ -94,6 +95,9 @@ impl TestContext { static_data.masked_members.clone(), static_data.member_to_alias.clone(), static_data.export_annotated_sql, + static_data + .convert_tz_for_raw_time_dimension + .unwrap_or(false), ) } @@ -103,6 +107,7 @@ impl TestContext { masked_members: Option>, member_to_alias: Option>, export_annotated_sql: bool, + convert_tz_for_raw_time_dimension: bool, ) -> Result { let base_tools = schema.create_base_tools_with_timezone(timezone.to_string())?; let join_graph = Rc::new(schema.create_join_graph()?); @@ -117,6 +122,7 @@ impl TestContext { join_graph, Some(timezone.to_string()), export_annotated_sql, + convert_tz_for_raw_time_dimension, masked_members, member_to_alias, )?; @@ -343,6 +349,7 @@ impl TestContext { .unwrap_or(false), ) .pre_aggregation_id(yaml_options.pre_aggregation_id) + .convert_tz_for_raw_time_dimension(yaml_options.convert_tz_for_raw_time_dimension) .member_to_alias(yaml_options.member_to_alias) .masked_members(yaml_options.masked_members) .timezone(yaml_options.timezone) @@ -447,8 +454,9 @@ impl TestContext { let tables = Self::collect_pre_agg_source_tables(pre_agg.source()); let yaml = Self::build_pre_agg_query_yaml(pre_agg); - let pa_ctx = Self::new_with_options(self.schema.clone(), Tz::UTC, None, None, false) - .expect("Failed to create pre-agg context"); + let pa_ctx = + Self::new_with_options(self.schema.clone(), Tz::UTC, None, None, false, false) + .expect("Failed to create pre-agg context"); let (raw_sql, _) = pa_ctx .build_sql_with_used_pre_aggregations(&yaml) diff --git a/rust/cubesqlplanner/cubesqlplanner/src/tests/integration/snapshots/cubesqlplanner__tests__integration__time_dimensions__convert_tz_for_raw_time_dimensions.snap b/rust/cubesqlplanner/cubesqlplanner/src/tests/integration/snapshots/cubesqlplanner__tests__integration__time_dimensions__convert_tz_for_raw_time_dimensions.snap new file mode 100644 index 0000000000000..b1b372ccda9df --- /dev/null +++ b/rust/cubesqlplanner/cubesqlplanner/src/tests/integration/snapshots/cubesqlplanner__tests__integration__time_dimensions__convert_tz_for_raw_time_dimensions.snap @@ -0,0 +1,15 @@ +--- +source: cubesqlplanner/src/tests/integration/time_dimensions.rs +expression: result +--- +orders__created_at | orders__created_at_month | orders__count +--------------------+--------------------------+-------------- +2024-01-15 02:00:00 | 2024-01-01 00:00:00 | 1 +2024-01-15 07:00:00 | 2024-01-01 00:00:00 | 1 +2024-01-20 06:00:00 | 2024-01-01 00:00:00 | 1 +2024-02-10 01:00:00 | 2024-02-01 00:00:00 | 1 +2024-02-15 03:00:00 | 2024-02-01 00:00:00 | 1 +2024-03-01 08:00:00 | 2024-03-01 00:00:00 | 1 +2024-03-10 00:00:00 | 2024-03-01 00:00:00 | 1 +2024-03-15 05:00:00 | 2024-03-01 00:00:00 | 1 +2024-04-01 03:00:00 | 2024-04-01 00:00:00 | 1 diff --git a/rust/cubesqlplanner/cubesqlplanner/src/tests/integration/time_dimensions.rs b/rust/cubesqlplanner/cubesqlplanner/src/tests/integration/time_dimensions.rs index 932c438a9c3ad..f6a51d3a1e2a4 100644 --- a/rust/cubesqlplanner/cubesqlplanner/src/tests/integration/time_dimensions.rs +++ b/rust/cubesqlplanner/cubesqlplanner/src/tests/integration/time_dimensions.rs @@ -360,3 +360,31 @@ async fn test_multiple_time_dimensions() { insta::assert_snapshot!(result); } } + +#[tokio::test(flavor = "multi_thread")] +async fn test_convert_tz_for_raw_time_dimensions() { + let ctx = create_context(); + + let query = indoc! {" + measures: + - orders.count + dimensions: + - orders.created_at + time_dimensions: + - dimension: orders.created_at + granularity: month + order: + - id: orders.created_at + timezone: \"America/Los_Angeles\" + convert_tz_for_raw_time_dimension: true + "}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx + .try_execute_pg(query, "integration_basic_tables.sql") + .await + { + insta::assert_snapshot!(result); + } +}