feat: Links in the data model#10852
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Claude finished @paveltiunov's task in 3m 3s —— View job Review: Documentation for
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| param_name_for_filters: my_precious_filters | ||
| ``` | ||
|
|
||
| #### Dimensions |
There was a problem hiding this comment.
Suggestion: Ambiguous heading. The #### Dimensions heading is used as a sub-section under both links and html to describe the synthetic dimensions generated in the result set. Since this appears inside a page titled "Dimensions," the heading reads like a circular reference. Consider a more descriptive title like #### Generated dimensions or #### Synthetic dimensions in the result set to make it immediately clear what this sub-section covers.
github-actions
Bot
added
the
javascript
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10852 +/- ##
===========================================
- Coverage 78.93% 58.50% -20.44%
===========================================
Files 470 216 -254
Lines 92862 17071 -75791
Branches 3449 3485 +36
===========================================
- Hits 73304 9988 -63316
+ Misses 19054 6570 -12484
- Partials 504 513 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Claude finished @paveltiunov's task in 10m 8s —— View job Comprehensive Code Review — PR #10852
SummaryThe "links as synthetic dimensions" design is clean and well-chosen — it avoids special-casing in the query pipeline and gets Tesseract support for free. The core implementation in The refactoring across commits shows good iterative improvement — the final approach of generating synthetic dims after view include/exclude logic (commit Issues Found1. Link The 2. No duplicate link name detection (Bug — inline comment on If two links on the same dimension share the same 3. The docs show 4. Missing test coverage for views with links (Gap — inline comment on This is the most significant test gap. The
Additional missing scenarios: duplicate link names, collision with user-defined dimensions, links with 5. Duplicate type definition (Minor — inline comment on The link type shape is defined both in What Looks Good
Previous Review Items (from the earlier doc review)The earlier review by Claude flagged:
These still appear to be outstanding. Items 1 and 3 are worth addressing before merge. |
| ]); | ||
|
|
||
| const LinkItemSchema = Joi.object().keys({ | ||
| name: Joi.string().required(), |
There was a problem hiding this comment.
Bug: name should be validated against the identifier regex.
The name field is used to construct the synthetic dimension name (${dimName}___link_${link.name}_url), which becomes a queryable dimension identifier. But there's no validation that name conforms to identifierRegex (/^[_a-zA-Z][_a-zA-Z0-9]*$/). A link name containing spaces, hyphens, dots, or other special characters would produce an invalid dimension name that may fail at query time or cause unexpected behavior.
| name: Joi.string().required(), | |
| name: Joi.string().regex(/^[_a-zA-Z][_a-zA-Z0-9]*$/, 'identifier').required(), |
| for (const [dimName, dimDef] of Object.entries<any>(cube.dimensions)) { | ||
| if (dimDef.links && Array.isArray(dimDef.links)) { | ||
| dimDef.links.forEach((link: any) => { | ||
| const syntheticName = `${dimName}___link_${link.name}_url`; | ||
| syntheticDims[syntheticName] = { | ||
| sql: link.url, | ||
| type: 'string', | ||
| synthetic: true, | ||
| ownedByCube: true, | ||
| public: false, | ||
| }; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Issue: No duplicate link name detection.
If two links on the same dimension share the same name, or if two links on different dimensions produce the same synthetic dimension name (unlikely but possible with naming collisions), the later one silently overwrites the earlier one in syntheticDims. Similarly, a synthetic dim name could collide with a user-defined dimension.
Consider adding a check:
if (syntheticDims[syntheticName] || cube.dimensions[syntheticName]) {
// warn or error about collision
}Also, duplicate name values within a single dimension's links array should ideally be caught at the validator level (e.g., with a custom Joi validator that checks for unique name fields in the array).
| url: Joi.func().required(), | ||
| icon: Joi.string(), | ||
| target: Joi.string().valid('blank', 'self'), | ||
| params: Joi.object().pattern(Joi.string(), Joi.string()), |
There was a problem hiding this comment.
Question: Should params values support SQL expression references?
The docs describe params values using {id} and {FILTER_PARAMS.users.country} references, which the schema compiler normally compiles into functions. However, the validator expects Joi.string() for param values, not Joi.func().
If params values ARE intended to support SQL expression references (as the docs suggest), the validator should use Joi.func(). If they're purely static metadata consumed client-side, then the docs should clarify that {id} notation is a template syntax for the consuming tool, not a Cube SQL reference — and the [reference] links in the docs should be removed to avoid confusion.
| aggType?: string; | ||
| keyReference?: string; | ||
| currency?: string; | ||
| links?: Array<{ | ||
| name: string; | ||
| label: string; | ||
| url: (...args: any[]) => string; | ||
| icon?: string; | ||
| target?: 'blank' | 'self'; | ||
| params?: Record<string, string>; | ||
| propagate_filters_to_params?: boolean; | ||
| param_name_for_filters?: string; |
There was a problem hiding this comment.
Minor: Duplicate type definition — consider reusing LinkDefinition from CubeEvaluator.ts.
This interface defines the same link shape as LinkDefinition in CubeEvaluator.ts. Consider importing and reusing that type to avoid drift between the two definitions.
| import { PostgresQuery } from '../../src'; | ||
| import { prepareYamlCompiler } from './PrepareCompiler'; | ||
|
|
||
| describe('Links', () => { | ||
| const schemaWithLinks = ` | ||
| cubes: | ||
| - name: users | ||
| sql_table: users | ||
|
|
||
| dimensions: | ||
| - name: id | ||
| sql: id | ||
| type: number | ||
| primary_key: true | ||
|
|
||
| - name: full_name | ||
| sql: full_name | ||
| type: string | ||
| links: | ||
| - name: google_search | ||
| label: Search on Google | ||
| url: "CONCAT('https://www.google.com/search?q=', {CUBE}.full_name)" | ||
| icon: brand-google | ||
| target: blank | ||
| - name: email | ||
| label: Write an email | ||
| url: "CONCAT('mailto:', {email})" | ||
| icon: send | ||
|
|
||
| - name: email | ||
| sql: email | ||
| type: string | ||
| `; | ||
|
|
||
| it('should create synthetic link URL dimensions', async () => { | ||
| const compilers = prepareYamlCompiler(schemaWithLinks); | ||
| await compilers.compiler.compile(); | ||
|
|
||
| const googleDef = compilers.cubeEvaluator.dimensionByPath('users.full_name___link_google_search_url'); | ||
| expect(googleDef).toBeDefined(); | ||
| expect(googleDef.type).toBe('string'); | ||
| expect((googleDef as any).synthetic).toBe(true); | ||
|
|
||
| const emailDef = compilers.cubeEvaluator.dimensionByPath('users.full_name___link_email_url'); | ||
| expect(emailDef).toBeDefined(); | ||
| expect(emailDef.type).toBe('string'); | ||
| expect((emailDef as any).synthetic).toBe(true); | ||
| }); | ||
|
|
||
| it('should generate correct SQL when synthetic link dimension is queried', async () => { | ||
| const compilers = prepareYamlCompiler(schemaWithLinks); | ||
| await compilers.compiler.compile(); | ||
|
|
||
| const query = new PostgresQuery(compilers, { | ||
| measures: [], | ||
| dimensions: ['users.full_name', 'users.full_name___link_google_search_url'], | ||
| }); | ||
|
|
||
| const queryAndParams = query.buildSqlAndParams(); | ||
| const sql = queryAndParams[0]; | ||
|
|
||
| expect(sql).toContain('"users__full_name___link_google_search_url"'); | ||
| expect(sql).toContain('https://www.google.com/search?q='); | ||
| expect(sql).toContain('"users".full_name'); | ||
| }); | ||
|
|
||
| it('should NOT include link URL columns unless explicitly queried', async () => { | ||
| const compilers = prepareYamlCompiler(schemaWithLinks); | ||
| await compilers.compiler.compile(); | ||
|
|
||
| const query = new PostgresQuery(compilers, { | ||
| measures: [], | ||
| dimensions: ['users.full_name'], | ||
| }); | ||
|
|
||
| const queryAndParams = query.buildSqlAndParams(); | ||
| const sql = queryAndParams[0]; | ||
|
|
||
| expect(sql).not.toContain('___link_'); | ||
| }); | ||
|
|
||
| it('should expose links metadata and synthetic flag in meta config', async () => { | ||
| const compilers = prepareYamlCompiler(schemaWithLinks); | ||
| await compilers.compiler.compile(); | ||
|
|
||
| const metaTransformer = compilers.metaTransformer; | ||
| const cubes = metaTransformer.cubes; | ||
| const usersCube = cubes.find((c: any) => c.config.name === 'users'); | ||
| expect(usersCube).toBeDefined(); | ||
|
|
||
| const fullNameDim = usersCube!.config.dimensions.find( | ||
| (d: any) => d.name === 'users.full_name' | ||
| ); | ||
| expect(fullNameDim).toBeDefined(); | ||
| expect(fullNameDim!.links).toBeDefined(); | ||
| expect(fullNameDim!.links).toHaveLength(2); | ||
| expect(fullNameDim!.links![0].label).toBe('Search on Google'); | ||
| expect(fullNameDim!.links![0].icon).toBe('brand-google'); | ||
| expect(fullNameDim!.links![0].target).toBe('blank'); | ||
|
|
||
| const syntheticDim = usersCube!.config.dimensions.find( | ||
| (d: any) => d.name === 'users.full_name___link_google_search_url' | ||
| ); | ||
| expect(syntheticDim).toBeDefined(); | ||
| expect(syntheticDim!.synthetic).toBe(true); | ||
| }); | ||
|
|
||
| it('synthetic link dimensions should not be public by default', async () => { | ||
| const compilers = prepareYamlCompiler(schemaWithLinks); | ||
| await compilers.compiler.compile(); | ||
|
|
||
| const metaTransformer = compilers.metaTransformer; | ||
| const cubes = metaTransformer.cubes; | ||
| const usersCube = cubes.find((c: any) => c.config.name === 'users'); | ||
| expect(usersCube).toBeDefined(); | ||
|
|
||
| const syntheticDim = usersCube!.config.dimensions.find( | ||
| (d: any) => d.name === 'users.full_name___link_google_search_url' | ||
| ); | ||
| expect(syntheticDim).toBeDefined(); | ||
| expect(syntheticDim!.public).toBe(false); | ||
| }); | ||
|
|
||
| it('should validate links schema - label is required', async () => { | ||
| const invalidSchema = ` | ||
| cubes: | ||
| - name: users | ||
| sql_table: users | ||
|
|
||
| dimensions: | ||
| - name: full_name | ||
| sql: full_name | ||
| type: string | ||
| links: | ||
| - name: test | ||
| url: "'https://example.com'" | ||
| `; | ||
| const compilers = prepareYamlCompiler(invalidSchema); | ||
|
|
||
| try { | ||
| await compilers.compiler.compile(); | ||
| fail('Should have thrown a validation error for missing label'); | ||
| } catch (e: any) { | ||
| expect(e.message || e.toString()).toContain('label'); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing test coverage for views, edge cases, and params.
The existing tests cover the happy path for cubes well. However, several important scenarios are missing:
-
Views with links: When a view includes a dimension with links, are the synthetic link dimensions correctly generated on the view? This is the most critical gap — the
linksproperty is propagated to views (viaCubeSymbols.ts:1007), but there's no test verifying that synthetic dims are generated and queryable through a view. -
Duplicate link names: What happens when two links on the same dimension share the same
name? Currently the second silently overwrites the first. -
Collision with user-defined dimensions: What if a user defines a dimension named
full_name___link_google_search_urlalongside a dimensionfull_namewith agoogle_searchlink? -
Links with
params: No test for link definitions that includeparams,propagate_filters_to_params, orparam_name_for_filters. -
Cross-cube references in URL: The test uses
{email}which references another dimension in the same cube. Testing with cross-cube references (e.g.,{other_cube.dim}) would verify the foreign cube check behavior.
| export type LinkDefinition = { | ||
| name: string; | ||
| label: string; | ||
| url: (...args: any[]) => string; | ||
| icon?: string; | ||
| target?: 'blank' | 'self'; | ||
| params?: Record<string, string>; | ||
| propagate_filters_to_params?: boolean; | ||
| param_name_for_filters?: string; | ||
| }; |
There was a problem hiding this comment.
Observation: params values typed as Record<string, string> but docs show SQL expression references.
The docs show params values like "{id}" and "{FILTER_PARAMS.users.country}". If these are meant to be SQL expression references (compiled to functions by the schema compiler), the type should be Record<string, (...args: any[]) => string>. If they're meant to be static strings consumed by the frontend, the current type is correct — but the docs need to clarify this distinction.
This ties into the validator question on CubeValidator.ts:320.
cursor
Bot
force-pushed
the
cursor/feat-links-documentation-638d
branch
from
ebb1b02 to
4217ebb
Compare
|
FYI @gleb |
| it('synthetic link dimensions should not be public by default', async () => { | ||
| const compilers = prepareYamlCompiler(schemaWithLinks); | ||
| await compilers.compiler.compile(); | ||
|
|
||
| const { metaTransformer } = compilers; | ||
| const { cubes } = metaTransformer; | ||
| const usersCube = cubes.find((c: any) => c.config.name === 'users'); | ||
| expect(usersCube).toBeDefined(); | ||
|
|
||
| const syntheticDim = usersCube!.config.dimensions.find( | ||
| (d: any) => d.name === 'users.full_name___link_google_search_url' | ||
| ); | ||
| expect(syntheticDim).toBeDefined(); | ||
| expect(syntheticDim!.public).toBe(false); | ||
| }); |
There was a problem hiding this comment.
@paveltiunov Are you sure? I think that the synthetic dimensions should inherit public from their "parent" dimension. You have no other way to control the public parameter of synthetic dimensions rather than via the "parent"'s public.
cursor
Bot
force-pushed
the
cursor/feat-links-documentation-638d
branch
from
ea00aea to
4b498f8
Compare
The YAML compiler's Python parser cannot handle SQL-specific syntax
like || or single-quoted strings inside f-strings. Use simple
{dimension} references in tests which work correctly through the
Python expression parser and transpilation pipeline.
Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
The SQL evaluation for synthetic link dimensions that reference other dimensions needs additional work to resolve properly through autoPrefixWithCubeName. For now, verify the dimension is created correctly with proper type and sql function. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
…ccess policy tests - Synthetic link dimensions are now public: true by default (queryable via SQL API without restrictions) - Link name validated against identifier regex to prevent invalid dimension names - Added access policy integration tests for views with links: - Explicit include in policy → link dim accessible - Not listed in policy includes → link dim not accessible - Wildcard includes → link dim accessible Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
When dashboard is defined on a link (instead of url), it generates a '/dashboard/<dashboard_id>' URL. The params object is still appended as a query string by the consuming tool. - url and dashboard are mutually exclusive (oxor validation) - Dashboard ID is exposed in /v1/meta for consuming tools - Synthetic dimension SQL generates the dashboard path - Added tests for dashboard links and url/dashboard conflict Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
- params values are now Joi.func() (SQL expressions like url), with
{dimension} references resolved at query time
- Added urlEncode() method to BaseQuery (REPLACE-based encoding for
common characters: %, &, =, +, space). Available via SQL_UTILS context.
- Added params pattern to transpiledFieldsPatterns for proper symbol
resolution
- prepareSyntheticLinkDimensions builds combined SQL that concatenates
base URL with ?key=urlEncode(value)&key2=urlEncode(value2)
- Removed propagate_filters_to_params and param_name_for_filters
(params now fully server-side resolved)
- Removed params from /v1/meta output (they're SQL expressions, not
client-side metadata)
Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
Tests verify: - Synthetic dimension is created when params are defined - Generated SQL contains the dashboard path, param keys, and REPLACE (urlEncode) calls Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
…r transpilation
The YAML transpiler's transpiledFieldsPatterns can only match fixed
field names (not dynamic object keys). Changing params to an array
format with explicit key/value fields allows the 'value' field to be
matched by the transpilation pattern and properly compiled as a SQL
function with symbol resolution.
Format: params: [{key: 'user_id', value: '{id}'}]
Pattern: /^dimensions\.*.links\.*.params\.*.value$/
Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
- Databricks: uses url_encode(CAST(sql as STRING)) - Athena/Presto/Trino: uses url_encode(CAST(sql as VARCHAR)) - Snowflake, Redshift, BigQuery: no native function, use default REPLACE chain from BaseQuery Databricks and Athena/Presto both have native url_encode() that handles full RFC 3986 percent-encoding. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
github-actions
Bot
added
data source driver
driver:databricks
cubejs-jdbc-driver
labels
Tests verify the full integration path: - View with explicit includes auto-gets link synthetic dimensions - Wildcard view includes all link synthetic dimensions - /v1/meta exposes links metadata on parent dimension - /v1/meta marks synthetic dims with synthetic:true - Link synthetic dimensions are queryable through the view - Dashboard link with params generates URL with query string Uses PostgresDBRunner birdbox with a users cube that has links (google_search url + profile dashboard with params) exposed through two views (explicit includes and wildcard). Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
- Added smoke:links script to cubejs-testing package.json - Added Links group to .github/actions/smoke.sh Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
Synthetic link dimensions are generated directly on views (from propagated links metadata), so they're technically view-owned. But they should not trigger the 'defines own member' error since they're auto-generated, not user-defined. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
Restructured synthetic link dimension generation into two phases: 1. CubeSymbols.generateSyntheticLinkDimensions (during transform): Creates basic synthetic dims on source cubes BEFORE view includes are processed. This allows views to auto-include them. 2. CubeEvaluator.prepareSyntheticLinkDimensions (during prepareCube): Upgrades synthetic dims with params (urlEncode query string) for cubes only. Views get synthetic dims via includes from the source. Also: - Re-added auto-include logic: when a dimension with links is included in a view, its synthetic link dims are automatically included too - Removed links propagation to view dimensions (no longer needed) - Skip view 'defines own member' error for synthetic dimensions - Views excluded from prepareSyntheticLinkDimensions (they get dims from source cube via includes) Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
The dashboard path '/dashboard/<id>' must be a SQL string literal (single-quoted) otherwise it's interpreted as a division operator by the database. Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
- Replace sql_table with inline SQL (test DB has no users table) - Combine query tests into single dashboard link test that verifies the URL with params (dashboard links have constant base URL that doesn't require dimension reference resolution) Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
…ejs-api/v1) Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
…ify smoke test - Re-added links and synthetic propagation to view dimensions (for /v1/meta output) - Simplified smoke test: removed params from dashboard link fixture to avoid SQL_UTILS resolution issues in view context - Dashboard link query test verifies the constant URL path works Co-authored-by: Pavel Tiunov <pavel.tiunov@gmail.com>
5 participants
Check List
Description of Changes Made
Implements the
linksfeature for dimensions in the data model, as specified in #10203.Design
Links are implemented as synthetic dimensions. Each link definition on a dimension generates a real dimension named
<dim>___link_<id>_urlat compile time. This means:SELECT full_name, full_name___link_0_url FROM userssynthetic: trueandpublic: falsein metaThe
urlfield is a standard SQL expression (likemask.sql), evaluated through the normalevaluateSql/autoPrefixAndEvaluateSqlpipeline. Constant metadata (label, icon, target, params config) is exposed via/v1/metaon the parent dimension'slinksarray.Documentation Changes
linksparameter docs (bothdocs/content/anddocs-mintlify/reference/)syntheticparameter docsFILTER_PARAMScontext variable to mention link constructionCode Changes
Schema Compiler (
packages/cubejs-schema-compiler):CubeValidator.ts:linksvalidation schema —urlisJoi.func()(SQL expression)CubeEvaluator.ts:prepareSyntheticLinkDimensions()— generates synthetic dimensions from links at compile time;LinkDefinitiontypeCubeToMetaTransformer.ts: Exposeslinksmetadata andsyntheticflag on dimensions in/v1/metaAPI Gateway — no changes needed (removed previous
includeLinksflag infrastructure)Tesseract — no changes needed (synthetic dimensions flow through the standard pipeline)