From b921f35eb9172615c5aaaef4f75442aedb684051 Mon Sep 17 00:00:00 2001 From: Igor Lukanin Date: Fri, 31 Jul 2026 02:24:42 +0200 Subject: [PATCH] fix(tesseract): honour custom granularity origin for single-unit intervals --- .../src/planner/time_dimension/date_time.rs | 59 +++++++++++ .../src/planner/time_dimension/granularity.rs | 98 +++++++++++++++++-- .../integration_custom_granularity.yaml | 16 +++ .../tests/integration/custom_granularities.rs | 94 ++++++++++++++++++ 4 files changed, 260 insertions(+), 7 deletions(-) diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time.rs index 190e12d88c340..55c5be748bff9 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/date_time.rs @@ -54,6 +54,42 @@ impl QueryDateTime { ) } + /// Whether this instant sits exactly on the start of the given predefined granularity — + /// i.e. whether `DATE_TRUNC(granularity, self)` would leave it unchanged. + /// + /// Judged on the local wall clock, so a midnight that a DST gap pushed forward to 01:00 still + /// counts as the start of its day. + pub fn is_start_of(&self, granularity: &str) -> Result { + let dt = self.naive_local(); + // Midnight can be absent from the local calendar when a DST gap swallows it; the day then + // starts at whatever instant the gap resolves to, and that is still its start. + let midnight = dt.date().and_hms_opt(0, 0, 0).ok_or_else(|| { + CubeError::internal(format!("Failed to build midnight for {}", dt.date())) + })?; + let start_of_day = + Self::from_local_date_time(self.date_time.timezone(), midnight)?.naive_local(); + let starts_the_day = dt.time() == start_of_day.time() && dt.date() == start_of_day.date(); + let zero_time = starts_the_day && dt.nanosecond() == 0; + + let res = match granularity { + "year" => zero_time && dt.month() == 1 && dt.day() == 1, + "quarter" => zero_time && dt.day() == 1 && (dt.month() - 1).is_multiple_of(3), + "month" => zero_time && dt.day() == 1, + "week" => zero_time && dt.weekday() == Weekday::Mon, + "day" => zero_time, + "hour" => dt.minute() == 0 && dt.second() == 0 && dt.nanosecond() == 0, + "minute" => dt.second() == 0 && dt.nanosecond() == 0, + "second" => dt.nanosecond() == 0, + other => { + return Err(CubeError::internal(format!( + "Unexpected granularity '{other}' in date alignment check" + ))) + } + }; + + Ok(res) + } + pub fn date_time(&self) -> DateTime { self.date_time } @@ -205,6 +241,29 @@ impl QueryDateTime { mod tests { use super::*; + #[test] + fn is_start_of_accepts_a_midnight_swallowed_by_a_dst_gap() { + // Paraguay springs forward at midnight, so 2023-10-01T00:00 does not exist locally and + // resolves to 01:00. It is still the first instant of that day, month and quarter. + let tz = "America/Asuncion".parse::().unwrap(); + let d = QueryDateTime::from_date_str(tz, "2023-10-01").unwrap(); + + assert_eq!(d.default_format(), "2023-10-01T01:00:00.000"); + assert!(d.is_start_of("day").unwrap()); + assert!(d.is_start_of("month").unwrap()); + assert!(d.is_start_of("quarter").unwrap()); + assert!(!d.is_start_of("year").unwrap()); + } + + #[test] + fn is_start_of_still_rejects_a_real_time_component() { + let tz = "America/Asuncion".parse::().unwrap(); + // 2023-10-02 has an ordinary midnight, so 01:00 on it is genuinely mid-day. + let d = QueryDateTime::from_date_str(tz, "2023-10-02T01:00:00").unwrap(); + assert!(!d.is_start_of("day").unwrap()); + assert!(d.is_start_of("hour").unwrap()); + } + #[test] fn test_parse_date_time() { let tz = "Etc/GMT-3".parse::().unwrap(); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity.rs index 9e12aa2ff4c68..c578cbd6f2b7f 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity.rs @@ -67,8 +67,11 @@ impl Granularity { }); } - let origin = if let Some(origin) = origin { - QueryDateTime::from_date_str(timezone, &origin)? + // `origin` takes precedence over `offset`, and drops it: keeping the offset around would + // let the rendering shift buckets that `origin` — which every alignment check reads — + // knows nothing about. + let (origin, granularity_offset) = if let Some(origin) = origin { + (QueryDateTime::from_date_str(timezone, &origin)?, None) } else if let Some(offset) = &granularity_offset { // Week-based intervals expect the offset relative to the start of a week. let origin = Self::fix_origin_for_weeks_if_needed( @@ -76,15 +79,26 @@ impl Granularity { &granularity_interval, ); let interval = SqlInterval::from_str(offset)?; - origin.add_interval(&interval)? + (origin.add_interval(&interval)?, granularity_offset) } else { - Self::fix_origin_for_weeks_if_needed( - Self::default_origin(timezone)?, - &granularity_interval, + ( + Self::fix_origin_for_weeks_if_needed( + Self::default_origin(timezone)?, + &granularity_interval, + ), + granularity_offset, ) }; - let is_natural_aligned = granularity_interval.is_trivial(); + // A trivial interval can only ride the DATE_TRUNC path when its origin actually sits on + // that unit's natural boundary — `1 year` from 2024-04-01 is a fiscal year, and truncating + // to the calendar year would silently discard the origin. An `offset` origin is exempt: + // it is off-boundary by construction, and the aligned branch renders it by subtracting the + // offset before truncating and adding it back. + let is_natural_aligned = granularity_interval.is_trivial() + && (granularity_offset.is_some() + || origin.is_start_of(&granularity_interval.min_granularity()?)?); + Ok(Self { granularity, granularity_interval, @@ -293,4 +307,74 @@ mod tests { NaiveDate::from_ymd_opt(2024, 1, 3).unwrap() ); } + + #[test] + fn trivial_interval_with_off_boundary_origin_is_not_natural_aligned() { + // A fiscal year starting on April 1 cannot be rendered as DATE_TRUNC('year', ...). + assert!(!custom("1 year", Some("2024-04-01"), None).is_natural_aligned()); + } + + #[test] + fn trivial_interval_with_on_boundary_origin_stays_natural_aligned() { + // January 1 is the natural year boundary, so DATE_TRUNC is still correct. + assert!(custom("1 year", Some("2024-01-01"), None).is_natural_aligned()); + } + + #[test] + fn off_boundary_origin_is_judged_against_the_intervals_own_unit() { + // April 1 is off the year boundary but exactly on the month and quarter boundaries. + assert!(custom("1 month", Some("2024-04-01"), None).is_natural_aligned()); + assert!(custom("1 quarter", Some("2024-04-01"), None).is_natural_aligned()); + // ...and February 1 is on the month boundary but not the quarter one. + assert!(custom("1 month", Some("2024-02-01"), None).is_natural_aligned()); + assert!(!custom("1 quarter", Some("2024-02-01"), None).is_natural_aligned()); + } + + #[test] + fn origin_with_a_time_component_defeats_alignment_of_date_intervals() { + assert!(!custom("1 day", Some("2024-04-01T06:00:00"), None).is_natural_aligned()); + assert!(custom("1 hour", Some("2024-04-01T06:00:00"), None).is_natural_aligned()); + } + + #[test] + fn week_origin_is_judged_against_monday() { + // 2024-01-01 is a Monday; 2024-01-03 is a Wednesday. + assert!(custom("1 week", Some("2024-01-01"), None).is_natural_aligned()); + assert!(!custom("1 week", Some("2024-01-03"), None).is_natural_aligned()); + } + + #[test] + fn default_and_offset_origins_keep_their_existing_alignment() { + // No explicit origin: the default origin is the start of the year, and the week-only + // interval snaps to Monday — both natural boundaries, so DATE_TRUNC still applies. + assert!(custom("1 year", None, None).is_natural_aligned()); + assert!(custom("1 week", None, None).is_natural_aligned()); + // `offset` renders through the subtract/truncate/add branch, which stays aligned. + assert!(custom("1 week", None, Some("-1 day")).is_natural_aligned()); + } + + #[test] + fn explicit_origin_discards_the_offset() { + // `origin` takes precedence when both are set. The offset must be dropped outright — + // left in place it would shift the rendered buckets away from the origin that + // `align_date_to_origin` and the materialized time series both bin on. + let g = custom("1 year", Some("2024-04-01"), Some("3 months")); + assert_eq!(g.granularity_offset(), &None); + assert!(!g.is_natural_aligned()); + + // Same for an on-boundary origin, which would otherwise stay on the offset sub-branch. + let g = custom("1 year", Some("2024-01-01"), Some("3 months")); + assert_eq!(g.granularity_offset(), &None); + assert_eq!( + origin_date(&g), + NaiveDate::from_ymd_opt(2024, 1, 1).unwrap() + ); + assert!(g.is_natural_aligned()); + } + + #[test] + fn non_trivial_intervals_are_never_natural_aligned() { + assert!(!custom("15 minutes", None, None).is_natural_aligned()); + assert!(!custom("6 months", Some("2024-01-01"), None).is_natural_aligned()); + } } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_custom_granularity.yaml b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_custom_granularity.yaml index 0843e75236ee6..a25cd3b4774cc 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_custom_granularity.yaml +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_custom_granularity.yaml @@ -25,6 +25,14 @@ cubes: - name: fiscal_year interval: "1 year" offset: "1 month" + # Trivial interval with an off-boundary origin: must render through DATE_BIN, + # not DATE_TRUNC, or the origin is silently discarded. + - name: fiscal_year_by_1st_april + interval: "1 year" + origin: "2024-04-01" + - name: calendar_year_by_origin + interval: "1 year" + origin: "2024-01-01" - name: fiscal_year_alias type: time @@ -39,3 +47,11 @@ cubes: - name: total_amount type: sum sql: amount + - name: total_amount_prior_year + type: number + sql: "{CUBE.total_amount}" + multi_stage: true + time_shift: + - time_dimension: created_at + interval: "1 year" + type: prior diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/custom_granularities.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/custom_granularities.rs index 8694f856f8a9b..f439f209396cd 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/custom_granularities.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/custom_granularities.rs @@ -167,6 +167,100 @@ async fn test_type_time_alias_wraps_compound_exprs_before_tz_cast() { } } +#[tokio::test(flavor = "multi_thread")] +async fn test_fiscal_year_by_origin_does_not_truncate_to_calendar_year() { + let ctx = create_context(); + + let query = indoc! {" + measures: + - orders.count + time_dimensions: + - dimension: orders.created_at + granularity: fiscal_year_by_1st_april + dateRange: + - \"2024-04-01\" + - \"2026-03-31\" + order: + - id: orders.created_at + "}; + + let sql = ctx.build_sql(query).unwrap(); + + // `1 year` is a trivial interval, so the grain used to fall onto the DATE_TRUNC branch, + // which has no way to express an April 1 origin. + assert!( + !sql.to_lowercase().contains("date_trunc('year'"), + "off-boundary origin was truncated to the calendar year\nFull SQL:\n{sql}" + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_calendar_year_by_origin_still_truncates() { + let ctx = create_context(); + + let query = indoc! {" + measures: + - orders.count + time_dimensions: + - dimension: orders.created_at + granularity: calendar_year_by_origin + dateRange: + - \"2024-01-01\" + - \"2025-12-31\" + order: + - id: orders.created_at + "}; + + let sql = ctx.build_sql(query).unwrap(); + + // An origin that already sits on the year boundary keeps the cheaper DATE_TRUNC rendering. + assert!( + sql.to_lowercase().contains("date_trunc('year'"), + "on-boundary origin should still truncate\nFull SQL:\n{sql}" + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_fiscal_year_by_origin_with_time_shift_measure() { + let ctx = create_context(); + + // The shape from the original report: a time-shift measure alongside the plain one, both + // grouped by a fiscal-year grain. The shifted leaf must bin on the same origin as the + // unshifted one, or the two sides join on incompatible buckets. + let query = indoc! {" + measures: + - orders.total_amount + - orders.total_amount_prior_year + time_dimensions: + - dimension: orders.created_at + granularity: fiscal_year_by_1st_april + dateRange: + - \"2024-04-01\" + - \"2026-03-31\" + order: + - id: orders.created_at + "}; + + let sql = ctx.build_sql(query).unwrap(); + + assert!( + !sql.to_lowercase().contains("date_trunc('year'"), + "time-shifted leaf truncated to the calendar year\nFull SQL:\n{sql}" + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + #[tokio::test(flavor = "multi_thread")] async fn test_custom_granularity_with_daterange_filter() { let ctx = create_context();