[waralexrom]

What was broken

PrimaryCube joins SecondaryCube (many_to_one, different data sources) and a rollupJoin combines PrimaryCube.primary_rollup with SecondaryCube.main. A query with PrimaryCube.total_count and SecondaryCube.total_contract_value by SecondaryCube.location_id matched the rollupJoin, but the SQL read secondary_cube_main__usage_0, which was never loaded (relation "secondary_cube_main__usage_0" does not exist).

Tesseract plans this query correctly: the many_to_one side's measure is computed from SecondaryCube.main on its own (usage 0), and PrimaryCube.total_count is read through the rollupJoin (usage 1), whose legs include SecondaryCube.main again. Each usage produced a description of secondary_cube_main, and preAggregationsDescription() then deduplicated descriptions by table name, so only the __usage_1 mapping survived.

Fix

preAggregationDescriptionsForUsageInfos now merges descriptions of the same table: the usage mappings are combined, so the orchestrator substitutes every __usage_N name the SQL reads. When the descriptions carry different matched date ranges, the merged one spans both; if either reads all partitions, so does the merged one. Each usage keeps its own dateRange in the mapping.

Interaction with #12577

#12577 suffixes the legs of a rollupJoin scanned in a query with several usages; this PR builds on it. With both, the legs read __usage_1 and secondary_cube_main__usage_0 is loaded as well, so the query executes end to end. #12577 alone does not fix it: the new test fails without this change.

Fan-out

Summing a one-side measure through the join does not happen: by the joined cube's own dimensions Tesseract reads its leg rollup alone, and by a dimension of the many side the measure goes through a keys subquery that no rollup serves, so it does not match the rollupJoin at all.

Tests

• pre-aggregations.test.ts: every table the SQL reads is declared by preAggregationsDescription(), for a base-cube measure and for measures of both cubes (fails before the fix).
• Ran the pre-aggregations unit suite, other unit suites asserting on descriptions, and the Postgres pre-agg* integration suites under Tesseract.

