fix(schema-compiler): Allow dimension-only measure expressions over multiple cubes - #11330
Conversation
…ultiple cubes Signed-off-by: Alex Qyoun-ae <4062971+MazterQyou@users.noreply.github.com>
github-actions
Bot
added
rust
|
Claude finished @MazterQyou's task in 47s —— View job Code ReviewVerdict: LGTM overall — 0 high, 0 medium, 1 low (style nit) DetailsTodos
Summary The fix is small, symmetric across the legacy JS planner ( Test coverage Good coverage of the three interesting cases:
Nice touch keeping the legacy-error assertion as a separate Findings
Security / performance No new user input paths, no query surface changes beyond loosening a previously-thrown error. Docs No user-facing API changes; error message is self-explanatory. Docs update not required. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| } else { | ||
| return Err(CubeError::user(format!( | ||
| "Expected single cube for dimension-only measure {}, got {:?}", | ||
| node.full_name(), | ||
| cube_names | ||
| ))); | ||
| if cube_names | ||
| .iter() | ||
| .any(|cube_name| join.is_multiplied(cube_name)) | ||
| { | ||
| return Err(CubeError::user(format!( | ||
| "Dimension-only measure {} references cubes {:?} that lead to row multiplication. Please rewrite it using sub query.", | ||
| node.full_name(), | ||
| cube_names | ||
| ))); | ||
| } | ||
| // Dimensions from several cubes, but none of them is on the | ||
| // multiplied side of a join - safe to evaluate the expression | ||
| // on top of the join tree as a regular measure. | ||
| vec![MeasureResult { | ||
| cube_name: node.cube_name().clone(), | ||
| measure: node.clone(), | ||
| multiplied: false, | ||
| }] | ||
| }; |
There was a problem hiding this comment.
Nit: this } else { if ... { ... } ... } will trigger clippy's collapsible_else_if lint. Consider collapsing to } else if cube_names.iter().any(...) { ... } else { ... } to keep the block flat and appease clippy in CI.
There was a problem hiding this comment.
Pull request overview
This PR updates Cube’s schema compiler/planners to allow dimension-only member-expression measures to reference dimensions from multiple joined cubes when those references do not introduce row-multiplication risk, while still failing with an actionable error when row multiplication would occur.
Changes:
- Updated the Tesseract (Rust) multiplied-measures collector to allow multi-cube dimension-only expressions when none of the referenced cubes are multiplied.
- Updated the legacy planner (BaseQuery.js) to apply the same rule and preserve a clear row-multiplication error when unsafe.
- Added Postgres integration tests covering the safe multi-cube case, the unsafe case triggered by query dimensions, and planner-specific behavior when combined with one-to-many measures.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rust/cube/cubesqlplanner/cubesqlplanner/src/planner/collectors/multiplied_measures_collector.rs | Allows multi-cube dimension-only member expressions when referenced cubes are not multiplied; keeps an error for row multiplication. |
| packages/cubejs-schema-compiler/src/adapter/BaseQuery.js | Mirrors the same multi-cube allowance/row-multiplication error behavior for the legacy JS planner. |
| packages/cubejs-schema-compiler/test/integration/postgres/member-expression.test.ts | Adds integration coverage for safe/unsafe multi-cube dimension-only expressions across planner modes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return Err(CubeError::user(format!( | ||
| "Dimension-only measure {} references cubes {:?} that lead to row multiplication. Please rewrite it using sub query.", | ||
| node.full_name(), | ||
| cube_names | ||
| ))); |
| if (cubeNamesForMeasure.some(cubeName => this.multipliedJoinRowResult(cubeName))) { | ||
| throw new Error(`Dimension-only measure ${measureName} references cubes (${cubeNamesForMeasure}) that lead to row multiplication. Please rewrite it using sub query.`); | ||
| } |
Check List
Description of Changes Made
This PR allows dimension-only member-expression measures referencing dimensions from multiple joined cubes when none of the referenced cubes is on the multiplied side of a join (both tesseract and legacy planner), keeping the error with an actionable message for the row-multiplication case. Related test is included.