Skip to content

fix(schema-compiler): Preserve full time buckets in period-over-perio… #9502

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nchristo1213
Copy link

fix(schema-compiler): Preserve Full Time Buckets in Period-over-Period Queries

This PR replaces INNER JOIN with LEFT JOIN when stitching sub-queries in outerMeasuresJoinFullKeyQueryAggregate, ensuring all expected time buckets appear when calculating period-over-period (PoP) metrics.

Problem

When PoP queries used INNER JOIN, buckets missing from auxiliary sub-queries (q_1, q_2, ...) were dropped causing incomplete results or incorrect ratios.

Solution

Use LEFT JOIN instead, preserving all keys from the anchor query (q_0). Missing values in auxiliary queries yield NULL rather than removing the row entirely.

LEFT JOIN ${this.wrapInParenthesis((q))} as q_${i + 1} ON ${this.dimensionsJoinCondition(`q_${i}`, `q_${i + 1}`)}

Benefits

  • Retains continuity for PoP metrics
  • Allows ratio calculations to appear accurate when prior period data is missing
  • Ensures accuracy without introducing gaps or dropped time buckets

Tests

  • New fixture: sales cube with date, category, region, and PoP measures
  • Adjusted:
    • CAGR: Added NULL rows
    • multi-stage graph tests: Added secondary ordering
  • New tests:
    • Time-only PoP
    • One-dimension PoP
    • Two-dimension PoP

These tests ensure the correct number of periods appear and that PoP measures return NULL instead of dropping rows when previous-period values are unavailable.

Check List

  • Tests have been run in packages where changes 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

resolves #9353

@nchristo1213 nchristo1213 requested a review from a team as a code owner April 22, 2025 21:42
@github-actions github-actions bot added the pr:community Contribution from Cube.js community members. label Apr 22, 2025
Copy link

codecov bot commented Apr 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.07%. Comparing base (32f9b79) to head (ab02440).
Report is 7 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (32f9b79) and HEAD (ab02440). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (32f9b79) HEAD (ab02440)
cubesql 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #9502       +/-   ##
===========================================
- Coverage   80.54%   59.07%   -21.48%     
===========================================
  Files         382      153      -229     
  Lines       96586    13017    -83569     
  Branches     2202     2202               
===========================================
- Hits        77797     7690    -70107     
+ Misses      18476     5014    -13462     
  Partials      313      313               
Flag Coverage Δ
cube-backend 59.07% <ø> (ø)
cubesql ?

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.

…d queries

Switches from INNER JOIN to LEFT JOIN in outerMeasuresJoinFullKeyQueryAggregate to prevent dropped rows when previous-period data is missing.

Adds fixture and tests for period-over-period measures.
@nchristo1213 nchristo1213 force-pushed the pop-missing-time-buckets branch from ab02440 to ea13aaa Compare April 24, 2025 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:community Contribution from Cube.js community members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rolling Window Calculations with Multiple Dimensions
1 participant