fix(schema-compiler): a cube that extends another broke the parent's multi-stage measures #11641
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,255 @@ | ||
| import { PostgresQuery } from '../../src/adapter/PostgresQuery'; | ||
| import { prepareYamlCompiler } from './PrepareCompiler'; | ||
|
|
||
| // `extends` hands the extending cube the very definitions of the cube it extends, | ||
| // so every reference resolved per cube — a multi-stage `grain:`/`filter:`, a view | ||
| // default filter, a pre-aggregation's output column names — has to be written into | ||
| // an object owned by that cube. Written into the shared one it resolves to whichever | ||
| // cube is prepared last, and the other cubes end up carrying member paths of a cube | ||
| // that is not theirs. | ||
| describe('Multi-stage members of a cube that another cube extends', () => { | ||
| const baseFact = ` | ||
| - name: base_fact | ||
| sql: "SELECT 1 AS id, 1 AS dim_id, '2026-01-01'::date AS d, 10 AS v" | ||
| joins: | ||
| - name: dims | ||
| sql: "{CUBE}.dim_id = {dims}.id" | ||
| relationship: many_to_one | ||
| dimensions: | ||
| - name: id | ||
| sql: "{CUBE}.id" | ||
| type: number | ||
| primary_key: true | ||
| - name: d | ||
| sql: "{CUBE}.d" | ||
| type: time | ||
| - name: v | ||
| sql: "{CUBE}.v" | ||
| type: number | ||
| measures: | ||
| - name: v_sum | ||
| sql: "{v}" | ||
| type: sum | ||
| - name: daily_v | ||
| multi_stage: true | ||
| sql: "{v_sum}" | ||
| type: number | ||
| - name: linked | ||
| multi_stage: true | ||
| sql: "{daily_v}" | ||
| type: sum | ||
| grain: | ||
| include: | ||
| - d | ||
| - name: combining | ||
| multi_stage: true | ||
| sql: "CASE WHEN {d} IS NOT NULL THEN {daily_v} ELSE 0 END" | ||
| type: max | ||
| grain: | ||
| include: | ||
| - d | ||
| - name: outer_combining | ||
| multi_stage: true | ||
| sql: "{combining}" | ||
| type: number | ||
| - name: v_sum_all_dates | ||
| multi_stage: true | ||
| sql: "{v_sum}" | ||
| type: number | ||
| filter: | ||
| exclude: | ||
| - d | ||
| `; | ||
|
|
||
| const dims = ` | ||
| - name: dims | ||
| sql: "SELECT 1 AS id, 'a' AS name" | ||
| dimensions: | ||
| - name: id | ||
| sql: "{CUBE}.id" | ||
| type: number | ||
| primary_key: true | ||
| - name: name | ||
| sql: "{CUBE}.name" | ||
| type: string | ||
| `; | ||
|
|
||
| const childFact = ` | ||
| - name: child_fact | ||
| extends: base_fact | ||
| sql: "SELECT 1 AS id, 1 AS dim_id, '2026-01-01'::date AS d, 10 AS v, 'x' AS tag" | ||
| dimensions: | ||
| - name: tag | ||
| sql: "{CUBE}.tag" | ||
| type: string | ||
| `; | ||
|
|
||
| const model = (withChild: boolean) => `cubes:${dims}${baseFact}${withChild ? childFact : ''}`; | ||
|
|
||
| const compile = async (withChild: boolean) => { | ||
| const compilers = prepareYamlCompiler(model(withChild)); | ||
| await compilers.compiler.compile(); | ||
| return compilers; | ||
| }; | ||
|
|
||
| const buildSql = async (withChild: boolean, query: any, useNativeSqlPlanner: boolean) => { | ||
| const compilers = await compile(withChild); | ||
| return new PostgresQuery(compilers, { | ||
| timezone: 'UTC', | ||
| useNativeSqlPlanner, | ||
| ...query, | ||
| }).buildSqlAndParams(); | ||
| }; | ||
|
|
||
| it('resolves grain references against the cube that declares the member', async () => { | ||
| const { cubeEvaluator } = await compile(true); | ||
|
|
||
| expect(cubeEvaluator.evaluatedCubes.base_fact.measures.linked.grain?.includeReferences) | ||
| .toEqual(['base_fact.d']); | ||
| expect(cubeEvaluator.evaluatedCubes.child_fact.measures.linked.grain?.includeReferences) | ||
| .toEqual(['child_fact.d']); | ||
| }); | ||
|
|
||
| it('resolves filter references against the cube that declares the member', async () => { | ||
| const { cubeEvaluator } = await compile(true); | ||
|
|
||
| expect(cubeEvaluator.evaluatedCubes.base_fact.measures.v_sum_all_dates.filter?.excludeReferences) | ||
| .toEqual(['base_fact.d']); | ||
| expect(cubeEvaluator.evaluatedCubes.child_fact.measures.v_sum_all_dates.filter?.excludeReferences) | ||
| .toEqual(['child_fact.d']); | ||
| }); | ||
|
Comment on lines
+104
to
+120
Contributor
There was a problem hiding this comment. Good shape — asserting the parent's plan is identical with and without the extending cube is the right regression invariant, since "last cube prepared wins" is exactly what the bug was. Two coverage gaps worth considering, both cheap to add to the existing model string:
Also, the view-default-filter and pre-aggregation blocks only assert the resolved reference strings, not that a query/rollup match still builds — an |
||
|
|
||
| describe.each([ | ||
| ['native', true], | ||
| ['legacy', false], | ||
| ])('%s planner', (_name, useNativeSqlPlanner) => { | ||
| // Planning the parent's member must not depend on the extending cube being | ||
| // there at all, so the plan is compared against the same model without it. | ||
| const expectSamePlanWithAndWithoutChild = async (query: any) => { | ||
| const [withoutChild] = await buildSql(false, query, useNativeSqlPlanner); | ||
| const [withChild] = await buildSql(true, query, useNativeSqlPlanner); | ||
| expect(withChild).toEqual(withoutChild); | ||
| }; | ||
|
|
||
| it('plans a multi-stage measure with an explicit grain', async () => { | ||
| await expectSamePlanWithAndWithoutChild({ measures: ['base_fact.linked'] }); | ||
| await expect(buildSql(true, { measures: ['child_fact.linked'] }, useNativeSqlPlanner)).resolves.toBeDefined(); | ||
| }); | ||
|
|
||
| it('plans a chained multi-stage measure reading a grain dimension', async () => { | ||
| await expectSamePlanWithAndWithoutChild({ measures: ['base_fact.outer_combining'] }); | ||
| await expectSamePlanWithAndWithoutChild({ | ||
| measures: ['base_fact.outer_combining'], | ||
| dimensions: ['dims.name'], | ||
| }); | ||
| }); | ||
|
|
||
| it('plans a multi-stage measure with a filter directive', async () => { | ||
| await expectSamePlanWithAndWithoutChild({ | ||
| measures: ['base_fact.v_sum_all_dates'], | ||
| timeDimensions: [{ | ||
| dimension: 'base_fact.d', | ||
| granularity: 'month', | ||
| dateRange: ['2026-01-01', '2026-01-31'], | ||
| }], | ||
| }); | ||
| }); | ||
|
|
||
| it('plans a plain measure of a cube that another cube extends', async () => { | ||
| await expect(buildSql(true, { measures: ['base_fact.v_sum'] }, useNativeSqlPlanner)).resolves.toBeDefined(); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| describe('View default filters of a view that another view extends', () => { | ||
| const model = ` | ||
| cubes: | ||
| - name: orders | ||
| sql: "SELECT 1 AS id, 'usd' AS currency" | ||
| dimensions: | ||
| - name: id | ||
| sql: "{CUBE}.id" | ||
| type: number | ||
| primary_key: true | ||
| - name: currency | ||
| sql: "{CUBE}.currency" | ||
| type: string | ||
| measures: | ||
| - name: count | ||
| type: count | ||
|
|
||
| views: | ||
| - name: base_view | ||
| cubes: | ||
| - join_path: orders | ||
| includes: | ||
| - currency | ||
| - count | ||
| default_filters: | ||
| - member: currency | ||
| operator: equals | ||
| values: ["usd"] | ||
|
|
||
| - name: child_view | ||
| extends: base_view | ||
| `; | ||
|
|
||
| it('resolves the filter member against the view that declares the filter', async () => { | ||
| const { compiler, cubeEvaluator } = prepareYamlCompiler(model); | ||
| await compiler.compile(); | ||
|
|
||
| expect(cubeEvaluator.evaluatedCubes.base_view.defaultFilters?.map(f => f.memberReference)) | ||
| .toEqual(['base_view.currency']); | ||
| expect(cubeEvaluator.evaluatedCubes.child_view.defaultFilters?.map(f => f.memberReference)) | ||
| .toEqual(['child_view.currency']); | ||
| }); | ||
| }); | ||
|
|
||
| describe('Pre-aggregations of a cube that another cube extends', () => { | ||
| const model = ` | ||
| cubes: | ||
| - name: base | ||
| sql: "SELECT 1 AS id, '2026-01-01'::date AS d" | ||
| dimensions: | ||
| - name: id | ||
| sql: "{CUBE}.id" | ||
| type: number | ||
| primary_key: true | ||
| - name: d | ||
| sql: "{CUBE}.d" | ||
| type: time | ||
| measures: | ||
| - name: count | ||
| type: count | ||
| pre_aggregations: | ||
| - name: main | ||
| dimensions: | ||
| - id | ||
| measures: | ||
| - count | ||
| time_dimension: d | ||
| granularity: day | ||
| output_column_types: | ||
| - member: id | ||
| type: integer | ||
|
|
||
| - name: child | ||
| extends: base | ||
| sql: "SELECT 1 AS id, '2026-01-01'::date AS d, 'x' AS tag" | ||
| dimensions: | ||
| - name: tag | ||
| sql: "{CUBE}.tag" | ||
| type: string | ||
| `; | ||
|
|
||
| it('resolves output column names against the cube that declares the pre-aggregation', async () => { | ||
| const { compiler, cubeEvaluator } = prepareYamlCompiler(model); | ||
| await compiler.compile(); | ||
|
|
||
| const names = (cube: string) => (cubeEvaluator.evaluatedCubes[cube].preAggregations.main as any) | ||
| .outputColumnTypes.map((c: any) => c.name); | ||
|
|
||
| expect(names('base')).toEqual(['base.id']); | ||
| expect(names('child')).toEqual(['child.id']); | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix works (the
preAggregationsgetter inCubeSymbols.createCubememoizes a fresh merged map per cube object, so writing the key back only affects this cube), but it's worth noting two things:set preAggregations(_v) { /* Dont allow to modify */ }. Mutating the object the getter hands out sidesteps that intent and depends on an invariant declared in another file. A short reference to that invariant in the comment ("the map returned by the getter is per-cube, only its values are shared") would keep this from looking wrong to the next reader.outputColumnTypes. Everything above (lines 731–783) still mutates the shared pre-aggregation object in place —delete preAggregation.timeDimension,refreshRangeStart = buildRangeStart, etc. Those happen to be idempotent and not cube-scoped, so no correctness bug today, but thebuildRangeStart/refreshRangeStartwarning now fires only for whichever cube is prepared first. If you copied the entry unconditionally at the top of the loop body instead, the whole loop would operate on a cube-owned object and the class of bug this PR is fixing couldn't come back here.Minor: object spread drops accessors/non-enumerable properties. This file itself attaches a non-enumerable
maskSqlgetter to members (line 847), so the pattern isn't hypothetical — if a pre-aggregation ever grows one, it would silently vanish for the extending cube.