fix(tesseract): Issue with multistage rolling window without time dimension

[waralexrom]

Check List

• Tests have been run in packages where changes have been made if available
• Linter has been run for changed code
• Tests for the changes have been added if not covered yet
• Docs have been added / updated if required

[claude]

Claude finished @waralexrom's task in 1m 48s —— View job

---

Code Review: PR #10691

• Gather context and read changed files
• Review the core fix in multi_stage_query_planner.rs
• Review test fixtures and snapshots
• Review test cases
• Post comprehensive review

---

Summary

This PR fixes a bug where multistage rolling window measures (e.g., to_date type) would fail or produce incorrect results when no time dimension is present in the query. The fix correctly mirrors the multi-stage vs. non-multi-stage branching logic that already exists in the time-dimension-present path (lines 517–531) into the time_dimensions.is_empty() branch (lines 481–490).

---

Code Quality

The core fix (multi_stage_query_planner.rs) — two changes:

1. Hoisted base_member creation (line 476): let base_member = MemberSymbol::new_measure(measure.new_unrolling()) is moved above the time_dimensions.is_empty() check so it's available in both branches. This eliminates duplication — previously it was only created in the time-dimension-present path (line 507 before). Good refactoring.

2. Multi-stage branching in the no-time-dimension path (lines 481–490): The old code unconditionally called add_rolling_window_base(member.clone(), base_state, ungrouped, descriptions) which has two issues for multi-stage measures:

   • It passed member.clone() (the original rolling window measure) instead of base_member (the unrolled version). Multi-stage measures need their references resolved via make_queries_descriptions.
   • It passed ungrouped instead of false. For non-multi-stage measures in the no-time-dimension path, ungrouped would be true for non-additive measures (e.g., avg), but now false is hardcoded for both branches.

   The new code correctly routes multi-stage measures through make_queries_descriptions, which recursively resolves multi-stage member references — matching the pattern at lines 517–531.

One observation on the ungrouped parameter change:

In the non-multi-stage branch, the old code passed ungrouped (which is measure.is_rolling_window() && !measure.is_addictive()) while the new code passes false. This changes behavior for non-additive, non-multi-stage rolling window measures without time dimensions (e.g., the new rolling_avg_to_date test). The test_to_date_avg_no_time_dimension snapshot test confirms this produces a correct result (137.75), so the change appears intentional and correct — without a time dimension, there's no window to apply ungrouped semantics over.

---

Security

No security concerns — this is internal query planning logic with no user-facing input handling changes.

---

Performance

No performance concerns. The make_queries_descriptions path for multi-stage measures is necessary for correctness and mirrors the existing pattern. The base_member hoisting avoids any redundant computation.

---

Testing

New test fixtures (YAML):

• rolling_avg_to_date: avg-type to_date rolling window — tests the non-additive, non-multi-stage path
• rolling_sum_to_date_multistage: sum-type to_date rolling window with multi_stage: true referencing {total_amount} — tests the multi-stage path

New test cases:

• test_to_date_no_granularity_multistage — multistage rolling window without time dimension
• test_to_date_avg_no_time_dimension — avg (non-additive) rolling window without time dimension

Both test cases use the same match-on-result-or-snapshot-error pattern. Snapshots look correct:

• to_date_no_granularity_multistage: 2755.00 (sum of total_amount within month-to-date range)
• to_date_avg_no_time_dimension: 137.7500000000000000 (avg of amount within month-to-date range)

Suggestion: Consider adding a test for a multistage rolling window measure with a time dimension to confirm the existing path wasn't regressed. The current tests only exercise the time_dimensions.is_empty() branch.

---

Minor Nits

1. Missing blank line at to_date_variations.rs:179–180: There's no blank line between the closing } of test_to_date_no_granularity and the #[tokio::test] attribute of test_to_date_no_granularity_multistage. The other test functions in this file consistently have a blank line separator.

2. Existing typo (not introduced by this PR): is_addictive at line 461 — this is likely meant to be is_additive. Not a blocker, just noting it.

---

Verdict

This is a clean, well-scoped bug fix. The approach correctly mirrors the already-established multi-stage branching pattern into a previously unhandled code path. The test coverage is appropriate for the fix.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.82%. Comparing base (0db9e70) to head (2a9d6d0).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10691      +/-   ##
==========================================
- Coverage   83.41%   78.82%   -4.60%     
==========================================
  Files         250      465     +215     
  Lines       75261    91930   +16669     
  Branches        0     3354    +3354     
==========================================
+ Hits        62780    72461    +9681     
- Misses      12481    18979    +6498     
- Partials        0      490     +490     

Flag	Coverage Δ
cube-backend	58.07% <ø> (?)
cubesql	83.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.