fix(schema-compiler): resolve multi_stage order_by template in owning cube context#10858
Open
tlangton3 wants to merge 1 commit into
Open
Conversation
… cube context When a multi_stage measure (e.g. type: rank) is exposed through a view, its order_by template — which references other cube members by bare name — was being evaluated against the view's symbol table rather than the underlying cube's. If the view didn't expose the referenced member under its bare name (either not at all, or only under an alias), the resolver returned undefined and BaseQuery.evaluateSql crashed at the unguarded _objectWithResolvedProperties access with: TypeError: Cannot read properties of undefined (reading '_objectWithResolvedProperties') The referenced member is a cube-internal implementation detail of the rank — it doesn't need to be (and shouldn't be) exposed on every view that includes the rank. Fix: - CubeSymbols.generateIncludeMembers now tags view-generated measure definitions that carry an order_by template with orderByCubeName (the underlying cube's name), so the consumer of the symbol knows where the template's references were authored. - BaseQuery.evaluateSymbolSql uses symbol.orderByCubeName (falling back to cubeName for direct cube access) when evaluating the order_by template, matching the Rust planner's semantic of compiling measure templates in the owning cube's context. - BaseQuery.evaluateSql now throws a clear UserError when cubeEvaluator.resolveSymbol returns undefined, instead of leaving an unguarded _objectWithResolvedProperties access to surface as a generic TypeError. Adds a regression test exercising the production shape: a view that exposes only the rank + a time dim, with the rank's order_by referencing a cube measure not exposed on the view. Fixes cube-js#10856.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
A
multi_stage: truemeasure whoseorder_by.sqltemplate references another measure by bare name (e.g.{num_parcels}) crashes Cube at schema-compile time when queried through a view that doesn't expose the referenced member under its bare name — either not at all, or only under an alias. The crash presents as a genericTypeError: Cannot read properties of undefined (reading '_objectWithResolvedProperties')fromBaseQuery.evaluateSql.The referenced member is a cube-internal implementation detail of the rank — it doesn't need to be (and shouldn't be) exposed on every view that includes the rank. The Rust planner already follows this semantic; this PR brings the JS schema-compiler in line.
Fixes #10856.
Changes
packages/cubejs-schema-compiler/src/compiler/CubeSymbols.ts—generateIncludeMembersnow tags view-generated measure definitions that carry anorderBytemplate withorderByCubeName(the underlying cube's name from the include path).packages/cubejs-schema-compiler/src/adapter/BaseQuery.js—evaluateSymbolSqlusessymbol.orderByCubeNamewhen evaluating theorder_bytemplate, falling back tocubeNamefor direct cube access.evaluateSqlnow throws a clearUserErrorwhencubeEvaluator.resolveSymbolreturns undefined, replacing the unguarded_objectWithResolvedPropertiesaccess.packages/cubejs-schema-compiler/test/unit/multi-stage-orderby-view-alias.test.ts— new regression test exercising the production shape (view exposes the rank + a time dim, rank'sorder_byreferences a cube measure not included on the view).Implementation Details
The bug surfaced in the JS schema-compiler.
CubeSymbols.generateIncludeMembers(the path that builds the view's exposed measure symbols from underlying cube members) was propagating the rank'sorderBytemplate as raw text onto the view's generated definition, losing the "this template was authored on cube X" context. Then at query time,BaseQuery.evaluateSymbolSqlevaluated the template against the consumer's queried cubeName (the view), callingresolveSymbol(viewName, name). When the barenamewasn't a key in the view's symbol table — because the view stores aliased members under their alias key, not the original name — the lookup returned undefined, and the next line accessed._objectWithResolvedPropertieswithout a guard.TimeDimensionSymbol-style implicit suffixing isn't involved here; this is purely a name-not-present-on-the-view resolution gap.The Rust planner (
rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_symbol.rs:544-550) already compilesorder_byat schema-compile time inpath.cube_name()— the owning cube. The crash fires earlier in JS schema validation, before Tesseract takes over for SQL generation.The fix carries
orderByCubeNamethrough to the view-generated symbol soevaluateSqlcan pass it as the resolution context, matching the Rust behaviour. Falls back tocubeNamefor direct cube access (where the view path doesn't apply). Scoped narrowly toorderBy—filters,case, etc. weren't surfaced in the reported repro and aren't included in this patch.The defensive guard on
BaseQuery.js:3593is hygiene: even with the substantive fix, an unguarded._objectWithResolvedPropertiesonundefinedis a latent footgun. Any future resolution failure now surfaces a clearUserErrornaming the missing symbol and the cube it was looked up against.Testing
yarn tsc(cubejs-schema-compiler) — clean.npx jest --no-coverage --testPathPattern='dist/test/unit/multi-stage-orderby-view-alias'— passes.npx jest --no-coverage dist/test/unit/views.test.js dist/test/unit/postgres-query.test.js dist/test/unit/hierarchies.test.js— 25/25 passes (no regressions in adjacent areas).multi-stage-orderby-view-alias.test.tsverified to fail on master with the documentedTypeErrorand pass after the fix; the generated SQL contains the cube'sparcelscolumn in the rank'sORDER BY.EOF