From 7ae23ca92b13e8efa3834164b79e9d35f9e19844 Mon Sep 17 00:00:00 2001 From: Aleksandr Romanenko Date: Sun, 6 Sep 2026 15:58:40 +0200 Subject: [PATCH] fix(schema-compiler): rollup_join cube hints from time dimensions `rollup_join` derives its join tree from cube hints collected off the pre-aggregation's own references, and `cubesHintsFromPreAggregation` read only `measures` and `dimensions`. A `rollup_join` whose single reference to one of its cubes is that cube's `time_dimension` therefore produced no hint for it: the join tree covered one cube, `targetJoins` came back empty and the pre-aggregation was rejected with "Nothing to join in rollup join". Naming a dimension of that cube is not a workaround. It puts the hint in, but the rollup then only matches queries that request that dimension, and this shape cannot satisfy both requirements at once. Collect hints from `timeDimensions` as well. They are appended last, so the existing hint order - and with it the join tree root - is unchanged, and `R.uniq` absorbs cubes already covered by a measure or a dimension. This also lines the hint set up with the one the native SQL planner uses: measures, dimensions, segments and time dimensions. Both planners are affected. Under the native planner matching runs in Rust, but the pre-aggregation description - and with it the join tree - is still built here, so the same rejection surfaced through `findPreAggregationForQueryRust`. The native planner's own hint collection already covers time dimensions; a test pins that. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/adapter/PreAggregations.ts | 3 +- .../rollup-join-time-dimension-hints.test.ts | 142 ++++++++++++++++++ .../pre_aggregations_compiler.rs | 28 ++++ .../rollup_join_time_dimension_hints.yaml | 59 ++++++++ 4 files changed, 231 insertions(+), 1 deletion(-) create mode 100644 packages/cubejs-schema-compiler/test/unit/rollup-join-time-dimension-hints.test.ts create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/rollup_join_time_dimension_hints.yaml diff --git a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.ts b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.ts index 212032f98b1df..3117aa5f1d45a 100644 --- a/packages/cubejs-schema-compiler/src/adapter/PreAggregations.ts +++ b/packages/cubejs-schema-compiler/src/adapter/PreAggregations.ts @@ -1139,7 +1139,8 @@ export class PreAggregations { private cubesHintsFromPreAggregation(preAggObj: PreAggregationForQuery): string[][] { return R.uniq( preAggObj.references.measures.concat( - preAggObj.references.dimensions + preAggObj.references.dimensions, + (preAggObj.references.timeDimensions || []).map(td => td.dimension) ).map(p => p.split('.').slice(0, -1)) ); } diff --git a/packages/cubejs-schema-compiler/test/unit/rollup-join-time-dimension-hints.test.ts b/packages/cubejs-schema-compiler/test/unit/rollup-join-time-dimension-hints.test.ts new file mode 100644 index 0000000000000..5fe6907dc3db0 --- /dev/null +++ b/packages/cubejs-schema-compiler/test/unit/rollup-join-time-dimension-hints.test.ts @@ -0,0 +1,142 @@ +import { PostgresQuery } from '../../src/adapter/PostgresQuery'; +import { prepareYamlCompiler } from './PrepareCompiler'; + +// `rollup_join` builds its join tree from cube hints collected off the +// pre-aggregation's own references. Those hints have to cover every cube the +// join is meant to span, including a cube whose single reference is its +// `time_dimension` - otherwise the join tree covers one cube, `targetJoins` +// comes back empty and the pre-aggregation is rejected with "Nothing to join in +// rollup join". +// +// The shape has no workaround: naming a dimension of that cube would put the +// hint in, but then that dimension has to be requested by every query for the +// rollup to match at all. +// +// Both planners build the pre-aggregation description here, so both are covered. +const model = (joinedPreAggregation: string) => ` +cubes: + - name: locations + sql: > + SELECT 1 AS id, 'A' AS board_id, '2026-01-01'::timestamp AS ts UNION ALL + SELECT 2 AS id, 'A' AS board_id, '2026-01-02'::timestamp + joins: + - name: boards + sql: "{CUBE.board_id} = {boards.board_id}" + relationship: many_to_one + dimensions: + - name: id + sql: "{CUBE}.id" + type: number + primary_key: true + - name: board_id + sql: "{CUBE}.board_id" + type: string + - name: ts + sql: "{CUBE}.ts" + type: time + measures: + - name: count + type: count + pre_aggregations: + - name: locations_rollup + type: rollup + dimensions: + - board_id + time_dimension: ts + granularity: day + + - name: boards + sql: > + SELECT 'A' AS board_id, 1 AS good + dimensions: + - name: board_id + sql: "{CUBE}.board_id" + type: string + primary_key: true + measures: + - name: good_count + sql: "{CUBE}.good" + type: sum + pre_aggregations: + - name: boards_rollup + type: rollup + measures: + - good_count + dimensions: + - board_id +${joinedPreAggregation} +`; + +// Reaches `locations` only through its time dimension. +const timeDimensionOnly = ` + - name: joined + type: rollup_join + rollups: + - locations.locations_rollup + - boards.boards_rollup + measures: + - good_count + time_dimension: locations.ts + granularity: day +`; + +// Same, plus a dimension from `locations`. +const withAnchorDimension = ` + - name: joined + type: rollup_join + rollups: + - locations.locations_rollup + - boards.boards_rollup + measures: + - good_count + dimensions: + - locations.board_id + time_dimension: locations.ts + granularity: day +`; + +const preAggregationSql = async (useNativeSqlPlanner: boolean, joinedPreAggregation: string, query: any) => { + const compilers = prepareYamlCompiler(model(joinedPreAggregation)); + await compilers.compiler.compile(); + + return new PostgresQuery(compilers, { + timezone: 'UTC', + useNativeSqlPlanner, + ...query, + }) + .preAggregations.preAggregationsDescription() + .map((d: any) => d.loadSql?.[0] ?? '') + .join('\n'); +}; + +describe.each([ + ['legacy', false], + ['tesseract', true], +])('rollup_join cube hints (%s planner)', (_name, useNativeSqlPlanner) => { + const timeDimensions = [{ + dimension: 'locations.ts', + granularity: 'day', + dateRange: ['2026-01-01', '2026-01-31'], + }]; + + it('builds the join when a cube is referenced only by time_dimension', async () => { + const sql = await preAggregationSql(useNativeSqlPlanner, timeDimensionOnly, { + measures: ['boards.good_count'], + timeDimensions, + }); + + expect(sql).toContain('locations_locations_rollup'); + expect(sql).toContain('boards_boards_rollup'); + }); + + it('builds the join when a dimension of that cube is referenced too', async () => { + const sql = await preAggregationSql(useNativeSqlPlanner, withAnchorDimension, { + measures: ['boards.good_count'], + dimensions: ['locations.board_id'], + timeDimensions, + }); + + expect(sql).toContain('locations_locations_rollup'); + expect(sql).toContain('boards_boards_rollup'); + }); +}); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/pre_aggregations_compiler.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/pre_aggregations_compiler.rs index df68099ec8e2e..23312785eef89 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/pre_aggregations_compiler.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/pre_aggregations_compiler.rs @@ -1284,4 +1284,32 @@ mod tests { _ => panic!("Expected PreAggregationSource::Join"), } } + + #[test] + fn test_compile_rollup_join_cube_reached_only_by_time_dimension() { + let schema = MockSchema::from_yaml_file("common/rollup_join_time_dimension_hints.yaml"); + let test_context = TestContext::new(schema).unwrap(); + let query_tools = test_context.query_tools().clone(); + + let cube_names = vec!["boards".to_string(), "locations".to_string()]; + let mut compiler = PreAggregationsCompiler::try_new(query_tools, &cube_names).unwrap(); + + let pre_agg_name = PreAggregationFullName::new("boards".to_string(), "joined".to_string()); + let compiled = compiler.compile_pre_aggregation(&pre_agg_name).unwrap(); + + let single_name = |source: &PreAggregationSource| match source { + PreAggregationSource::Single(table) => table.name.clone(), + _ => panic!("Expected Single source"), + }; + + match compiled.source.as_ref() { + PreAggregationSource::Join(join) => { + assert_eq!(join.items.len(), 1); + assert_eq!(single_name(&join.items[0].from), "locations_rollup"); + assert_eq!(single_name(&join.items[0].to), "boards_rollup"); + assert_eq!(single_name(&join.root), "locations_rollup"); + } + _ => panic!("Expected PreAggregationSource::Join"), + } + } } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/rollup_join_time_dimension_hints.yaml b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/rollup_join_time_dimension_hints.yaml new file mode 100644 index 0000000000000..0b3e91c6fa410 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/rollup_join_time_dimension_hints.yaml @@ -0,0 +1,59 @@ +cubes: + - name: locations + sql: "SELECT 1 as id, 'A' as board_id, '2026-01-01'::timestamp as ts" + + joins: + - name: boards + relationship: many_to_one + sql: "{CUBE.board_id} = {boards.board_id}" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: board_id + type: string + sql: board_id + - name: ts + type: time + sql: ts + measures: + - name: count + type: count + pre_aggregations: + - name: locations_rollup + type: rollup + dimensions: + - board_id + time_dimension: ts + granularity: day + + - name: boards + sql: "SELECT 'A' as board_id, 1 as good" + + dimensions: + - name: board_id + type: string + sql: board_id + primary_key: true + measures: + - name: good_count + type: sum + sql: good + pre_aggregations: + - name: boards_rollup + type: rollup + measures: + - good_count + dimensions: + - board_id + # `locations` is reached only through its time dimension. + - name: joined + type: rollupJoin + measures: + - good_count + time_dimension: locations.ts + granularity: day + rollups: + - locations.locations_rollup + - boards.boards_rollup