The legacy planner does not match the rollupJoin for this model at all (a separate issue, covered by #12519).

Fixes #11124

🤖 Generated with Claude Code

[claude]

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

---

Looks correct, no blocking issues: 0 high, 0 medium, 2 low.

Review details

• Read diff
• Read surrounding code (PreAggregations.ts, description dedup, orchestrator usageMapping consumer)
• Review tests
• Review threads
• Post review

Correctness (Tesseract only; the legacy planner never fills preAggregationUsageInfos)

The root cause is real. Before the fix, preAggregationsDescription() at PreAggregations.ts:183 ran R.uniqBy(tableName) and dropped the second description of secondary_cube_main, along with its __usage_N entry. I checked the merge in mergeDescriptionsByTable against the code that consumes it:

• No key collisions in usageMapping. Its keys are __usage_{index} suffixes, and the planner assigns those indices once per query (physical_plan_builder/processors/pre_aggregation.rs:33). Spreading the two mappings together cannot overwrite an entry.
• Each usage still loads only its own partitions. PreAggregationPartitionRangeLoader.ts:337 picks partitions per suffix from usageInfo.dateRange. The merged matchedTimeDimensionDateRange only sets the outer span, and it widens to all partitions (undefined) whenever either side is unbounded. Both behaviours are what we want.
• The rest of the description is safe to take from the first one. Both descriptions describe the same physical rollup under the same query, so the other fields (loadSql, partition settings, etc.) are the same.

Findings (inline)

• Low: PreAggregations.ts:271: the range union repeats what mergeUsageDateRanges() already does.
• Low: pre-aggregations.test.ts:2340: no test covers the date-range branch of the merge. Neither rollup is partitioned, so matchedTimeDimensionDateRange is always undefined there.

Other areas

• Security / performance: nothing to flag. The merge is a single O(n) pass over the descriptions.
• Docs: none needed. This is a bug fix with no new options or behaviour for users to configure.
• Comments: the 2-line comment at PreAggregations.ts:256 explains why the merge exists, so it is worth keeping.

Not run

I did not run any tests. Installing dependencies in the package needed an approval that this run doesn't have. The review is from reading the code. The PR description says the pre-aggregations unit suite and the Postgres pre-agg* integration suites passed under Tesseract.

Review threads

The listing returned no prior threads of mine (0). I resolved none and re-affirmed none, and posted 2 new inline comments.

[codecov]

Codecov Report

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

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

HEAD has 1 upload less than BASE

Flag	BASE (ef48cd2)	HEAD (b072601)
cubesql	1	0

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #12593       +/-   ##
===========================================
- Coverage   83.16%   66.18%   -16.98%     
===========================================
  Files         531      258      -273     
  Lines      124739    21997   -102742     
  Branches     5293     5295        +2     
===========================================
- Hits       103734    14559    -89175     
+ Misses      20209     6642    -13567     
  Partials      796      796               

Flag	Coverage Δ
cube-backend	66.18% <100.00%> (+0.01%)	⬆️
cubesql	?

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

☔ View full report in Codecov by Harness.
📢 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.

[claude]

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

---

LGTM, no blocking issues. 0 high, 0 medium, 1 low (process only, no inline comments).

Review details

• Gather context (diff, issue rollup_join not matched when query includes a measure from the secondary (right-side) cube, despite that measure being explicitly listed in the rollup_join definition #11124, changed files)
• Review PreAggregations.ts merge logic
• Review tests
• Review threads
• Post findings

Findings

Low: Fixes #11124 closes the issue before the reported query works. The PR body says #12577 is also needed for the query to run end to end, and #12577 is still open. Merging this PR first would auto-close #11124 while the query still fails, because the rollupJoin legs are read under their bare names. Change the line to Refs #11124, or merge #12577 first.

The issue's model was reported against the legacy planner's rollupJoin matching. That part is tracked in #12519, so the issue is only fully resolved with Tesseract enabled.

Verified

• mergeDescriptionsByTable (packages/cubejs-schema-compiler/src/adapter/PreAggregations.ts:258):
  • The merged entry stays where the first description for that table was. The later R.uniqBy(tableName) in preAggregationsDescription() (:183) now finds nothing to drop for local descriptions.
  • When both descriptions have a date range, the merged range spans both. When either has no range, the merged range is undefined, which matches the per-usage logic at :239-243. Both descriptions are for the same table, so they share its partitioning, and a missing range can only mean "reads every partition".
  • Each usage keeps its own dateRange in usageMapping. PreAggregationPartitionRangeLoader (packages/cubejs-query-orchestrator/src/orchestrator/PreAggregationPartitionRangeLoader.ts:337) uses that per-usage range to pick each usage's partitions, and the wider matchedTimeDimensionDateRange only widens the set of partitions that get loaded. So the result is correct.
  • Spreading usageMapping can't overwrite a suffix: each __usage_N comes from a different usage, and one rollupJoin can't include the same leg twice.
• The behaviour matches the API gateway's cross-query mergePreAggregationDescriptions (packages/cubejs-api-gateway/src/gateway.ts:244), which is more conservative: it drops ranges whenever the descriptions differ. The two compose without problems.
• Tests: the integration-style unit test checks every table the SQL reads against the loaded tableName + suffix set. That is the right thing to assert for the relation does not exist symptom. The three direct tests on mergeDescriptionsByTable cover the union, the "no range" case and keeping both usage maps. The test widens a protected static member to public in a subclass, as the root CLAUDE.md asks.
• Comments: the 2-line comment at :256 explains why the merge exists and is worth keeping.
• Docs: none needed. This is a bug fix with no new options or behaviour for users to configure.
• I did not run the tests locally because no node_modules were installed in this checkout.

Review threads

The listing returned 2 of my own unresolved threads. I resolved both: date ranges are now merged through mergeUsageDateRanges, and mergeDescriptionsByTable now has direct unit tests. I re-affirmed none.

[claude]

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

---

No issues found. 0 high, 0 medium, 0 low. Last round's only finding, Fixes #11124 landing before #12577, no longer applies because #12577 merged at 2026-11-04T14:09Z.

Review details

• Read diff and changed files
• Read issue rollup_join not matched when query includes a measure from the secondary (right-side) cube, despite that measure being explicitly listed in the rollup_join definition #11124
• Analyse merge logic and tests
• Review threads
• Post review

What I checked

These checks cover Tesseract only. The legacy planner doesn't match this rollupJoin at all, which is tracked in #12519.

• mergeDescriptionsByTable (packages/cubejs-schema-compiler/src/adapter/PreAggregations.ts:258)
  • Date range: when both descriptions have a range, the merged one spans both. When either lacks one, the merged range is undefined, the same rule as the per-usage logic at :239-243. A missing range can come from an unbounded usage or from a leg with no matched time dimension; either way, loading every partition is the safe choice.
  • Per-usage ranges: each usage keeps its own dateRange in usageMapping. The orchestrator picks each suffix's partitions from that range (packages/cubejs-query-orchestrator/src/orchestrator/PreAggregationPartitionRangeLoader.ts:337), so a wider matchedTimeDimensionDateRange only means more partitions get loaded. Results don't change.
  • Suffix collisions: none possible. Each __usage_N key belongs to exactly one usage info.
  • Later dedup: with local descriptions now unique per table, the R.uniqBy(tableName) at :183 has nothing to drop for them.
• Lambda bound: lambdaSourceDateRange (:447) only looks at the usage infos of the rollupLambda itself, so this merge doesn't affect it.
• Gateway merge: this composes with the API gateway's cross-query merge mergePreAggregationDescriptions (packages/cubejs-api-gateway/src/gateway.ts:244), which is stricter and drops ranges outright.
• Tests:
  • The end-to-end assertion checks every table the SQL reads against the set the orchestrator is told to load (tableName plus each usage suffix). That is the right check for the reported relation "secondary_cube_main__usage_0" does not exist.
  • Three direct tests on the merge cover the combined usages, the date-range union, and the case where one description has no range.
  • The protected static method is widened to public in a test subclass, as the root CLAUDE.md asks.
• Comments: the 2-line comment at :256 explains why the merge is needed and is worth keeping.
• Docs: none needed. This is a bug fix with nothing for users to configure.
• Not run: I didn't run the tests because this checkout has no node_modules.

Review threads

The listing returned no prior threads of mine (0). I resolved none and re-affirmed none.