diff --git a/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js b/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js index 09e5c5fcb5554..21c3320d6db29 100644 --- a/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js +++ b/packages/cubejs-schema-compiler/src/adapter/MemberSqlTemplateCompiler.js @@ -11,15 +11,19 @@ * { * template: string | string[], * symbolPaths: string[][], // {arg:N} - * filterParams: [{ cube_name, name, column }], // {fp:N}, column = fn|string + * filterParams: [{ cube_name, name, column }], // {fp:N} * filterGroups: [{ filterParams: [...] }], // {fg:N} * securityContextValues: string[] // {sv:N} * } * * Member references are returned as recorded paths — the caller resolves them - * to symbols. FILTER_PARAMS column callbacks are deferred (returned as the raw - * JS function for the caller to invoke at render time); SECURITY_CONTEXT is - * resolved eagerly here against the provided context. + * to symbols. SECURITY_CONTEXT is resolved eagerly here against the provided + * context. + * + * A FILTER_PARAMS `column` is a plain string, or — when the data model gave a + * callback — a recording of its own in the same shape, with the filter values it + * takes as `{fpv:N}` placeholders and `valueParamsCount` of them. A callback + * taking its values through a rest parameter is returned as the bare function. * * The module holds no planner state — `securityContext` and `sqlUtils` are * passed in — so it can be unit-tested in isolation. @@ -29,11 +33,25 @@ const ARG_PREFIX = 'arg'; const FILTER_PARAM_PREFIX = 'fp'; const FILTER_GROUP_PREFIX = 'fg'; const SECURITY_VALUE_PREFIX = 'sv'; +const FILTER_VALUE_PREFIX = 'fpv'; function placeholder(prefix, index) { return `{${prefix}:${index}}`; } +// The proxies handed to the member's `sql` function are captured by any +// `FILTER_PARAMS` column callback it declares, so a nested compile cannot hand +// them a different state. They record into `state.target`, which +// `compileColumnCallback` swaps for the duration of that compile. +function emptyRecording() { + return { + symbolPaths: [], + filterParams: [], + filterGroups: [], + securityContextValues: [], + }; +} + // Returns the index of an equal path if it already exists, otherwise appends // and returns the new index. Paths are short arrays of strings, compared by // value, so repeated references collapse to a single placeholder. @@ -68,12 +86,12 @@ function memberReferenceProxy(path, state) { return undefined; } if (prop === 'sql') { - const index = uniqueInsertPath(state.symbolPaths, [...path, '__sql_fn']); + const index = uniqueInsertPath(state.target.symbolPaths, [...path, '__sql_fn']); const ph = placeholder(ARG_PREFIX, index); return () => ph; } if (prop === 'toString' || prop === 'valueOf') { - const index = uniqueInsertPath(state.symbolPaths, path); + const index = uniqueInsertPath(state.target.symbolPaths, path); const ph = placeholder(ARG_PREFIX, index); return () => ph; } @@ -84,13 +102,112 @@ function memberReferenceProxy(path, state) { // ---- FILTER_PARAMS / FILTER_GROUP ------------------------------------------ +// Declared parameters of a column callback: how many filter values it can take, +// and whether it takes them as a rest parameter. `Function.length` stops at the +// first defaulted parameter, so the parameter list is read from the source. +function declaredValueParams(fn) { + const source = fn.toString(); + const open = source.indexOf('('); + const arrow = source.indexOf('=>'); + if (open === -1 || (arrow !== -1 && arrow < open)) { + // `v => …`, a single parameter without parentheses. + return { count: 1, rest: false, inner: '' }; + } + + let depth = 0; + let close = -1; + for (let i = open; i < source.length; i++) { + const ch = source[i]; + if (ch === '(' || ch === '[' || ch === '{') depth++; + else if (ch === ')' || ch === ']' || ch === '}') { + depth--; + if (depth === 0 && ch === ')') { + close = i; + break; + } + } + } + if (close === -1) { + return { count: 0, rest: false, inner: source }; + } + + const inner = source.slice(open + 1, close); + const params = []; + let start = 0; + depth = 0; + for (let i = 0; i <= inner.length; i++) { + const ch = inner[i]; + if (ch === '(' || ch === '[' || ch === '{') depth++; + else if (ch === ')' || ch === ']' || ch === '}') depth--; + if (i === inner.length || (ch === ',' && depth === 0)) { + const param = inner.slice(start, i).trim(); + if (param) params.push(param); + start = i + 1; + } + } + + return { count: params.length, rest: params.some(p => p.startsWith('...')), inner }; +} + +// Parsing the parameter list can come up short — a bound or native function +// exposes no list, and a `)` inside a comment or a string default ends the scan +// early. Too few placeholders would render the missing values as `undefined`, so +// a count is trusted only when nothing could have thrown the scan off. +// +// `Function.length` counts the parameters before the first defaulted one, so it +// catches a scan that stopped short of that point and nothing after it. Past +// there only the text can vouch: a list holding neither a string nor a comment +// has nothing for the scan to trip over. A list that does holds a callback back +// to the render-time path, where every unreadable list already goes. +function valueParamsAreCertain(fn, count, inner) { + if (fn.toString().includes('[native code]')) { + return false; + } + if (count < fn.length) { + return false; + } + return !/['"`]|\/\*|\/\//.test(inner); +} + +// Compiles a column callback into a template of its own: its filter values +// become `{fpv:N}` placeholders and whatever it references is recorded into its +// own lists, so the placeholders it emits index its own dependencies rather than +// the enclosing member's. A rest parameter consumes as many values as the query +// happens to supply, which a fixed set of placeholders cannot express, so such a +// callback is left for the caller to invoke at render time. +function compileColumnCallback(column, state) { + const { count, rest, inner } = declaredValueParams(column); + if (rest || !valueParamsAreCertain(column, count, inner)) { + return column; + } + + const values = []; + for (let i = 0; i < count; i++) { + values.push(placeholder(FILTER_VALUE_PREFIX, i)); + } + + const recording = emptyRecording(); + const outer = state.target; + state.target = recording; + try { + const template = parseTemplateResult(column(...values)); + return { template, valueParamsCount: count, ...recording }; + } finally { + state.target = outer; + } +} + function filterParamsItemProxy(cubeName, name, state) { return { filter(column) { - const item = { cube_name: cubeName, name, column }; + const item = { + cube_name: cubeName, + name, + column: typeof column === 'function' ? compileColumnCallback(column, state) : column, + }; const toString = () => { - const index = state.filterParams.length; - state.filterParams.push(item); + const index = state.target.filterParams.length; + state.target.filterParams.push(item); return placeholder(FILTER_PARAM_PREFIX, index); }; // `__member` lets FILTER_GROUP recover the item; `toString` records and @@ -120,8 +237,8 @@ function filterGroupFn(state) { } return arg.__member; }); - const index = state.filterGroups.length; - state.filterGroups.push({ filterParams }); + const index = state.target.filterGroups.length; + state.target.filterGroups.push({ filterParams }); return placeholder(FILTER_GROUP_PREFIX, index); }; } @@ -164,7 +281,7 @@ function coerceToStringValue(value) { } function recordSecurityValue(value, state) { - return placeholder(SECURITY_VALUE_PREFIX, uniqueInsertString(state.securityContextValues, value)); + return placeholder(SECURITY_VALUE_PREFIX, uniqueInsertString(state.target.securityContextValues, value)); } function securityFilterFn(value, required, state) { @@ -247,23 +364,13 @@ function parseTemplateResult(result) { * @param {object} sqlUtils the SQL_UTILS object passed through to the template */ function compileMemberSql(sqlFn, argNames, securityContext, sqlUtils) { - const state = { - symbolPaths: [], - filterParams: [], - filterGroups: [], - securityContextValues: [], - }; + const root = emptyRecording(); + const state = { target: root }; const args = argNames.map(name => buildArg(name, state, securityContext, sqlUtils)); const template = parseTemplateResult(sqlFn(...args)); - return { - template, - symbolPaths: state.symbolPaths, - filterParams: state.filterParams, - filterGroups: state.filterGroups, - securityContextValues: state.securityContextValues, - }; + return { template, ...root }; } exports.compileMemberSql = compileMemberSql; diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/multi-stage-grain.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/multi-stage-grain.test.ts index d7a44751903c4..80e5087c2c323 100644 --- a/packages/cubejs-schema-compiler/test/integration/postgres/multi-stage-grain.test.ts +++ b/packages/cubejs-schema-compiler/test/integration/postgres/multi-stage-grain.test.ts @@ -133,6 +133,22 @@ cubes: - orders.status include: - orders.id + + # ── grain.exclude + filter.exclude on the same dim ──────────── + # "Share of the whole universe" denominator: the value ignores both + # the query filter on status and the status partition. The result row + # set must stay the query's — dropping the filter may not resurrect + # rows the query filtered out. + - name: amount_grain_and_filter_exclude_status + sql: "{CUBE.total_amount}" + type: sum + multi_stage: true + grain: + exclude: + - orders.status + filter: + exclude: + - orders.status `); if (getEnv('nativeSqlPlanner')) { @@ -257,6 +273,23 @@ cubes: { orders__status: 'pending', orders__category: 'books', orders__total_amount: '50', orders__amount_grain_keep_status_include_id: '125' }, { orders__status: 'pending', orders__category: 'electronics', orders__total_amount: '75', orders__amount_grain_keep_status_include_id: '125' }, ], { joinGraph, cubeEvaluator, compiler })); + + // ── grain.exclude + filter.exclude on the same dim ──────────── + it('grain.exclude + filter.exclude: keeps the query row set', async () => dbRunner.runQueryTest({ + measures: ['orders.total_amount', 'orders.amount_grain_and_filter_exclude_status'], + dimensions: ['orders.status', 'orders.category'], + filters: [ + { member: 'orders.status', operator: 'equals', values: ['completed'] }, + ], + order: [{ id: 'orders.status' }, { id: 'orders.category' }], + timezone: 'UTC', + }, [ + // The denominator is the per-category total over every status (books=250, + // electronics=275), while the rows stay the filtered ones — no pending or + // cancelled row may come back through the join. + { orders__status: 'completed', orders__category: 'books', orders__total_amount: '170', orders__amount_grain_and_filter_exclude_status: '250' }, + { orders__status: 'completed', orders__category: 'electronics', orders__total_amount: '200', orders__amount_grain_and_filter_exclude_status: '275' }, + ], { joinGraph, cubeEvaluator, compiler })); } else { // These tests rely on Tesseract; v1 planner does not implement the directive. test.skip('exclude: drops a dim from the partition', () => { expect(1).toBe(1); }); @@ -267,5 +300,6 @@ cubes: test.skip('keep_only: two-element array narrows to the (status, category) cell', () => { expect(1).toBe(1); }); test.skip('include: two-element array extends the leaf grain', () => { expect(1).toBe(1); }); test.skip('keep_only + include: keep narrows, include extends', () => { expect(1).toBe(1); }); + test.skip('grain.exclude + filter.exclude: keeps the query row set', () => { expect(1).toBe(1); }); } }); diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations-ungrouped-cross-cube.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations-ungrouped-cross-cube.test.ts new file mode 100644 index 0000000000000..fa2de31cfe3a2 --- /dev/null +++ b/packages/cubejs-schema-compiler/test/integration/postgres/pre-aggregations-ungrouped-cross-cube.test.ts @@ -0,0 +1,233 @@ +import { PostgresQuery } from '../../../src/adapter/PostgresQuery'; +import { prepareYamlCompiler } from '../../unit/PrepareCompiler'; + +const CUBES = ` +cubes: + - name: visitors + sql: "SELECT * FROM visitors" + joins: + - name: visitor_checkins + relationship: one_to_many + sql: "{CUBE.id} = {visitor_checkins.visitor_id}" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: source + type: string + sql: source + + - name: visitor_checkins + sql: "SELECT * FROM visitor_checkins" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: visitor_id + type: number + sql: visitor_id + - name: created_at + type: time + sql: created_at + measures: + - name: count + type: count + pre_aggregations: +`; + +// Renames every member, so the query spells members no pre-aggregation mentions. +const VIEW = ` +views: + - name: visitors_view + cubes: + - join_path: visitors + includes: + - id + - source + prefix: true + - join_path: visitors.visitor_checkins + includes: + - visitor_id + - count + prefix: true +`; + +// Grouped by a non-key dimension plus a cross-cube one, so its rows are already +// collapsed and reading them flat would drop the join. +const COLLAPSED_ROLLUP = ` + - name: joined_rollup + type: rollup + measures: + - count + dimensions: + - visitor_id + - visitors.source + time_dimension: created_at + granularity: day +`; + +// The same collapsed rollup without a time dimension, so the query's member set +// matches it exactly. Nothing but the primary-key rule stands between this query +// and a join-less read of collapsed rows. +const COLLAPSED_ROLLUP_NO_TIME_DIMENSION = ` + - name: no_keys_rollup + type: rollup + measures: + - count + dimensions: + - visitor_id + - visitors.source +`; + +// Same cross-cube shape, but grouped by the primary keys of both cubes, so each +// stored row is one raw joined row. +const KEYED_ROLLUP = ` + - name: joined_keys_rollup + type: rollup + measures: + - count + dimensions: + - id + - visitor_id + - visitors.id + - visitors.source +`; + +const BASE_QUERY = { + ungrouped: true, + // Without this a joined ungrouped query is rejected by `initUngrouped` unless + // it selects the primary keys of every joined cube. It is a server-level + // option (`CUBEJS_ALLOW_UNGROUPED_WITHOUT_PRIMARY_KEY`) that inherits + // `CUBESQL_SQL_PUSH_DOWN`'s default, so it is normally on. + allowUngroupedWithoutPrimaryKey: true, + timezone: 'America/Los_Angeles', + preAggregationsSchema: '', +}; + +const PLANNERS: [string, boolean][] = [ + ['legacy planner', false], + ['native planner', true], +]; + +describe('PreAggregations ungrouped cross-cube query', () => { + jest.setTimeout(200000); + + describe('rollup whose grouping collapses raw rows', () => { + const { compiler, joinGraph, cubeEvaluator } = prepareYamlCompiler( + CUBES + COLLAPSED_ROLLUP + VIEW + ); + + it.each(PLANNERS)('is not matched, keeping the join (%s)', async (_label, useNativeSqlPlanner) => { + await compiler.compile(); + + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + ...BASE_QUERY, + dimensions: ['visitor_checkins.visitor_id', 'visitors.source'], + filters: [{ member: 'visitors.source', operator: 'equals', values: ['google'] }], + useNativeSqlPlanner, + } as any); + + const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription(); + const [sql] = query.buildSqlAndParams(); + + expect(preAggregationsDescription).toEqual([]); + expect(sql).not.toContain('joined_rollup'); + expect(sql.toLowerCase()).toContain('join'); + }); + + it.each(PLANNERS)('is not matched through a view either (%s)', async (_label, useNativeSqlPlanner) => { + await compiler.compile(); + + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + ...BASE_QUERY, + dimensions: ['visitors_view.visitor_checkins_visitor_id', 'visitors_view.visitors_source'], + filters: [{ member: 'visitors_view.visitors_source', operator: 'equals', values: ['google'] }], + useNativeSqlPlanner, + } as any); + + const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription(); + const [sql] = query.buildSqlAndParams(); + + expect(preAggregationsDescription).toEqual([]); + expect(sql).not.toContain('joined_rollup'); + expect(sql.toLowerCase()).toContain('join'); + }); + }); + + // Native planner only: legacy matches this rollup and reads it without the + // join, so asserting legacy here would pin that bug. + describe('collapsed rollup whose member set matches the query exactly', () => { + const { compiler, joinGraph, cubeEvaluator } = prepareYamlCompiler( + CUBES + COLLAPSED_ROLLUP_NO_TIME_DIMENSION + VIEW + ); + + it('is not matched, keeping the join (native planner)', async () => { + await compiler.compile(); + + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + ...BASE_QUERY, + dimensions: ['visitor_checkins.visitor_id', 'visitors.source'], + useNativeSqlPlanner: true, + } as any); + + const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription(); + const [sql] = query.buildSqlAndParams(); + + expect(preAggregationsDescription).toEqual([]); + expect(sql).not.toContain('no_keys_rollup'); + expect(sql.toLowerCase()).toContain('join'); + }); + }); + + describe('rollup grouped by the primary keys of both cubes', () => { + const { compiler, joinGraph, cubeEvaluator } = prepareYamlCompiler( + CUBES + KEYED_ROLLUP + VIEW + ); + + it.each(PLANNERS)('is matched when the query carries the primary keys (%s)', async (_label, useNativeSqlPlanner) => { + await compiler.compile(); + + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + ...BASE_QUERY, + dimensions: [ + 'visitors.id', + 'visitor_checkins.id', + 'visitors.source', + 'visitor_checkins.visitor_id', + ], + useNativeSqlPlanner, + } as any); + + const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription(); + const [sql] = query.buildSqlAndParams(); + + expect(preAggregationsDescription.map((d: any) => d.tableName)).toEqual([ + 'visitor_checkins_joined_keys_rollup', + ]); + expect(sql).toContain('visitor_checkins_joined_keys_rollup'); + }); + + // Native planner only: legacy rejects this rollup even though one stored row + // is one raw joined row. Going through the view also pins that the view is + // not mistaken for a third cube. + it('is matched through a view (native planner)', async () => { + await compiler.compile(); + + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + ...BASE_QUERY, + dimensions: ['visitors_view.visitors_source', 'visitors_view.visitor_checkins_visitor_id'], + useNativeSqlPlanner: true, + } as any); + + const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription(); + const [sql] = query.buildSqlAndParams(); + + expect(preAggregationsDescription.map((d: any) => d.tableName)).toEqual([ + 'visitor_checkins_joined_keys_rollup', + ]); + expect(sql).toContain('visitor_checkins_joined_keys_rollup'); + }); + }); +}); diff --git a/packages/cubejs-schema-compiler/test/unit/filter-params-callback-column.test.ts b/packages/cubejs-schema-compiler/test/unit/filter-params-callback-column.test.ts new file mode 100644 index 0000000000000..a19cfd44801d7 --- /dev/null +++ b/packages/cubejs-schema-compiler/test/unit/filter-params-callback-column.test.ts @@ -0,0 +1,437 @@ +/* eslint-disable no-template-curly-in-string */ +import { PostgresQuery } from '../../src/adapter/PostgresQuery'; +import { prepareJsCompiler } from './PrepareCompiler'; + +// A `FILTER_PARAMS.….filter(cb)` column callback runs at render time, and the +// member references inside it must resolve to that member's SQL — the same as +// when the column is handed over as a template string. Here the callback is the +// only place referencing the time dimension, and it sits in a measure `filters:` +// entry, so nothing else in that SQL function records the reference. +const model = [ + 'cube(\'commission\', {', + ' sql: `SELECT * FROM commission`,', + ' measures: {', + ' dailyMrr: {', + ' sql: `${CUBE}.total`,', + ' type: `sum`,', + ' filters: [', + ' { sql: `${CUBE.pricingDuration} = \'DAILY\'` },', + ' { sql: `${FILTER_PARAMS.commission.reconciliationDate.filter((from, to) => `${CUBE.reconciliationDate} >= ${from} AND ${CUBE.reconciliationDate} < ${to}`)}` }', + ' ]', + ' }', + ' },', + ' dimensions: {', + ' id: {', + ' sql: `id`,', + ' type: `number`,', + ' primaryKey: true', + ' },', + ' partner: {', + ' sql: `partner`,', + ' type: `string`', + ' },', + ' currency: {', + ' sql: `currency`,', + ' type: `string`', + ' },', + ' pricingDuration: {', + ' sql: `pricing_duration`,', + ' type: `string`', + ' },', + ' reconciliationDate: {', + ' sql: `reconciliation_date`,', + ' type: `time`', + ' }', + ' },', + ' preAggregations: {', + ' main: {', + ' measures: [CUBE.dailyMrr],', + ' dimensions: [CUBE.partner, CUBE.currency],', + ' timeDimension: CUBE.reconciliationDate,', + ' granularity: `month`,', + ' partitionGranularity: `year`', + ' }', + ' }', + '});', +].join('\n'); + +// The callback's own SQL, with the time dimension resolved and the date bounds +// bound as params. +const PUSHED_DOWN_PREDICATE = /"commission"\.reconciliation_date >= \$\d+ AND "commission"\.reconciliation_date < \$\d+/; + +async function queryFor(useNativeSqlPlanner: boolean, options = {}) { + const { compiler, joinGraph, cubeEvaluator } = prepareJsCompiler(model); + await compiler.compile(); + + return new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['commission.dailyMrr'], + dimensions: ['commission.partner'], + filters: [{ + member: 'commission.currency', + operator: 'equals', + values: ['USD'], + }], + timeDimensions: [{ + dimension: 'commission.reconciliationDate', + granularity: 'month', + dateRange: ['2025-07-01', '2026-06-30'], + }], + timezone: 'UTC', + useNativeSqlPlanner, + ...options, + }); +} + +describe('FILTER_PARAMS callback column', () => { + describe.each([ + ['legacy planner', false], + ['native planner', true], + ])('%s', (_name, useNativeSqlPlanner) => { + it('renders the callback of a measure filter against the cube table', async () => { + // `pricingDuration` is outside the pre-aggregation, so the query reads the + // cube itself and the measure filter is rendered. + const query = await queryFor(useNativeSqlPlanner, { + dimensions: ['commission.partner', 'commission.pricingDuration'], + }); + const [sql] = query.buildSqlAndParams(); + + expect(sql).toMatch(PUSHED_DOWN_PREDICATE); + }); + + it('renders the callback of a measure filter in a pre-aggregation build query', async () => { + const query = await queryFor(useNativeSqlPlanner); + const [description]: any = query.preAggregations?.preAggregationsDescription(); + const [loadSql] = description.loadSql; + + expect(loadSql).toMatch(PUSHED_DOWN_PREDICATE); + }); + + // The column applies what its filter supplies, so an operator carrying no + // values leaves nothing to apply. + it('applies nothing when the filter on the column carries no values', async () => { + const query = await queryFor(useNativeSqlPlanner, { + dimensions: ['commission.partner', 'commission.pricingDuration'], + timeDimensions: [], + filters: [{ member: 'commission.reconciliationDate', operator: 'set' }], + }); + const [sql] = query.buildSqlAndParams(); + + expect(sql).not.toMatch(PUSHED_DOWN_PREDICATE); + expect(sql).toContain('1 = 1'); + }); + }); + + // A one-sided date operator carries one bound where this column takes both, so + // there is nothing to bind its second placeholder to and the column applies + // nothing. The filter still reaches the query on its own. + // + // The legacy planner instead fills the missing bound in with the current time, + // which for `beforeDate` lands the given value on the opposite side of the + // range and leaves the measure empty for every row the query keeps — so this + // is a divergence on purpose. + it('applies nothing for a filter carrying fewer values than the column takes', async () => { + const query = await queryFor(true, { + dimensions: ['commission.partner', 'commission.pricingDuration'], + timeDimensions: [], + filters: [{ + member: 'commission.reconciliationDate', + operator: 'beforeDate', + values: ['2025-07-01'], + }], + }); + const [sql] = query.buildSqlAndParams(); + + expect(sql).not.toMatch(PUSHED_DOWN_PREDICATE); + expect(sql).toContain('1 = 1'); + }); + + // A column renders only where its filter reaches the query, so the cube it + // reads is needed exactly there — and nowhere else. + describe.each([ + ['a member of another cube', '${users.city} IS NOT NULL AND ${CUBE.createdAt} >= ${from}'], + ['another cube directly', '${users}.city IS NOT NULL AND ${CUBE.createdAt} >= ${from}'], + ])('a column reading %s', (_name, callbackBody) => { + const schema = [ + 'cube(\'orders\', {', + ' sql: `SELECT * FROM orders`,', + ' joins: {', + ' users: {', + ' sql: `${CUBE}.user_id = ${users}.id`,', + ' relationship: `belongsTo`', + ' }', + ' },', + ' measures: {', + ' total: {', + ' sql: `${CUBE}.amount`,', + ' type: `sum`,', + ' filters: [', + ` { sql: \`\${FILTER_PARAMS.orders.createdAt.filter((from, to) => \`${callbackBody}\`)}\` }`, + ' ]', + ' }', + ' },', + ' dimensions: {', + ' id: {', + ' sql: `id`,', + ' type: `number`,', + ' primaryKey: true', + ' },', + ' createdAt: {', + ' sql: `created_at`,', + ' type: `time`', + ' }', + ' }', + '});', + 'cube(\'users\', {', + ' sql: `SELECT * FROM users`,', + ' dimensions: {', + ' id: {', + ' sql: `id`,', + ' type: `number`,', + ' primaryKey: true', + ' },', + ' city: {', + ' sql: `city`,', + ' type: `string`', + ' }', + ' }', + '});', + ].join('\n'); + + async function sqlFor(useNativeSqlPlanner: boolean, withFilter: boolean) { + const { compiler, joinGraph, cubeEvaluator } = prepareJsCompiler(schema); + await compiler.compile(); + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['orders.total'], + filters: withFilter + ? [{ member: 'orders.createdAt', operator: 'inDateRange', values: ['2025-07-01', '2026-06-30'] }] + : [], + timezone: 'UTC', + useNativeSqlPlanner, + }); + + return query.buildSqlAndParams()[0]; + } + + it.each([ + ['legacy planner', false], + ['native planner', true], + ])('joins that cube for %s when the filter reaches the query', async (_planner, useNativeSqlPlanner) => { + const sql = await sqlFor(useNativeSqlPlanner, true); + + expect(sql).toMatch(/join\s+users/i); + expect(sql).toContain('"users".city IS NOT NULL'); + }); + + it.each([ + ['legacy planner', false], + ['native planner', true], + ])('leaves that cube out for %s when the filter does not', async (_planner, useNativeSqlPlanner) => { + const sql = await sqlFor(useNativeSqlPlanner, false); + + expect(sql).not.toMatch(/join\s+users/i); + expect(sql).toContain('1 = 1'); + }); + }); +}); + +// A cube's `sql` builds the table the query reads from, so nothing a member +// reference could resolve against is in scope inside it — whichever way the +// reference is written. +describe('member references in a cube\'s sql', () => { + const cubeWith = (cubeSql: string, extraCube = '') => [ + extraCube, + 'cube(\'commission\', {', + ` sql: \`${cubeSql}\`,`, + ' measures: {', + ' total: {', + ' sql: `${CUBE}.total`,', + ' type: `sum`', + ' }', + ' },', + ' dimensions: {', + ' id: {', + ' sql: `id`,', + ' type: `number`,', + ' primaryKey: true', + ' },', + ' reconciliationDate: {', + ' sql: `reconciliation_date`,', + ' type: `time`', + ' }', + ' }', + '});', + ].join('\n'); + + async function sqlFor(schema: string, withFilter: boolean) { + const { compiler, joinGraph, cubeEvaluator } = prepareJsCompiler(schema); + await compiler.compile(); + + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['commission.total'], + timeDimensions: withFilter + ? [{ dimension: 'commission.reconciliationDate', dateRange: ['2025-07-01', '2026-06-30'] }] + : [], + timezone: 'UTC', + contextSymbols: { securityContext: { tenantId: 'acme' } }, + useNativeSqlPlanner: true, + }); + + return query.buildSqlAndParams()[0]; + } + + const REJECTED = /references member `commission\.reconciliationDate`/; + + // Each spelling reaches the same place, so each has to be reported the same + // way rather than resolved or left to render a qualifier for a table the query + // does not read. + it.each([ + ['a direct reference', 'SELECT * FROM commission WHERE ${CUBE.reconciliationDate} IS NOT NULL'], + ['a string filter param column', 'SELECT * FROM commission WHERE ${FILTER_PARAMS.commission.reconciliationDate.filter(`${CUBE.reconciliationDate}`)}'], + ['a filter param column callback', 'SELECT * FROM commission WHERE ${FILTER_PARAMS.commission.reconciliationDate.filter((from, to) => `${CUBE.reconciliationDate} >= ${from}`)}'], + ])('rejects %s', async (_name, cubeSql) => { + // Reported whether or not the query supplies the filter, since resolving the + // reference is what a cube's sql cannot do at all. + await expect(sqlFor(cubeWith(cubeSql), false)).rejects.toThrow(REJECTED); + await expect(sqlFor(cubeWith(cubeSql), true)).rejects.toThrow(REJECTED); + }); + + it('keeps a filter param column that reads plain columns', async () => { + const schema = cubeWith('SELECT * FROM commission WHERE ${FILTER_PARAMS.commission.reconciliationDate.filter((from, to) => `reconciliation_date >= ${from} AND reconciliation_date < ${to}`)}'); + + expect(await sqlFor(schema, false)).toContain('WHERE 1 = 1'); + expect(await sqlFor(schema, true)).toMatch(/reconciliation_date >= \$\d+ AND reconciliation_date < \$\d+/); + }); + + // A security context value needs no member in scope — it becomes a query + // param — so the row-level-security shape keeps working. + it('keeps a security context value inside a filter param column', async () => { + const schema = cubeWith('SELECT * FROM commission WHERE ${FILTER_PARAMS.commission.reconciliationDate.filter((from, to) => `reconciliation_date >= ${from} AND tenant = ${SECURITY_CONTEXT.tenantId.unsafeValue()}`)}'); + + expect(await sqlFor(schema, true)).toContain('tenant = acme'); + }); + + it('keeps a reference to another cube\'s sql', async () => { + const base = [ + 'cube(\'base\', {', + ' sql: `SELECT * FROM raw`,', + ' dimensions: {', + ' id: {', + ' sql: `id`,', + ' type: `number`,', + ' primaryKey: true', + ' }', + ' }', + '});', + ].join('\n'); + + expect(await sqlFor(cubeWith('SELECT * FROM ${base.sql()}', base), false)).toMatch(/FROM\s+raw\b/); + }); +}); + +// A column renders wherever the symbol carrying it does, which includes symbols a +// query reaches only through a filter, a segment or an order item. Its cube has +// to be joined in every one of those, or the qualifier it renders has no table +// behind it. +describe('a column reached through something other than a selected member', () => { + const COLUMN = '${FILTER_PARAMS.orders.createdAt.filter((from, to) => `${users.city} IS NOT NULL AND ${CUBE.createdAt} >= ${from}`)}'; + + const schema = [ + 'cube(\'orders\', {', + ' sql: `SELECT * FROM orders`,', + ' joins: {', + ' users: {', + ' sql: `${CUBE}.user_id = ${users}.id`,', + ' relationship: `belongsTo`', + ' }', + ' },', + ' measures: {', + ' count: {', + ' type: `count`', + ' },', + ` total: { sql: \`\${CUBE}.amount\`, type: \`sum\`, filters: [{ sql: \`${COLUMN}\` }] },`, + ' grouped: {', + ' sql: `${CUBE}.amount`,', + ' type: `sum`,', + ' filters: [{ sql: `${FILTER_GROUP(', + ' FILTER_PARAMS.orders.createdAt.filter((from, to) => `${users.city} IS NOT NULL AND ${CUBE.createdAt} >= ${from}`),', + ' FILTER_PARAMS.orders.status.filter((v) => `${CUBE.status} = ${v}`)', + ' )}` }]', + ' }', + ' },', + ' dimensions: {', + ' id: {', + ' sql: `id`,', + ' type: `number`,', + ' primaryKey: true', + ' },', + ' status: {', + ' sql: `status`,', + ' type: `string`', + ' },', + ' createdAt: {', + ' sql: `created_at`,', + ' type: `time`', + ' },', + ` flagged: { sql: \`CASE WHEN ${COLUMN} THEN 1 ELSE 0 END\`, type: \`number\` }`, + ' },', + ` segments: { recent: { sql: \`${COLUMN}\` } }`, + '});', + 'cube(\'users\', {', + ' sql: `SELECT * FROM users`,', + ' dimensions: {', + ' id: {', + ' sql: `id`,', + ' type: `number`,', + ' primaryKey: true', + ' },', + ' city: {', + ' sql: `city`,', + ' type: `string`', + ' }', + ' }', + '});', + ].join('\n'); + + const RANGE = { member: 'orders.createdAt', operator: 'inDateRange', values: ['2025-07-01', '2026-06-30'] }; + + async function sqlFor(query: any, useNativeSqlPlanner: boolean) { + const { compiler, joinGraph, cubeEvaluator } = prepareJsCompiler(schema); + await compiler.compile(); + + return new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + ...query, + timezone: 'UTC', + useNativeSqlPlanner, + }).buildSqlAndParams()[0]; + } + + it.each([ + ['a segment', { measures: ['orders.count'], segments: ['orders.recent'], filters: [RANGE] }], + ['a dimension only named in a filter', { measures: ['orders.count'], filters: [RANGE, { member: 'orders.flagged', operator: 'equals', values: ['1'] }] }], + ['a measure only named in a having filter', { measures: ['orders.count'], filters: [RANGE, { member: 'orders.total', operator: 'gt', values: ['1'] }] }], + // A FILTER_GROUP renders as one predicate, so its members share one verdict; + // an OR group survives only when every member of it matches the query. + ['a filter group under an or filter', { measures: ['orders.grouped'], filters: [{ or: [RANGE, { member: 'orders.status', operator: 'equals', values: ['x'] }] }] }], + ])('joins the cube read through %s', async (_name, query) => { + for (const useNativeSqlPlanner of [false, true]) { + const sql = await sqlFor(query, useNativeSqlPlanner); + + expect(sql).toContain('"users".city IS NOT NULL'); + expect(sql).toMatch(/join\s+users/i); + } + }); + + // The legacy planner cannot build this one at all: collecting its join hints + // recurses until the stack runs out. + it('joins the cube read through a filter group under an and filter', async () => { + const query = { + measures: ['orders.grouped'], + filters: [RANGE, { member: 'orders.status', operator: 'equals', values: ['x'] }], + }; + + const sql = await sqlFor(query, true); + + expect(sql).toContain('"users".city IS NOT NULL'); + expect(sql).toMatch(/join\s+users/i); + }); +}); diff --git a/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts b/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts index ebe37b975c398..e93a969b76a5d 100644 --- a/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts +++ b/packages/cubejs-schema-compiler/test/unit/member-sql-template-compiler.test.ts @@ -87,7 +87,7 @@ describe('MemberSqlTemplateCompiler — FILTER_PARAMS / FILTER_GROUP', () => { expect(res.filterParams).toEqual([{ cube_name: 'orders', name: 'status', column: 't.status' }]); }); - it('keeps the column callback as a function (deferred) and records {fp:N}', () => { + it('compiles a column callback into a template of its own and records {fp:N}', () => { const res = compileMemberSql( (FILTER_PARAMS) => `${FILTER_PARAMS.orders.status.filter((c) => `${c} > 0`)}`, ['FILTER_PARAMS'] @@ -96,9 +96,97 @@ describe('MemberSqlTemplateCompiler — FILTER_PARAMS / FILTER_GROUP', () => { expect(res.filterParams).toHaveLength(1); expect(res.filterParams[0].cube_name).toBe('orders'); expect(res.filterParams[0].name).toBe('status'); + // The filter value it takes becomes a placeholder of its own. + expect(res.filterParams[0].column.template).toBe('{fpv:0} > 0'); + expect(res.filterParams[0].column.valueParamsCount).toBe(1); + expect(res.filterParams[0].column.symbolPaths).toEqual([]); + }); + + // What a callback references belongs to the callback, not to the member whose + // sql declared it: the placeholders it emits index its own dependency list. + it('records a callback reference into the callback, not the enclosing member', () => { + const res = compileMemberSql( + (CUBE, FILTER_PARAMS) => `${FILTER_PARAMS.orders.createdAt.filter((from, to) => `${CUBE.createdAt} >= ${from} AND ${CUBE.createdAt} < ${to}`)}`, + ['CUBE', 'FILTER_PARAMS'] + ); + expect(res.template).toBe('{fp:0}'); + expect(res.symbolPaths).toEqual([]); + expect(res.filterParams[0].column.template) + .toBe('{arg:0} >= {fpv:0} AND {arg:0} < {fpv:1}'); + expect(res.filterParams[0].column.symbolPaths).toEqual([['CUBE', 'createdAt']]); + }); + + it('numbers callback references separately from the enclosing template', () => { + const res = compileMemberSql( + (CUBE, FILTER_PARAMS) => `${CUBE.a} AND ${FILTER_PARAMS.orders.b.filter((v) => `${CUBE.b} = ${v}`)}`, + ['CUBE', 'FILTER_PARAMS'] + ); + expect(res.template).toBe('{arg:0} AND {fp:0}'); + expect(res.symbolPaths).toEqual([['CUBE', 'a']]); + expect(res.filterParams[0].column.template).toBe('{arg:0} = {fpv:0}'); + expect(res.filterParams[0].column.symbolPaths).toEqual([['CUBE', 'b']]); + }); + + it('counts a defaulted parameter as a filter value', () => { + const res = compileMemberSql( + (FILTER_PARAMS) => `${FILTER_PARAMS.orders.a.filter((from, to = 1) => `d BETWEEN ${from} AND ${to}`)}`, + ['FILTER_PARAMS'] + ); + expect(res.filterParams[0].column.template).toBe('d BETWEEN {fpv:0} AND {fpv:1}'); + }); + + // A rest parameter takes as many values as the query supplies, which a fixed + // set of placeholders cannot express. + it('leaves a rest-parameter column callback uncompiled', () => { + const res = compileMemberSql( + (CUBE, FILTER_PARAMS) => `${FILTER_PARAMS.orders.a.filter((...vals) => vals.map(v => `${CUBE.a} = ${v}`).join(' OR '))}`, + ['CUBE', 'FILTER_PARAMS'] + ); expect(typeof res.filterParams[0].column).toBe('function'); - const captured = res.filterParams[0].column; - expect(captured('X')).toBe('X > 0'); + }); + + // Too few placeholders would render the missing values as `undefined`, so a + // parameter list that cannot be read in full is left to render time. + it.each([ + // eslint-disable-next-line no-extra-bind + ['a bound callback', ((from, to) => `d >= ${from} AND d < ${to}`).bind(null)], + ['a comment closing the parameter list', (from /* ) */, to) => `d >= ${from} AND d < ${to}`], + // `Function.length` counts the parameters before the first default, so it + // cannot speak for the parse past that point — whichever parameter carries + // the string that breaks the scan. + ['a default containing a paren', (from, to = '(') => `d >= ${from} AND d < ${to}`], + ['a first parameter defaulted to a paren', (from = ')', to) => `d >= ${from} AND d < ${to}`], + ['a paren in a default with a parameter behind it', (from, to = ')', third) => `d >= ${from} AND d < ${to} AND x = ${third}`], + // A quote alone is enough to hold the callback back: whether it hides a paren + // is exactly what the scan cannot tell. + ['a default containing a quoted string', (from, to = 'x') => `d >= ${from} AND d < ${to}`], + ])('leaves %s uncompiled', (_name, column) => { + const res = compileMemberSql( + (FILTER_PARAMS) => `${FILTER_PARAMS.orders.a.filter(column)}`, + ['FILTER_PARAMS'] + ); + expect(typeof res.filterParams[0].column).toBe('function'); + }); + + // A defaulted parameter only costs `Function.length` its witness; a list with + // nothing for the scan to trip over is still read in full. + it('compiles a callback whose first parameter has a plain default', () => { + const res = compileMemberSql( + (FILTER_PARAMS) => `${FILTER_PARAMS.orders.a.filter((from = 1, to) => `d >= ${from} AND d < ${to}`)}`, + ['FILTER_PARAMS'] + ); + expect(res.filterParams[0].column.template).toBe('d >= {fpv:0} AND d < {fpv:1}'); + }); + + it('records a security context value referenced from a column callback into the callback', () => { + const res = compileMemberSql( + (SECURITY_CONTEXT, FILTER_PARAMS) => `${FILTER_PARAMS.orders.a.filter((v) => `t = ${SECURITY_CONTEXT.tenantId} AND a = ${v}`)}`, + ['SECURITY_CONTEXT', 'FILTER_PARAMS'], + { tenantId: 'acme' } + ); + expect(res.securityContextValues).toEqual([]); + expect(res.filterParams[0].column.template).toBe('t = {sv:0} AND a = {fpv:0}'); + expect(res.filterParams[0].column.securityContextValues).toEqual(['acme']); }); it('records a filter group from filter-param args and yields {fg:0}', () => { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs index ac744baf5a063..d2a94f0e902a4 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/cube_bridge/member_sql.rs @@ -37,12 +37,68 @@ impl NativeDeserialize for SqlTemplate { } } +/// A column callback compiled into a template of its own. `{fpv:N}` +/// marks the Nth filter value the planner supplies at render time; +/// every other placeholder indexes `args`, the dependencies the +/// callback body touched. +#[derive(Clone, Debug)] +pub struct CompiledFilterParamsColumn { + pub template: SqlTemplate, + pub args: SqlTemplateArgs, + pub value_params_count: usize, +} + +impl CompiledFilterParamsColumn { + fn clone_to_context( + &self, + context_ref: &dyn NativeContextHolderRef, + ) -> Result { + Ok(Self { + template: self.template.clone(), + args: self.args.clone_to_context(context_ref)?, + value_params_count: self.value_params_count, + }) + } +} + +impl NativeDeserialize for CompiledFilterParamsColumn { + fn from_native(native_object: NativeObjectHandle) -> Result { + let object = native_object.to_struct()?; + let template = SqlTemplate::from_native(object.get_field("template")?)?; + let symbol_paths = Vec::>::from_native(object.get_field("symbolPaths")?)?; + let filter_params = deserialize_filter_params_vec(object.get_field("filterParams")?)?; + let filter_groups = object + .get_field("filterGroups")? + .to_array()? + .to_vec()? + .into_iter() + .map(FilterGroupItem::from_native) + .collect::, _>>()?; + let values = Vec::::from_native(object.get_field("securityContextValues")?)?; + let value_params_count = f64::from_native(object.get_field("valueParamsCount")?)? as usize; + Ok(Self { + template, + args: SqlTemplateArgs { + symbol_paths, + filter_params, + filter_groups, + security_context: SecutityContextProps { values }, + }, + value_params_count, + }) + } +} + /// Column argument passed to -/// `FILTER_PARAMS.cube.member.filter(...)`: either a plain column -/// name string, or a JS callback that produces the SQL snippet. +/// `FILTER_PARAMS.cube.member.filter(...)`: a plain column name +/// string, a callback already compiled into its own template, or — +/// when the callback takes its values as a rest parameter, so a fixed +/// set of value placeholders cannot express it — the raw JS callback +/// to invoke at render time. #[derive(Clone)] pub enum FilterParamsColumn { String(String), + Compiled(Rc), Callback(Rc), } @@ -53,6 +109,9 @@ impl FilterParamsColumn { ) -> Result { let res = match self { Self::String(s) => Self::String(s.clone()), + Self::Compiled(compiled) => { + Self::Compiled(Rc::new(compiled.clone_to_context(context_ref)?)) + } Self::Callback(callback) => Self::Callback(callback.clone_to_context(context_ref)?), }; Ok(res) @@ -66,6 +125,9 @@ impl NativeSerialize for FilterParamsColumn { ) -> Result, CubeError> { match self { FilterParamsColumn::String(s) => s.to_native(context.clone()), + FilterParamsColumn::Compiled(_) => Err(CubeError::internal( + "Compiled filter params column cannot be serialized back".to_string(), + )), FilterParamsColumn::Callback(cb) => { if let Ok(callback) = cb .clone() @@ -84,8 +146,13 @@ impl NativeSerialize for FilterParamsColumn { } impl NativeDeserialize for FilterParamsColumn { fn from_native(native_object: NativeObjectHandle) -> Result { + // A compiled column arrives as a struct carrying its template; a rest-param + // callback arrives as a bare function, which has no such field. let column = if let Ok(string_column) = String::from_native(native_object.clone()) { FilterParamsColumn::String(string_column) + } else if let Ok(compiled) = CompiledFilterParamsColumn::from_native(native_object.clone()) + { + FilterParamsColumn::Compiled(Rc::new(compiled)) } else { let callback = NativeFilterParamsCallback::from_native(native_object.clone())?; FilterParamsColumn::Callback(Rc::new(callback)) @@ -98,6 +165,7 @@ impl std::fmt::Debug for FilterParamsColumn { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::String(arg0) => f.debug_tuple("String").field(arg0).finish(), + Self::Compiled(compiled) => f.debug_tuple("Compiled").field(compiled).finish(), Self::Callback(_) => f .debug_tuple("Callback") .field(&"JsFunc".to_string()) diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/join.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/join.rs index 884eac91c6507..c1ffb306b2c72 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/join.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/join.rs @@ -11,6 +11,10 @@ use typed_builder::TypedBuilder; pub struct LogicalJoinItem { cube: Rc, on_sql: Rc, + /// Required, with no default: `false` is the answer that lets a rollup + /// collapsing this edge's rows serve a raw-row query, so a construction site + /// must not be able to omit it. + splits_rows: bool, } impl LogicalJoinItem { @@ -21,6 +25,12 @@ impl LogicalJoinItem { pub fn on_sql(&self) -> &Rc { &self.on_sql } + + /// Whether joining this cube in splits a row of its parent into + /// several, i.e. it sits on the `one_to_many` side of its edge. + pub fn splits_rows(&self) -> bool { + self.splits_rows + } } impl PrettyPrint for LogicalJoinItem { @@ -129,6 +139,7 @@ impl LogicalNode for LogicalJoin { Ok(LogicalJoinItem::builder() .cube(item.clone().into_logical_node()?) .on_sql(self_item.on_sql().clone()) + .splits_rows(self_item.splits_rows()) .build()) }) .collect::, _>>()?; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs index 0d6ee57f17ef6..25754b62f082a 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/logical_plan/optimizers/pre_aggregation/optimizer.rs @@ -2,17 +2,19 @@ use super::PreAggregationsCompiler; use super::*; use crate::logical_plan::visitor::{LogicalPlanRewriter, NodeRewriteResult}; use crate::logical_plan::*; -use crate::planner::collectors::has_multi_stage_members; +use crate::planner::collectors::{collect_cube_names_from_symbols, has_multi_stage_members}; use crate::planner::filter::FilterItem; use crate::planner::filter::FilterOp; use crate::planner::join_hints::JoinHints; use crate::planner::multi_fact_join_groups::{MeasuresJoinHints, MultiFactJoinGroups}; use crate::planner::planners::multi_stage::TimeShiftState; +use crate::planner::planners::CommonUtils; use crate::planner::state::State; use crate::planner::time_dimension::QueryDateTime; use crate::planner::MemberSymbol; use cubenativeutils::CubeError; -use std::collections::HashSet; +use std::cell::RefCell; +use std::collections::{HashMap, HashSet}; use std::rc::Rc; pub struct PreAggregationUsage { @@ -35,11 +37,24 @@ impl PreAggregationUsage { } } +/// What a query does with the rows underneath it, which decides whether a +/// pre-aggregation's grain has to match those rows one for one. +enum RowGrain { + /// The query aggregates, so a coarser stored grain is still usable. + Aggregated, + /// The query returns raw rows over the given join. `None` when the node is + /// not a plain cube join, leaving nothing to establish row identity against. + RawRows(Option>), +} + pub struct PreAggregationOptimizer { query_tools: Rc, allow_multi_stage: bool, usages: Vec, usage_counter: usize, + /// Resolved primary-key names per cube. Every candidate pre-aggregation asks + /// for the same cubes, and resolving them crosses the JS bridge. + primary_keys_cache: RefCell>>, } impl PreAggregationOptimizer { @@ -49,6 +64,7 @@ impl PreAggregationOptimizer { allow_multi_stage, usages: Vec::new(), usage_counter: 0, + primary_keys_cache: RefCell::new(HashMap::new()), } } @@ -88,6 +104,7 @@ impl PreAggregationOptimizer { root.query(), compiled_pre_aggregations, &TimeShiftState::default(), + true, )? { return Ok(Some(Rc::new( RootQuery::builder().ctes(vec![]).query(rewritten).build(), @@ -109,18 +126,22 @@ impl PreAggregationOptimizer { std::mem::take(&mut self.usages) } + // `is_user_query` marks the query the user actually asked for, as opposed to + // an internal multi-stage leaf. Only the former's `ungrouped` flag means + // "return raw rows"; a leaf may carry it purely for how its stage renders. fn try_rewrite_query( &mut self, query: &Rc, compiled_pre_aggregations: &[Rc], time_shifts: &TimeShiftState, + is_user_query: bool, ) -> Result>, CubeError> { for pre_aggregation in compiled_pre_aggregations.iter() { let external = pre_aggregation.external.unwrap_or(false); let date_range = Self::extract_date_range(&query.filter(), &self.query_tools, time_shifts, external); if let Some(rewritten) = - self.try_rewrite_simple_query(query, pre_aggregation, date_range)? + self.try_rewrite_simple_query(query, pre_aggregation, date_range, is_user_query)? { return Ok(Some(rewritten)); } @@ -134,10 +155,25 @@ impl PreAggregationOptimizer { query: &Rc, pre_aggregation: &Rc, date_range: Option<(String, String)>, + is_user_query: bool, ) -> Result>, CubeError> { - if let Some(matched_measures) = - self.is_schema_and_filters_match(&query.schema(), &query.filter(), pre_aggregation)? - { + // Row identity for an ungrouped read is judged against the join this + // very node will render, taken from the node itself rather than + // re-resolved, so the two can never disagree. + let row_grain = if is_user_query && query.modifers().ungrouped { + RowGrain::RawRows(match query.source() { + QuerySource::LogicalJoin(join) => Some(join.clone()), + _ => None, + }) + } else { + RowGrain::Aggregated + }; + if let Some(matched_measures) = self.is_schema_and_filters_match( + &query.schema(), + &query.filter(), + pre_aggregation, + row_grain, + )? { let source = self.make_pre_aggregation_source(pre_aggregation, &matched_measures, date_range)?; let new_query = Query::builder() @@ -169,9 +205,16 @@ impl PreAggregationOptimizer { &TimeShiftState::default(), external, ); - if let Some(matched_measures) = - self.is_schema_and_filters_match(schema, filter, pre_aggregation)? - { + // This node holds no `Query` of its own, so its `ungrouped` flag is + // not reachable here and no join is available to judge row identity + // against. An ungrouped request routed through here can still be + // served by a pre-aggregation that collapses its rows. + if let Some(matched_measures) = self.is_schema_and_filters_match( + schema, + filter, + pre_aggregation, + RowGrain::Aggregated, + )? { let source = self.make_pre_aggregation_source( pre_aggregation, &matched_measures, @@ -238,6 +281,7 @@ impl PreAggregationOptimizer { &multi_stage_leaf_measure.query, compiled_pre_aggregations, &multi_stage_leaf_measure.evaluation_context.time_shifts, + false, )? { let new_leaf = Rc::new(MultiStageLeafMeasure { measures: multi_stage_leaf_measure.measures.clone(), @@ -481,6 +525,7 @@ impl PreAggregationOptimizer { schema: &Rc, filters: &Rc, pre_aggregation: &CompiledPreAggregation, + row_grain: RowGrain, ) -> Result>, CubeError> { let helper = OptimizerHelper::new(); @@ -498,6 +543,12 @@ impl PreAggregationOptimizer { return Ok(None); } + if let RowGrain::RawRows(node_join) = &row_grain { + if !self.is_raw_rows_match(node_join.as_ref(), pre_aggregation)? { + return Ok(None); + } + } + // The query's join groups answer both the multiplicativity gate // and the join-path comparison below, so build them once. let query_groups = self.query_join_groups(schema, &all_measures)?; @@ -517,6 +568,20 @@ impl PreAggregationOptimizer { return Ok(None); }; + // An ungrouped read projects stored columns as they are, with no + // aggregate around them, so a measure kept as a mergeable sketch would + // reach the client as the sketch instead of a number. + if matches!(row_grain, RowGrain::RawRows(_)) { + for symbol in pre_aggregation.measures.iter() { + if !matched_measures.contains(symbol.full_name().as_str()) { + continue; + } + if symbol.as_measure()?.kind().is_stored_as_state() { + return Ok(None); + } + } + } + // Even when the query itself has no multiplied measures, a measure that // is multiplied in the pre-aggregation (because the pre-agg groups by a // multiplier dimension) stores a different value than the query expects, @@ -566,6 +631,125 @@ impl PreAggregationOptimizer { Ok(Some(matched_measures)) } + // An ungrouped query returns raw rows, so a pre-aggregation may serve it + // only when each of its stored rows is exactly one row of the raw join. + // + // What identifies such a row is the key of the join root plus the key of + // every cube joined in on an edge that splits a row of its parent into + // several. A cube reached only over non-splitting edges cannot make the + // output finer, so its key is not needed, while a cube that merely transits + // the tree still splits rows and counts even when the query names no member + // of it. The root is always needed: it is what tells apart the rows an outer + // join leaves unmatched. A cube without a primary key has no row identity at + // all, so nothing can be read raw from it. + // + // The join comes from the node being rewritten, so it is by construction the + // one that node renders — filters, join hints and order-by members included. + fn is_raw_rows_match( + &self, + node_join: Option<&Rc>, + pre_aggregation: &CompiledPreAggregation, + ) -> Result { + // Only a plain rollup describes the grain its rows were stored at. A + // join or union source is described by declared members that need not + // reflect what the underlying rollups actually store, so its grain + // cannot be established here. + if !matches!( + pre_aggregation.source.as_ref(), + PreAggregationSource::Single(_) + ) { + return Ok(false); + } + + // Anything but a plain cube join — a full-key aggregate, or a source + // already rewritten to a pre-aggregation — has no single join to judge + // row identity against. + let Some(node_join) = node_join else { + return Ok(false); + }; + let Some(root) = node_join.root() else { + return Ok(false); + }; + + let stored_dimensions: HashSet = pre_aggregation + .dimensions + .iter() + .map(|d| d.clone().resolve_reference_chain().full_name()) + .collect(); + + let joined_cubes: HashSet = std::iter::once(root.name().clone()) + .chain( + node_join + .joins() + .iter() + .map(|item| item.cube().name().clone()), + ) + .collect(); + + // The pre-aggregation must not be stored at a finer grain than the node + // reads either: a cube it groups by but the node never joins splits its + // rows further, so the same row would come back more than once. + for cube_name in Self::pre_aggregation_grain_cubes(pre_aggregation)? { + if !joined_cubes.contains(&cube_name) { + return Ok(false); + } + } + + let identifying_cubes = std::iter::once(root.name().clone()).chain( + node_join + .joins() + .iter() + .filter(|item| item.splits_rows()) + .map(|item| item.cube().name().clone()), + ); + + for cube_name in identifying_cubes { + let keys = self.resolved_primary_keys(&cube_name)?; + if keys.is_empty() { + return Ok(false); + } + if !keys.iter().all(|key| stored_dimensions.contains(key)) { + return Ok(false); + } + } + + Ok(true) + } + + /// Cubes that set the grain the pre-aggregation stores its rows at, which is + /// what it groups by: its dimensions and time dimensions, plus its segments, + /// which are appended to the dimension list when the table is materialized + /// and so group the stored rows too. Measures are excluded — a measure joins + /// whatever its own SQL references, but that join is aggregated away inside + /// the rollup and leaves its row count untouched. + fn pre_aggregation_grain_cubes( + pre_aggregation: &CompiledPreAggregation, + ) -> Result, CubeError> { + let members = pre_aggregation + .dimensions + .iter() + .chain(pre_aggregation.time_dimensions.iter()) + .chain(pre_aggregation.segments.iter()) + .cloned() + .collect::>(); + collect_cube_names_from_symbols(&members) + } + + fn resolved_primary_keys(&self, cube_name: &String) -> Result, CubeError> { + if let Some(cached) = self.primary_keys_cache.borrow().get(cube_name) { + return Ok(cached.clone()); + } + let keys = CommonUtils::new(self.query_tools.clone()) + .primary_keys_dimensions(cube_name)? + .into_iter() + .map(|key| key.resolve_reference_chain().full_name()) + .collect::>(); + self.primary_keys_cache + .borrow_mut() + .insert(cube_name.clone(), keys.clone()); + Ok(keys) + } + fn query_join_groups( &self, schema: &Rc, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_filter.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_filter.rs index 4cb5e629b3b17..c72348f2b5dd5 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_filter.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/base_filter.rs @@ -21,7 +21,7 @@ impl ToSql for BaseFilter { if !filters_ctx.filter_params_columns.is_empty() { let symbol_to_match = resolve_base_symbol(self.raw_member_evaluator_ref()).resolve_reference_chain(); - if let Some(filter_params_column) = filters_ctx + if let Some(filter_params_item) = filters_ctx .filter_params_columns .get(&symbol_to_match.full_name()) { @@ -31,8 +31,10 @@ impl ToSql for BaseFilter { .get(&symbol_to_match.full_name()) .and_then(|shift| shift.interval.as_ref()); return self.typed_filter().to_sql_for_filter_params( - filter_params_column, + filter_params_item, time_shift, + visitor, + node_processor, &query_tools, templates, filters_ctx, diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/typed_filter.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/typed_filter.rs index e9b7f3077115e..bb9acc21f293e 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/typed_filter.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/filter/typed_filter.rs @@ -5,6 +5,7 @@ use crate::physical_plan::sql_nodes::SqlNode; use crate::physical_plan::SqlEvaluatorVisitor; use crate::planner::filter::typed_filter::{resolve_base_symbol, FilterOp, TypedFilter}; use crate::planner::query_tools::QueryTools; +use crate::planner::sql_call::SqlCallFilterParamsItem; use crate::planner::sql_templates::PlanSqlTemplates; use crate::planner::FiltersContext; use crate::planner::SqlInterval; @@ -48,15 +49,17 @@ impl ToSql for TypedFilter { impl TypedFilter { pub fn to_sql_for_filter_params( &self, - column: &FilterParamsColumn, + item: &SqlCallFilterParamsItem, time_shift: Option<&SqlInterval>, + visitor: &SqlEvaluatorVisitor, + node_processor: Rc, query_tools: &Rc, plan_templates: &PlanSqlTemplates, filters_context: &FiltersContext, ) -> Result { let use_db_time_zone = !filters_context.use_local_tz; - match column { + match &item.column { FilterParamsColumn::String(column_sql) => { // Inside a time-shifted CTE the FILTER_PARAMS column must carry the // same shift as the regular time-dimension filter, otherwise its @@ -82,47 +85,92 @@ impl TypedFilter { }; dispatch_to_sql(self.operation(), &ctx) } + FilterParamsColumn::Compiled(compiled) => { + if time_shift.is_some() { + return Err(CubeError::user(format!( + "FILTER_PARAMS column for `{}` is a callback, which cannot carry the time \ + shift the surrounding query applies; pass the column as a string instead", + item.filter_symbol_name + ))); + } + let values = + self.filter_param_values(query_tools, plan_templates, use_db_time_zone)?; + // A column applies what its filter supplies, and nothing when the + // filter cannot supply what the column takes — a `set` or `notSet` + // operator carries no values at all, and a one-sided date operator + // carries one where the column takes both bounds. The filter still + // reaches the query on its own; only its restatement inside this + // SQL is dropped, which is narrower than binding a bound the + // filter never gave. + if values.len() < compiled.value_params_count { + return plan_templates.always_true(); + } + let Some(call) = &item.compiled_call else { + return Err(CubeError::internal(format!( + "Compiled filter params column for `{}` has no call", + item.filter_symbol_name + ))); + }; + call.eval_with_filter_values( + visitor, + node_processor, + query_tools.clone(), + plan_templates, + &values, + ) + } FilterParamsColumn::Callback(callback) => { // A callback column is opaque SQL produced by user code, so a // time shift can't be wrapped around it; it is rendered as-is. - let args = match self.operation() { - // RollingWindowOffset carries [from, to, trailing, leading, offset]; - // only the from/to dates are filter-param args for the callback. - FilterOp::DateRange(_) - | FilterOp::DateSingle(_) - | FilterOp::RollingWindowOffset(_) => { - let ctx = FilterSqlContext { - member_sql: "", - query_tools, - plan_templates, - use_db_time_zone, - use_raw_values: self.use_raw_values(), - }; - let from = self - .values() - .first() - .and_then(|v| v.to_param_string()) - .map(|v| ctx.format_and_allocate_from_date_no_cast(&v)) - .transpose()?; - let to = self - .values() - .get(1) - .and_then(|v| v.to_param_string()) - .map(|v| ctx.format_and_allocate_to_date_no_cast(&v)) - .transpose()?; - [from, to].into_iter().flatten().collect() - } - _ => self - .values() - .iter() - .filter_map(|v| v.to_param_string()) - .map(|v| query_tools.allocate_param(&v)) - .collect::>(), - }; + let args = + self.filter_param_values(query_tools, plan_templates, use_db_time_zone)?; callback.call(&args) } } } + + // The filter's values, formatted the way a `FILTER_PARAMS` column expects to + // receive them. + fn filter_param_values( + &self, + query_tools: &Rc, + plan_templates: &PlanSqlTemplates, + use_db_time_zone: bool, + ) -> Result, CubeError> { + let args = match self.operation() { + // RollingWindowOffset carries [from, to, trailing, leading, offset]; + // only the from/to dates are filter-param args for the callback. + FilterOp::DateRange(_) | FilterOp::DateSingle(_) | FilterOp::RollingWindowOffset(_) => { + let ctx = FilterSqlContext { + member_sql: "", + query_tools, + plan_templates, + use_db_time_zone, + use_raw_values: self.use_raw_values(), + }; + let from = self + .values() + .first() + .and_then(|v| v.to_param_string()) + .map(|v| ctx.format_and_allocate_from_date_no_cast(&v)) + .transpose()?; + let to = self + .values() + .get(1) + .and_then(|v| v.to_param_string()) + .map(|v| ctx.format_and_allocate_to_date_no_cast(&v)) + .transpose()?; + [from, to].into_iter().flatten().collect() + } + _ => self + .values() + .iter() + .filter_map(|v| v.to_param_string()) + .map(|v| query_tools.allocate_param(&v)) + .collect::>(), + }; + Ok(args) + } } fn dispatch_to_sql(op: &FilterOp, ctx: &FilterSqlContext) -> Result { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs index 47f195bdc2995..1a76613fa0cef 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan/visitor_context.rs @@ -45,7 +45,7 @@ impl VisitorContext { pub fn new_for_filter_params( query_tools: Rc, nodes_factory: &SqlNodesFactory, - filter_params_columns: HashMap, + filter_params_columns: HashMap, time_shifts: TimeShiftState, ) -> Self { let filters_context = FiltersContext { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/processors/full_key_aggregate/keys_aggregate_strategy.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/processors/full_key_aggregate/keys_aggregate_strategy.rs index 4374f24f1a13b..15314e04caa1e 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/processors/full_key_aggregate/keys_aggregate_strategy.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/physical_plan_builder/processors/full_key_aggregate/keys_aggregate_strategy.rs @@ -175,14 +175,26 @@ impl FullKeyAggregateStrategy for KeysFullKeyAggregateStrategy<'_> { }) .collect::, _>>()?; - // Null-safe dimension join when keys are derived from the - // measure refs (FULL JOIN-equivalent shape). For the JOIN-model - // path (explicit keys) it's a one-to-one match — null-safety is - // unnecessary but stays correct. + // Both shapes join on the null-safe comparison, unconditionally. + // A plain `=` never matches NULL to NULL, so a key row whose + // dimension is NULL would keep its place in the grid and lose the + // measure. + // + // It has to be unconditional because nothing here can rule NULL + // out. The data model carries no nullability information, and even + // a column declared NOT NULL reaches this join as NULL once the + // dimension is read through an outer join in the cube's join + // graph — so the grid can hold a NULL key whatever the source + // table says. + // + // The cost is the planner's ability to hash- or merge-join these + // keys on engines that treat the null-safe operator as unhashable. + // Correctness is not tradeable against it: the alternative drops a + // measure value silently, on a row that is still returned. join_builder.left_join_subselect( query, query_alias, - JoinCondition::new_dimension_join(conditions, !has_explicit_keys), + JoinCondition::new_dimension_join(conditions, true), ); } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/compiler.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/compiler.rs index 40fabd1420216..ac14f1142ad85 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/compiler.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/compiler.rs @@ -251,6 +251,25 @@ impl Compiler { &mut self, cube_name: &String, member_sql: Rc, + ) -> Result, CubeError> { + self.compile_sql_call_impl(cube_name, member_sql, false) + } + + /// Compiles a cube's own `sql`, where a member reference is rejected + /// rather than resolved. + pub fn compile_cube_sql_call( + &mut self, + cube_name: &String, + member_sql: Rc, + ) -> Result, CubeError> { + self.compile_sql_call_impl(cube_name, member_sql, true) + } + + fn compile_sql_call_impl( + &mut self, + cube_name: &String, + member_sql: Rc, + is_cube_sql: bool, ) -> Result, CubeError> { let call_builder = SqlCallBuilder::new( self, @@ -258,6 +277,11 @@ impl Compiler { self.base_tools.clone(), self.security_context.clone(), ); + let call_builder = if is_cube_sql { + call_builder.for_cube_sql() + } else { + call_builder + }; let sql_call = call_builder.build(&cube_name, member_sql.clone())?; Ok(Rc::new(sql_call)) } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/tree_ops.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/tree_ops.rs index 84638d735c939..6933a2327c47a 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/tree_ops.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/filter/tree_ops.rs @@ -102,3 +102,30 @@ pub fn has_filter_for_member(member_name: &String, filters: &[FilterItem]) -> bo } false } + +/// Structural equality that also compares the member each leaf filter targets. +/// `FilterItem`'s own `PartialEq` compares a filter's type, operator and values +/// but not its member, so two filters differing only in the dimension they +/// restrict count as equal there. Groups are compared element-wise in order. +/// Segments carry their member in `full_name` and compare as-is. +pub fn eq_with_member(a: &FilterItem, b: &FilterItem) -> bool { + match (a, b) { + (FilterItem::Item(a), FilterItem::Item(b)) => a.member_name() == b.member_name() && a == b, + (FilterItem::Group(a), FilterItem::Group(b)) => { + a.operator == b.operator + && a.items.len() == b.items.len() + && a.items + .iter() + .zip(b.items.iter()) + .all(|(a, b)| eq_with_member(a, b)) + } + _ => a == b, + } +} + +/// True when `items` holds a filter equal to `item` under [`eq_with_member`]. +pub fn contains_with_member(items: &[FilterItem], item: &FilterItem) -> bool { + items + .iter() + .any(|candidate| eq_with_member(item, candidate)) +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/join_tree.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/join_tree.rs index ebf0fb78edc72..0903d9984a732 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/join_tree.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/join_tree.rs @@ -10,14 +10,21 @@ pub struct JoinTreeItem { cube: Rc, original_from: String, on_sql: Rc, + splits_rows: bool, } impl JoinTreeItem { - pub fn new(cube: Rc, original_from: String, on_sql: Rc) -> Self { + pub fn new( + cube: Rc, + original_from: String, + on_sql: Rc, + splits_rows: bool, + ) -> Self { Self { cube, original_from, on_sql, + splits_rows, } } @@ -32,6 +39,16 @@ impl JoinTreeItem { pub fn on_sql(&self) -> &Rc { &self.on_sql } + + /// Whether joining this cube in splits a row of its parent into + /// several — true exactly for the `one_to_many` side of an edge. + /// Unlike `JoinTree::is_multiplied`, which answers whether a cube's + /// own rows repeat and is only populated for the cubes the query + /// asked for, this is a property of the edge itself and so also + /// holds for cubes that merely transit the tree. + pub fn splits_rows(&self) -> bool { + self.splits_rows + } } /// A resolved join tree: the root cube plus its joined cubes with the diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_planner.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_planner.rs index 6b70c32812a6c..99edf84fbdaa8 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_planner.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_planner.rs @@ -87,6 +87,7 @@ impl JoinPlanner { LogicalJoinItem::builder() .cube(Cube::new(item.cube().clone())) .on_sql(item.on_sql().clone()) + .splits_rows(item.splits_rows()) .build() }) .collect(); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_tree_builder.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_tree_builder.rs index c5a360ce78a3a..343face2164b1 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_tree_builder.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/join_tree_builder.rs @@ -26,10 +26,12 @@ impl JoinTreeBuilder { let static_data = join_definition.static_data(); let cube = self.utils.cube_from_path(static_data.original_to.clone())?; let on_sql = self.utils.compile_join_condition(join_definition.clone())?; + let relationship = join_definition.join()?.static_data().relationship.clone(); joins.push(JoinTreeItem::new( cube, static_data.original_from.clone(), on_sql, + relationship_splits_rows(&relationship), )); } Ok(JoinTree::new( @@ -39,3 +41,48 @@ impl JoinTreeBuilder { )) } } + +/// Whether joining the `to` side of an edge with this relationship splits one row +/// of the `from` side into several. Only many-to-one and one-to-one keep the row +/// count, and a relationship arrives either normalized or in one of its model +/// spellings, so every spelling of those two is listed. Anything unrecognized +/// counts as splitting: requiring a primary key that is not needed only refuses a +/// usable pre-aggregation, while omitting a needed one serves collapsed rows. +fn relationship_splits_rows(relationship: &str) -> bool { + !matches!( + relationship, + "belongsTo" + | "belongs_to" + | "many_to_one" + | "manyToOne" + | "hasOne" + | "has_one" + | "one_to_one" + | "oneToOne" + ) +} + +#[cfg(test)] +mod tests { + use super::relationship_splits_rows; + + #[test] + fn splits_rows_covers_every_relationship_spelling() { + for keeps_row_count in ["belongsTo", "many_to_one", "hasOne", "one_to_one"] { + assert!( + !relationship_splits_rows(keeps_row_count), + "`{keeps_row_count}` joins at most one row" + ); + } + for splits in ["hasMany", "one_to_many"] { + assert!( + relationship_splits_rows(splits), + "`{splits}` joins many rows" + ); + } + assert!( + relationship_splits_rows("something_else"), + "an unrecognized relationship has to be treated as splitting" + ); + } +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs index a52869d71a828..a5befaa8c1136 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/planners/multi_stage/multi_stage_query_planner.rs @@ -9,6 +9,7 @@ use crate::logical_plan::*; use crate::planner::collectors::has_multi_stage_members; use crate::planner::collectors::member_childs; use crate::planner::filter::base_filter::FilterType; +use crate::planner::filter::tree_ops; use crate::planner::filter::BaseFilter; use crate::planner::filter::FilterItem; use crate::planner::filter::FilterOperator; @@ -177,12 +178,16 @@ impl MultiStageQueryPlanner { .multi_stage() .map(|ms| ms.grain.clone()) .unwrap_or_default(); - // Window-path eligibility intentionally checks only `include`: - // `exclude` and `keep_only` are realised through the window's - // PARTITION BY at render time, so they don't disqualify the - // path. `include` extends the leaf grain, which the JOIN-model - // is required for. Revisit if window-path expands to cases - // where exclude/keep_only affect render correctness. + // Of the `grain` lists only `include` disqualifies the window path + // here: `exclude` and `keep_only` are realised through the window's + // PARTITION BY at render time, while `include` extends the leaf + // grain, which the JOIN-model is required for. + // + // Grain is not the only disqualifier. A `filter` directive that + // drops a filter the query restricts the grid by also rules the + // window path out. Detecting that needs the inherited state, which + // only exists further down in `make_queries_descriptions` — the + // flag is revoked there. let has_include = grain.include.as_ref().is_some_and(|v| !v.is_empty()); let use_window_path = matches!(member_type, MultiStageInodeMemberType::Aggregate) && !has_include @@ -473,6 +478,12 @@ impl MultiStageQueryPlanner { let member = member.resolve_reference_chain(); let member = transforms::apply_static_filter_to_symbol(&member, state.dimensions_filters())?; + // `filter: include` lets a stage filter a member the query around it does + // not, so activity is settled against this stage's own set. + let member = transforms::apply_filter_params_activity_to_symbol( + &member, + &transforms::filter_params_activity_filters(&state.all_filter_items()), + )?; let state = if member.is_dimension() { let mut new_state = state.as_ref().clone(); new_state.remove_multistage_dimensions(resolved_multi_stage_dimensions)?; @@ -519,7 +530,7 @@ impl MultiStageQueryPlanner { alias.clone(), ) } else { - let (multi_stage_member, is_ungrupped) = self + let (mut multi_stage_member, is_ungrupped) = self .create_multi_stage_inode_member(member.clone(), resolved_multi_stage_dimensions)?; let mut dimensions_to_add = multi_stage_member @@ -551,16 +562,55 @@ impl MultiStageQueryPlanner { // The window-path Aggregate inode skips step 2: the leaf stays // at the parent state plus any `include` extension, and the // window function does the `exclude` collapse at outer level. - let use_window_path = multi_stage_member.use_window_path(); - let new_state = { - let mut new_state = match directive_filter.as_ref().map(|f| &f.mode) { + let filtered_state = { + let mut filtered_state = match directive_filter.as_ref().map(|f| &f.mode) { Some(MultiStageFilterMode::Fixed) => self.root_state().as_ref().clone(), Some(MultiStageFilterMode::Relative) | None => state.as_ref().clone(), }; if let Some(filter) = &directive_filter { - apply_filter_directive_to_state(filter, &mut new_state); + apply_filter_directive_to_state(filter, &mut filtered_state); } + filtered_state + }; + + // Step 1 can drop a filter the query restricts the grid by. That is + // the point of the directive — the aggregation input widens — but + // the rows the inode *reports* must stay the query's, and only the + // JOIN-model can hold the two apart: its keys side enumerates the + // grid while its measure side spans the widened set. A window + // expression has one row set serving both roles, so it reports the + // widened rows too and values the query filtered out come back as + // result rows. Hand such an inode to the JOIN-model. + let query_filter_dropped = + query_filters_dropped(self.root_state(), &state, &filtered_state); + if query_filter_dropped { + multi_stage_member = multi_stage_member.with_use_window_path(false); + } + + // Whether this inode can actually act on that, decided here so both + // halves of the decision stay together — the keys side that carries + // the query's rows is requested further down. + // + // A parent state with no dimensions is a single-row grid that no + // filter change can widen, and an empty key-dimension list + // degenerates into a cross join rather than being rejected. A + // Dimension inode never reads `keys_input`, so building one leaves + // unreferenced CTEs behind. And a Rank inode ranks within whatever + // its source carries: handing it the keys side would shrink the + // ranked population to the grid and collapse the ranks, trading a + // widened row set for wrong values. Rank needs its rows restricted + // *after* the window, which this assembly cannot express. + let needs_query_grid = query_filter_dropped + && (!state.dimensions().is_empty() || !state.time_dimensions().is_empty()) + && !matches!( + multi_stage_member.inode_type(), + MultiStageInodeMemberType::Dimension | MultiStageInodeMemberType::Rank + ); + + let use_window_path = multi_stage_member.use_window_path(); + let new_state = { + let mut new_state = filtered_state; if !use_window_path && matches!( @@ -596,14 +646,15 @@ impl MultiStageQueryPlanner { scope, )?; - // JOIN-model: when new_state misses any dim that was on the - // parent's `state`, this inode shrinks the parent grain. We - // build keys-side descriptions per child on the parent state - // so the FullKeyAggregate can broadcast measure values back - // to the full query grain. Window-path Aggregate inodes - // (sum-of-sum / sum-of-count with no leaf-extending `include`) - // handle broadcast via the window expression instead and don't - // need keys_input. + // JOIN-model: when the measure side no longer enumerates the rows + // this inode has to report — because new_state misses a dim that + // was on the parent's `state`, or because a dropped query filter + // widened it — we build keys-side descriptions per child on the + // parent state, so the FullKeyAggregate broadcasts measure values + // onto the query grain and only onto it. Window-path Aggregate + // inodes (sum-of-sum / sum-of-count with no leaf-extending + // `include`) handle broadcast via the window expression instead and + // don't need keys_input. let mut keys_input: Vec> = vec![]; if !use_window_path { let new_state_has = |sym: &Rc| { @@ -619,7 +670,11 @@ impl MultiStageQueryPlanner { .iter() .chain(state.time_dimensions().iter()) .any(|d| !new_state_has(d)); - if any_missing { + // A dropped query filter needs the keys side for the same + // reason a shrunk grain does — the measure side no longer + // enumerates the query's rows — except here the grid keeps + // every dimension and only the row count within it grows. + if any_missing || needs_query_grid { self.make_childs( member.clone(), state.clone(), @@ -1091,6 +1146,52 @@ fn filter_directive_match_names(symbol: &Rc) -> Vec { } } +// True when `narrowed` lost a *query-level* filter that `base` restricts the +// grid by. Three conditions per filter: `base` has it, the query asked for it, +// and `narrowed` doesn't have it. +// +// The query membership check is what keeps the notion anchored to the result +// grid. A filter a parent multi-stage member introduced through its own +// `filter: include` narrows that parent's view, not the grid the query asked +// for; a child dropping it (`mode: fixed`) therefore cannot widen the grid +// past the query, and needs no keys side. +// +// Measure filters are absent from the comparison because they never reach a +// CTE state to begin with — `build_root_state` drops them — so there is no +// query-level measure filter for a directive to lose. Filters added on top of +// `base` don't count either: they shrink the grid, which is safe. +// +// The query-membership check compares whole filters, so a filter whose values +// were rewritten between the root state and `base` reads as one the query never +// asked for, and the drop goes undetected. That is deliberately out of scope: a +// path that rewrites filter values on the way down has to bound the row set by +// other means. The rolling-window date-range rewrite is the one such path, and +// it does — its rows come from the time series and its values through the frame +// condition, both built from the query's own range. A new rewriting path has to +// establish the same, or anchor this check on the member instead of the value. +fn query_filters_dropped( + root: &QueryProperties, + base: &QueryProperties, + narrowed: &QueryProperties, +) -> bool { + fn any_dropped(root: &[FilterItem], base: &[FilterItem], narrowed: &[FilterItem]) -> bool { + base.iter().any(|item| { + tree_ops::contains_with_member(root, item) + && !tree_ops::contains_with_member(narrowed, item) + }) + } + + any_dropped( + root.dimensions_filters(), + base.dimensions_filters(), + narrowed.dimensions_filters(), + ) || any_dropped( + root.time_dimensions_filters(), + base.time_dimensions_filters(), + narrowed.time_dimensions_filters(), + ) || any_dropped(root.segments(), base.segments(), narrowed.segments()) +} + fn apply_filter_directive_to_state(filter: &MultiStageFilter, state: &mut QueryProperties) { if let Some(exclude) = &filter.exclude { let names: Vec = exclude diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs index c69ad672ab318..2633890570c8e 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/query_properties.rs @@ -234,10 +234,51 @@ impl QueryProperties { } // Push every entry of `dimensions_filters` into matching `case` - // expressions on each member, filter and order item. Run once at - // construction; mutators do not re-apply it. + // expressions, and mark every FILTER_PARAMS binding by whether the query + // filters the members it renders from. Both cover each member, filter and + // order item. Run once at construction; mutators do not re-apply it. fn apply_static_filters(&mut self) -> Result<(), CubeError> { let dimensions_filters = self.dimensions_filters.clone(); + // A FILTER_PARAMS binding may name any filtered member, not only a + // dimension, so its activity is read from the whole set. + // + // A multi-stage stage may then filter more than the query around it. It + // builds its own `QueryProperties` from its state, so this runs again for + // it and settles activity against the set that stage renders with. + let all_filters = transforms::filter_params_activity_filters(&self.all_filter_items()); + for dim in self.dimensions.iter_mut() { + *dim = transforms::apply_filter_params_activity_to_symbol(dim, &all_filters)?; + } + for dim in self.time_dimensions.iter_mut() { + *dim = transforms::apply_filter_params_activity_to_symbol(dim, &all_filters)?; + } + for meas in self.measures.iter_mut() { + *meas = transforms::apply_filter_params_activity_to_symbol(meas, &all_filters)?; + } + // A column renders wherever its symbol does, which includes the symbols + // a query reaches only through a filter, a segment or an order item. + for filter_item in self.dimensions_filters.iter_mut() { + *filter_item = + transforms::apply_filter_params_activity_to_filter_item(filter_item, &all_filters)?; + } + for filter_item in self.measures_filters.iter_mut() { + *filter_item = + transforms::apply_filter_params_activity_to_filter_item(filter_item, &all_filters)?; + } + for filter_item in self.time_dimensions_filters.iter_mut() { + *filter_item = + transforms::apply_filter_params_activity_to_filter_item(filter_item, &all_filters)?; + } + for filter_item in self.segments.iter_mut() { + *filter_item = + transforms::apply_filter_params_activity_to_filter_item(filter_item, &all_filters)?; + } + for order_item in self.order_by.iter_mut().flatten() { + order_item.member_evaluator = transforms::apply_filter_params_activity_to_symbol( + &order_item.member_evaluator, + &all_filters, + )?; + } for dim in self.dimensions.iter_mut() { *dim = transforms::apply_static_filter_to_symbol(dim, &dimensions_filters)?; } @@ -395,14 +436,20 @@ impl QueryProperties { /// Concatenation of `time_dimensions_filters`, `dimensions_filters`, and /// `segments` into a single `Filter`. `measures_filters` are not included. - pub fn all_filters(&self) -> Option { - let items = self - .time_dimensions_filters + /// `time_dimensions_filters`, `dimensions_filters` and `segments` as a flat + /// list. `measures_filters` are HAVING-style and stay out. + pub fn all_filter_items(&self) -> Vec { + self.time_dimensions_filters .iter() .chain(self.dimensions_filters.iter()) .chain(self.segments.iter()) .cloned() - .collect_vec(); + .collect_vec() + } + + /// The same set as `all_filter_items`, as a single `Filter`. + pub fn all_filters(&self) -> Option { + let items = self.all_filter_items(); if items.is_empty() { None } else { @@ -1105,15 +1152,31 @@ impl QueryProperties { /// Equality over members (chain-resolved), the three filter slots, /// segments and time-shifts. Excludes ordering, limits, planner flags /// and join hints; for those fields use the full [`PartialEq`]. + /// + /// Filters are compared with [`tree_ops::eq_with_member`] rather than with + /// `FilterItem`'s own equality, which looks at a filter's operator and + /// values but not at the member it restricts. Two states filtering + /// different dimensions to the same value are different states, and + /// conflating them makes a CTE serve a filter it was never built for. pub fn eq_as_state(&self, other: &Self) -> bool { Self::members_equivalent(&self.dimensions, &other.dimensions) + && Self::filters_equivalent(&self.dimensions_filters, &other.dimensions_filters) && Self::members_equivalent(&self.time_dimensions, &other.time_dimensions) - && self.dimensions_filters == other.dimensions_filters - && self.time_dimensions_filters == other.time_dimensions_filters - && self.measures_filters == other.measures_filters - && self.segments == other.segments + && Self::filters_equivalent( + &self.time_dimensions_filters, + &other.time_dimensions_filters, + ) + && Self::filters_equivalent(&self.measures_filters, &other.measures_filters) + && Self::filters_equivalent(&self.segments, &other.segments) && self.time_shifts == other.time_shifts } + + fn filters_equivalent(a: &[FilterItem], b: &[FilterItem]) -> bool { + a.len() == b.len() + && a.iter() + .zip(b.iter()) + .all(|(a, b)| tree_ops::eq_with_member(a, b)) + } } /// Equality over every semantic field. Members are compared by reference- diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs index 76115c9cc4437..b7aee3eb3417f 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs @@ -95,6 +95,8 @@ impl SqlDependency { /// - `fp:N` — Nth filter param. /// - `fg:N` — Nth filter group. /// - `sv:N` — Nth security-context value. +/// - `fpv:N` — Nth filter value, supplied when a compiled +/// `FILTER_PARAMS` column callback is rendered. pub struct SqlCallArg; impl SqlCallArg { @@ -102,6 +104,7 @@ impl SqlCallArg { const FILTER_PARAM_PREFIX: &'static str = "fp"; const FILTER_GROUP_PREFIX: &'static str = "fg"; const SECURITY_VALUE_PREFIX: &'static str = "sv"; + const FILTER_VALUE_PREFIX: &'static str = "fpv"; pub fn dependency(i: usize) -> String { format!("{{{}:{}}}", Self::ARG_PREFIX, i) @@ -124,6 +127,17 @@ impl SqlCallArg { pub struct SqlCallFilterParamsItem { pub filter_symbol_name: String, pub column: FilterParamsColumn, + /// The column callback compiled into a call of its own, when the + /// data model gave one. Its placeholders index its own + /// dependencies, so it is rendered on its own rather than spliced + /// into the enclosing template. + pub compiled_call: Option>, + /// Whether the query this call is being planned for filters the + /// member this binding names. Only then does the column render, so + /// only then do the members it reads belong to the enclosing + /// member's dependencies — and only then may they pull a cube into + /// the join. + pub active: bool, } /// `FILTER_GROUP` binding from the data-model SQL: several @@ -195,6 +209,20 @@ impl SqlCall { node_processor: Rc, query_tools: Rc, templates: &PlanSqlTemplates, + ) -> Result { + self.eval_with_filter_values(visitor, node_processor, query_tools, templates, &[]) + } + + /// Renders the call with the filter values a compiled + /// `FILTER_PARAMS` column takes through its `{fpv:N}` + /// placeholders. + pub fn eval_with_filter_values( + &self, + visitor: &SqlEvaluatorVisitor, + node_processor: Rc, + query_tools: Rc, + templates: &PlanSqlTemplates, + filter_values: &[String], ) -> Result { if let SqlTemplate::String(template) = &self.template { let (filter_params, filter_groups, deps, context_values) = @@ -206,6 +234,7 @@ impl SqlCall { &filter_params, &filter_groups, &context_values, + filter_values, ) } else { Err(CubeError::internal( @@ -237,6 +266,7 @@ impl SqlCall { &filter_params, &filter_groups, &context_values, + &[], )?] } SqlTemplate::StringVec(templates) => templates @@ -248,6 +278,7 @@ impl SqlCall { &filter_params, &filter_groups, &context_values, + &[], ) }) .collect::, _>>()?, @@ -349,13 +380,27 @@ impl SqlCall { let filter_params = filter_params .iter() .map(|s| { - Self::substitute_template(s, &deps, &filter_params, &filter_groups, &context_values) + Self::substitute_template( + s, + &deps, + &filter_params, + &filter_groups, + &context_values, + &[], + ) }) .collect::, _>>()?; let filter_groups = filter_groups .iter() .map(|s| { - Self::substitute_template(s, &deps, &filter_params, &filter_groups, &context_values) + Self::substitute_template( + s, + &deps, + &filter_params, + &filter_groups, + &context_values, + &[], + ) }) .collect::, _>>()?; @@ -363,13 +408,14 @@ impl SqlCall { } /// Substitute placeholders in template string with computed values in a single pass - /// Supports placeholders: {arg:N}, {fp:N}, {fg:N}, {sv:N} + /// Supports placeholders: {arg:N}, {fp:N}, {fg:N}, {sv:N}, {fpv:N} fn substitute_template( template: &str, deps: &[String], filter_params: &[String], filter_groups: &[String], security_values: &[String], + filter_values: &[String], ) -> Result { let mut result = String::with_capacity(template.len()); let mut chars = template.chars().peekable(); @@ -404,6 +450,7 @@ impl SqlCall { SqlCallArg::FILTER_PARAM_PREFIX => filter_params.get(idx), SqlCallArg::FILTER_GROUP_PREFIX => filter_groups.get(idx), SqlCallArg::SECURITY_VALUE_PREFIX => security_values.get(idx), + SqlCallArg::FILTER_VALUE_PREFIX => filter_values.get(idx), _ => { result.push('{'); result.push_str(&placeholder); @@ -456,7 +503,7 @@ impl SqlCall { let mut filter_params_columns = HashMap::new(); for itm in items { filter_params_columns - .insert(itm.filter_symbol_name.clone(), itm.column.clone()); + .insert(itm.filter_symbol_name.clone(), (*itm).clone()); } let context = VisitorContext::new_for_filter_params( @@ -515,6 +562,25 @@ impl SqlCall { .collect() } + fn active_filter_params(&self) -> impl Iterator { + self.filter_params + .iter() + .chain( + self.filter_groups + .iter() + .flat_map(|g| g.filter_params.iter()), + ) + .filter(|item| item.active) + } + + fn filter_params_mut(&mut self) -> impl Iterator { + self.filter_params.iter_mut().chain( + self.filter_groups + .iter_mut() + .flat_map(|g| g.filter_params.iter_mut()), + ) + } + pub fn struct_eq(&self, other: &Self) -> bool { self.template == other.template && self.deps.len() == other.deps.len() @@ -540,6 +606,12 @@ impl SymbolDeps for Rc { SqlDependency::CubeRef(cr) => visitor.cube_ref(cr)?, } } + // An active column renders, so what it reads is read by this call too. + for item in self.active_filter_params() { + if let Some(call) = &item.compiled_call { + call.visit_deps(visitor)?; + } + } std::ops::ControlFlow::Continue(()) } @@ -550,6 +622,19 @@ impl SymbolDeps for Rc { visitor.symbol(s)?; } } + // Reached whatever the activity, so a rewrite never leaves an inactive + // column holding a symbol every other reference to it has replaced. + for item in call.filter_params.iter_mut() { + visitor.filter_params_group(std::slice::from_mut(item))?; + } + for group in call.filter_groups.iter_mut() { + visitor.filter_params_group(&mut group.filter_params)?; + } + for item in call.filter_params_mut() { + if let Some(call) = &mut item.compiled_call { + call.visit_deps_mut(visitor)?; + } + } *self = Rc::new(call); Ok(()) } @@ -613,6 +698,7 @@ impl crate::utils::debug::DebugSql for SqlCall { &filter_params, &filter_groups, &context_values, + &[], ) .unwrap() } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call_builder.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call_builder.rs index 67fe0133df560..80028ec328784 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call_builder.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call_builder.rs @@ -19,6 +19,11 @@ pub struct SqlCallBuilder<'a> { cube_evaluator: Rc, base_tools: Rc, security_context: Rc, + /// Set while compiling a cube's own `sql`. That sql builds the table the + /// query reads from, so no member is in scope inside it — a member + /// reference there is rejected instead of resolved, wherever in the sql or + /// in one of its `FILTER_PARAMS` columns it appears. + is_cube_sql: bool, } impl<'a> SqlCallBuilder<'a> { @@ -33,9 +38,15 @@ impl<'a> SqlCallBuilder<'a> { cube_evaluator, base_tools, security_context, + is_cube_sql: false, } } + pub fn for_cube_sql(mut self) -> Self { + self.is_cube_sql = true; + self + } + pub fn build( mut self, cube_name: &String, @@ -46,54 +57,80 @@ impl<'a> SqlCallBuilder<'a> { self.security_context.clone(), member_sql.args_names().clone(), )?; - let (template, template_args) = (compiled.template, compiled.args); + self.build_from_template(cube_name, compiled.template, &compiled.args) + } - let deps = template_args + // Assembles a `SqlCall` from an already-compiled template and the + // dependencies it recorded. Recurses for a `FILTER_PARAMS` column that came + // back compiled, since such a column is a call of its own with its own + // dependency list. + fn build_from_template( + &mut self, + cube_name: &String, + template: SqlTemplate, + args: &SqlTemplateArgs, + ) -> Result { + let deps = args .symbol_paths .iter() .map(|path| self.build_dependency(cube_name, path)) .collect::, _>>()?; - let filter_params = template_args + let filter_params = args .filter_params .iter() - .map(|itm| self.build_filter_params_item(itm)) + .map(|itm| self.build_filter_params_item(cube_name, itm)) .collect::, _>>()?; - let filter_groups = template_args + let filter_groups = args .filter_groups .iter() - .map(|itm| self.build_filter_group_item(itm)) + .map(|itm| self.build_filter_group_item(cube_name, itm)) .collect::, _>>()?; - let result = SqlCall::new( - template.clone(), + Ok(SqlCall::new( + template, deps, filter_params, filter_groups, - template_args.security_context.clone(), - ); - Ok(result) + args.security_context.clone(), + )) } fn build_filter_params_item( &mut self, + cube_name: &String, item: &FilterParamsItem, ) -> Result { + let compiled_call = match &item.column { + FilterParamsColumn::Compiled(compiled) => Some(Rc::new(self.build_from_template( + cube_name, + compiled.template.clone(), + &compiled.args, + )?)), + _ => None, + }; + Ok(SqlCallFilterParamsItem { filter_symbol_name: format!("{}.{}", item.cube_name, item.name), column: item.column.clone(), + compiled_call, + // Turned on per query, and again per subquery, once the filters are + // known. Off until then, so a symbol compiled for comparison rather + // than for a query carries no dependency the query would not. + active: false, }) } fn build_filter_group_item( &mut self, + cube_name: &String, item: &FilterGroupItem, ) -> Result { let filter_params = item .filter_params .iter() - .map(|itm| self.build_filter_params_item(itm)) + .map(|itm| self.build_filter_params_item(cube_name, itm)) .collect::, _>>()?; Ok(SqlCallFilterGroupItem { filter_params }) } @@ -112,6 +149,20 @@ impl<'a> SqlCallBuilder<'a> { ) .map_err(|e| CubeError::user(format!("Error in `{}`: {}", dep_path.join("."), e)))?; + if self.is_cube_sql { + if let SymbolPathType::Dimension | SymbolPathType::Measure | SymbolPathType::Segment = + symbol_path.path_type() + { + return Err(CubeError::user(format!( + "`sql` of cube `{}` references member `{}`. A cube's sql builds the table the \ + query reads from, so no member is in scope there — reference the underlying \ + column instead", + current_cube_name, + symbol_path.full_name() + ))); + } + } + let path = symbol_path.path().clone(); match symbol_path.path_type() { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/cube_symbol.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/cube_symbol.rs index a47a0926582aa..37ed3f9cee3a7 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/cube_symbol.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/cube_symbol.rs @@ -194,7 +194,7 @@ impl CubeTableSymbolFactory { is_table_sql, } = self; let sql = if let Some(sql) = sql { - Some(compiler.compile_sql_call(&cube_name, sql)?) + Some(compiler.compile_cube_sql_call(&cube_name, sql)?) } else { None }; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/deps.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/deps.rs index 747da036d8111..b35371a20491d 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/deps.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/deps.rs @@ -38,6 +38,17 @@ pub trait DepVisitor { /// member-symbol slot and may replace it. pub trait DepVisitorMut { fn symbol(&mut self, slot: &mut Rc) -> Result<(), CubeError>; + + /// The `FILTER_PARAMS` bindings reached on the way, grouped the way + /// they are rendered: a `FILTER_GROUP` arrives as one slice, a + /// standalone binding as a slice of one. Bindings that contribute no + /// dependencies are handed over all the same. + fn filter_params_group( + &mut self, + _items: &mut [crate::planner::SqlCallFilterParamsItem], + ) -> Result<(), CubeError> { + Ok(()) + } } /// A node whose dependency slots can be walked (read) or rebuilt diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/mod.rs index 8781d1f336db5..0f4278e1fea0a 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/mod.rs @@ -120,6 +120,14 @@ impl MeasureKind { } } + /// True if a rollup stores this kind as a mergeable sketch rather than the + /// final value — either it already is that state form, or it is the kind + /// that has one. Derived from [`Self::as_state`] so both answers cannot + /// drift apart. + pub fn is_stored_as_state(&self) -> bool { + matches!(self, Self::AggregatedState(_)) || self.as_state().is_some() + } + pub fn measure_type_str(&self) -> &str { match self { Self::Count(_) | Self::MultipliedCount(_) => "count", diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/transforms/static_filter.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/transforms/static_filter.rs index 7cd890964e51b..662ef77e3eb33 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/transforms/static_filter.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/transforms/static_filter.rs @@ -2,6 +2,8 @@ use super::super::common::Case; use super::super::dimension_kinds::DimensionKind; use super::super::{DimensionSymbol, MeasureSymbol, MemberSymbol}; use crate::planner::filter::{Filter, FilterGroup, FilterGroupOperator, FilterItem}; +use crate::planner::symbols::deps::{DepVisitorMut, SymbolDeps}; +use crate::planner::SqlCallFilterParamsItem; use cubenativeutils::CubeError; use std::rc::Rc; @@ -93,3 +95,67 @@ fn replace_dimension_case(dimension: &DimensionSymbol, new_case: Case) -> Rc, + filters: &FilterItem, +) -> Result, CubeError> { + let mut visitor = ActivityVisitor { filters }; + let mut result = symbol.as_ref().clone(); + result.visit_deps_mut(&mut visitor)?; + Ok(Rc::new(result)) +} + +/// The filter set as the one item `apply_filter_params_activity_to_symbol` +/// matches against. +pub fn filter_params_activity_filters(filters: &[FilterItem]) -> FilterItem { + FilterItem::Group(Rc::new(FilterGroup { + operator: FilterGroupOperator::And, + items: filters.to_vec(), + })) +} + +struct ActivityVisitor<'a> { + filters: &'a FilterItem, +} + +impl DepVisitorMut for ActivityVisitor<'_> { + fn symbol(&mut self, slot: &mut Rc) -> Result<(), CubeError> { + let mut symbol = slot.as_ref().clone(); + symbol.visit_deps_mut(self)?; + *slot = Rc::new(symbol); + Ok(()) + } + + fn filter_params_group( + &mut self, + items: &mut [SqlCallFilterParamsItem], + ) -> Result<(), CubeError> { + // Matched against the whole group, since that is the predicate the group + // renders: an OR group survives only when every member of it matches. + let members = items + .iter() + .map(|item| &item.filter_symbol_name) + .collect::>(); + let active = self.filters.find_subtree_for_members(&members).is_some(); + for item in items.iter_mut() { + item.active = active; + } + Ok(()) + } +} + +/// `apply_filter_params_activity_to_symbol` over every symbol a filter item +/// carries. +pub fn apply_filter_params_activity_to_filter_item( + filter_item: &FilterItem, + filters: &FilterItem, +) -> Result { + super::map_filter_item_symbols(filter_item, &|symbol| { + apply_filter_params_activity_to_symbol(symbol, filters) + }) +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs index d5718c2295ff5..71c3c97dbd93c 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/visitor_context.rs @@ -1,10 +1,10 @@ -use crate::cube_bridge::member_sql::FilterParamsColumn; +use crate::planner::sql_call::SqlCallFilterParamsItem; use std::collections::HashMap; #[derive(Default)] pub struct FiltersContext { pub use_local_tz: bool, - pub filter_params_columns: HashMap, + pub filter_params_columns: HashMap, /// True when members resolve to pre-aggregation columns (a rollup read). A /// segment is then a stored boolean column, which some dialects can't use /// as a bare predicate (e.g. MSSQL `BIT` needs `= 1`). diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_filter_params_callback.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_filter_params_callback.rs new file mode 100644 index 0000000000000..82d4c36380a4a --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_filter_params_callback.rs @@ -0,0 +1,62 @@ +use crate::cube_bridge::filter_params_callback::FilterParamsCallback; +use cubenativeutils::wrappers::NativeContextHolderRef; +use cubenativeutils::CubeError; +use std::any::Any; +use std::rc::Rc; + +/// Mock `FILTER_PARAMS.….filter(callback)` column: holds the SQL the data +/// model's callback would produce, with `%N` marking the Nth filter value the +/// planner passes in at render time. Member references are already rendered as +/// `{arg:N}` placeholders by `MockMemberSql`, indexing the same dependency list +/// as the surrounding template. +pub struct MockFilterParamsCallback { + template: String, +} + +impl MockFilterParamsCallback { + pub fn new(template: impl Into) -> Self { + Self { + template: template.into(), + } + } +} + +impl FilterParamsCallback for MockFilterParamsCallback { + fn call(&self, filter_params: &Vec) -> Result { + let mut result = self.template.clone(); + // Highest index first: `%1` is a prefix of `%10`, so substituting it + // first would corrupt the two-digit slot. + for (i, param) in filter_params.iter().enumerate().rev() { + result = result.replace(&format!("%{}", i), param); + } + Ok(result) + } + + fn as_any(self: Rc) -> Rc { + self + } + + fn clone_to_context( + &self, + _context_ref: &dyn NativeContextHolderRef, + ) -> Result, CubeError> { + Ok(Rc::new(Self { + template: self.template.clone(), + })) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn substitutes_filter_values_by_index() { + let cb = MockFilterParamsCallback::new("{arg:0} >= %0 AND {arg:0} < %1"); + + assert_eq!( + cb.call(&vec!["$1".to_string(), "$2".to_string()]).unwrap(), + "{arg:0} >= $1 AND {arg:0} < $2" + ); + } +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_member_sql.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_member_sql.rs index 8afd5dd575806..6aa0d1836aae5 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_member_sql.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mock_member_sql.rs @@ -1,6 +1,8 @@ use crate::cube_bridge::member_sql::{ - CompiledMemberTemplate, MemberSql, SqlTemplate, SqlTemplateArgs, + CompiledMemberTemplate, FilterParamsColumn, FilterParamsItem, MemberSql, SqlTemplate, + SqlTemplateArgs, }; +use crate::test_fixtures::cube_bridge::MockFilterParamsCallback; use cubenativeutils::CubeError; use std::any::Any; use std::rc::Rc; @@ -148,6 +150,47 @@ impl MockMemberSql { continue; } + // `{FILTER_PARAMS:.:}` records a FILTER_PARAMS + // binding with a callback column and yields `{fp:N}`. Inside + // ``, `[path]` is a member reference — recorded into this + // template's own args, so the callback's output indexes the same + // dependency list — and `%N` stands for the Nth filter value the + // planner passes at render time. + if let Some(body) = path.strip_prefix("FILTER_PARAMS:") { + let (member, column) = body.split_once(':').ok_or_else(|| { + CubeError::user(format!( + "FILTER_PARAMS needs a `.:` body: {}", + body + )) + })?; + // The scanner above stops at the first `}`, so a column carrying + // one would have been cut short here. + if column.is_empty() || column.contains('{') { + return Err(CubeError::user(format!( + "FILTER_PARAMS column must be non-empty and reference members as `[path]`: {}", + column + ))); + } + let member_parts = member.split('.').collect::>(); + if member_parts.len() != 2 || member_parts.iter().any(|p| p.is_empty()) { + return Err(CubeError::user(format!( + "FILTER_PARAMS member must be `.`: {}", + member + ))); + } + let (cube_name, name) = (member_parts[0], member_parts[1]); + let column = Self::parse_column_references(column, &mut args, &mut args_names)?; + let index = args.insert_filter_params(FilterParamsItem { + cube_name: cube_name.to_string(), + name: name.to_string(), + column: FilterParamsColumn::Callback(Rc::new( + MockFilterParamsCallback::new(column), + )), + }); + result.push_str(&format!("{{fp:{}}}", index)); + continue; + } + // Parse the path and add to symbol_paths let path_parts: Vec = path.split('.').map(|s| s.to_string()).collect(); @@ -181,6 +224,46 @@ impl MockMemberSql { Ok((result, args, args_names)) } + + // Replaces every `[path.to.member]` in a callback column with the `{arg:N}` + // placeholder of its recorded path. + fn parse_column_references( + column: &str, + args: &mut SqlTemplateArgs, + args_names: &mut Vec, + ) -> Result { + let mut result = String::new(); + let mut rest = column; + + while let Some(open) = rest.find('[') { + result.push_str(&rest[..open]); + let after = &rest[open + 1..]; + let close = after.find(']').ok_or_else(|| { + CubeError::user(format!("Unclosed member reference in column: {}", column)) + })?; + + let path_parts: Vec = + after[..close].split('.').map(|s| s.to_string()).collect(); + if path_parts.iter().any(|p| p.is_empty()) { + return Err(CubeError::user(format!( + "Invalid member reference in column: {}", + column + ))); + } + + let arg_name = path_parts[0].clone(); + if !args_names.contains(&arg_name) { + args_names.push(arg_name); + } + let index = args.insert_symbol_path(path_parts); + result.push_str(&format!("{{arg:{}}}", index)); + + rest = &after[close + 1..]; + } + result.push_str(rest); + + Ok(result) + } } impl MockMemberSql { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mod.rs index fae44573b99ef..de1fc6018a932 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/cube_bridge/mod.rs @@ -16,6 +16,7 @@ mod mock_dimension_definition; mod mock_driver_tools; mod mock_evaluator; mod mock_expression_struct; +mod mock_filter_params_callback; mod mock_geo_item; mod mock_granularity_definition; mod mock_join_definition; @@ -54,6 +55,7 @@ pub use mock_dimension_definition::MockDimensionDefinition; pub use mock_driver_tools::MockDriverTools; pub use mock_evaluator::MockCubeEvaluator; pub use mock_expression_struct::MockExpressionStruct; +pub use mock_filter_params_callback::MockFilterParamsCallback; pub use mock_geo_item::MockGeoItem; pub use mock_granularity_definition::MockGranularityDefinition; pub use mock_join_definition::MockJoinDefinition; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml index 0dad048dda9c7..cfae8627de248 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml @@ -125,6 +125,12 @@ cubes: else: sql: "{CUBE.status}" + # Every books row is NULL here, so a query grouping by it carries a + # NULL dimension key through the multi-stage assembly. + - name: category_nullable + type: string + sql: "NULLIF(category, 'books')" + - name: customer_name type: string sql: "{customers.name}" @@ -636,6 +642,91 @@ cubes: keep_only: - orders.status + # Both directives name `status`: the value ignores the query filter + # on it (`filter`) and collapses its partition (`grain`). The result + # row set must still be the query's. + - name: amount_grain_and_filter_exclude_status + type: sum + sql: "{CUBE.total_amount}" + multi_stage: true + grain: + exclude: + - orders.status + filter: + exclude: + - orders.status + + # Share-of-total shape: a grand-total denominator (grain drops every + # dimension) that also ignores the query filter on `status`, consumed + # by a ratio that coalesces a missing denominator to 0 — so a widened + # row set surfaces as extra rows reading 0 rather than NULL. + - name: amount_grand_total_unfiltered + type: sum + sql: "{CUBE.total_amount}" + multi_stage: true + grain: + exclude: + - orders.status + - orders.category + filter: + exclude: + - orders.status + + - name: amount_share_percent + type: number + multi_stage: true + sql: "coalesce(100 * {CUBE.total_amount} / nullif({CUBE.amount_grand_total_unfiltered}, 0), 0)" + + # A rolling window rewrites the query date range for its base state + # (extending it backwards by the trailing interval). The inner measure + # below drops that date filter, so detecting the drop must not depend + # on the filter still carrying the query's own bounds. + - name: amount_no_date_bound + type: sum + sql: "{CUBE.total_amount}" + multi_stage: true + filter: + exclude: + - orders.created_at + + - name: rolling_amount_no_date_bound + type: sum + sql: "{CUBE.amount_no_date_bound}" + multi_stage: true + rolling_window: + trailing: 3 month + + # The `include` predicate matches the dropped query filter on + # operator and values while naming a different member, so the CTE + # dedup key has to compare members to keep the two states apart. + - name: amount_exclude_status_include_lookalike + type: sum + sql: "{CUBE.total_amount}" + multi_stage: true + filter: + exclude: + - orders.status + include: + - member: orders.category + operator: notEquals + values: [pending] + + # A rank whose ordering ignores the query filter on `status` and whose + # partition drops it. Rank is deliberately left out of the keys side, + # so the ranks span the whole universe and the rows do too. + - name: category_rank_all_statuses + type: rank + multi_stage: true + grain: + exclude: + - orders.status + filter: + exclude: + - orders.status + order_by: + - sql: "{CUBE.total_amount}" + dir: desc + - name: amount_only_completed type: sum sql: "{CUBE.total_amount}" diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/pre_aggregation_matching_test.yaml b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/pre_aggregation_matching_test.yaml index 6d6ecdf05fd5f..a5f9ea15659f9 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/pre_aggregation_matching_test.yaml +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/pre_aggregation_matching_test.yaml @@ -170,3 +170,14 @@ cubes: - high_priority time_dimension: created_at granularity: day + + # Groups by the primary key, so every stored row is a single order — + # the only shape a rollup may serve an ungrouped query from. + - name: primary_key_rollup + type: rollup + measures: + - total_amount + dimensions: + - id + - status + - city diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/pre_aggregations_test.yaml b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/pre_aggregations_test.yaml index 9834011ad24c9..578e2299cc9a9 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/pre_aggregations_test.yaml +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/pre_aggregations_test.yaml @@ -126,6 +126,18 @@ cubes: - visitor_checkins.for_lambda - visitor_checkins2.for_lambda + # Same cross-cube shape as joined_rollup, but grouped by the primary keys + # of both cubes, so each stored row is one raw joined row + - name: joined_keys_rollup + type: rollup + measures: + - count + dimensions: + - id + - visitor_id + - visitors.id + - visitors.source + # Second cube for rollupLambda example - name: visitor_checkins2 sql: "SELECT * FROM visitor_checkins" @@ -154,3 +166,20 @@ cubes: time_dimension: created_at granularity: day partition_granularity: day + +views: + # Renames every member (prefix), so view members are references whose names + # differ from the cube members the pre-aggregations are declared over. + - name: visitors_view + cubes: + - join_path: visitors + includes: + - id + - source + prefix: true + - join_path: visitors.visitor_checkins + includes: + - id + - visitor_id + - count + prefix: true diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/ungrouped_multiplied_dup.yaml b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/ungrouped_multiplied_dup.yaml new file mode 100644 index 0000000000000..771cdb7d2647d --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/ungrouped_multiplied_dup.yaml @@ -0,0 +1,38 @@ +# Two customers share the same name, each with one order of the same status, so +# grouping by the requested dimensions would collapse two distinct primary keys +# into one row. +cubes: + - name: dup_customers + sql: "SELECT * FROM dup_customers" + joins: + - name: dup_orders + relationship: one_to_many + sql: "{CUBE.id} = {dup_orders.customer_id}" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: name + type: string + sql: name + measures: + # The aggregate lives inside the measure's own sql, so nothing may wrap it + # and the select it lands in has to group. + - name: ltv_agg_number + type: number + sql: "sum(ltv)" + + - name: dup_orders + sql: "SELECT * FROM dup_orders" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: customer_id + type: number + sql: customer_id + - name: status + type: string + sql: status diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/ungrouped_pre_agg_gate.yaml b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/ungrouped_pre_agg_gate.yaml new file mode 100644 index 0000000000000..1051003c62a71 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/ungrouped_pre_agg_gate.yaml @@ -0,0 +1,493 @@ +# Models for the ungrouped pre-aggregation eligibility rule. Each cube group is +# self-contained so a query only ever pulls in the cubes it names. +cubes: + # No primary key at all. `collapsing` groups by non-key columns only. + - name: no_pk_orders + sql: "SELECT * FROM orders" + dimensions: + - name: status + type: string + sql: status + - name: city + type: string + sql: city + measures: + - name: total + type: sum + sql: amount + pre_aggregations: + - name: collapsing + type: rollup + measures: + - total + dimensions: + - status + - city + + # Single cube that does declare a key, with a rollup that omits it. + - name: sc_orders + sql: "SELECT * FROM orders" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: status + type: string + sql: status + - name: city + type: string + sql: city + measures: + - name: total + type: sum + sql: amount + pre_aggregations: + - name: sc_coarse + type: rollup + measures: + - total + dimensions: + - status + - city + + # visitors -> checkins -> cities. `checkins` fans out but is only a transit + # hop, so a query naming just visitors and cities members still multiplies. + - name: chain_visitors + sql: "SELECT * FROM visitors" + joins: + - name: chain_checkins + relationship: one_to_many + sql: "{CUBE.id} = {chain_checkins.visitor_id}" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: source + type: string + sql: source + pre_aggregations: + - name: chain_rollup + type: rollup + dimensions: + - id + - source + - chain_cities.id + - chain_cities.name + + # Keyed on the root only, with a segment reaching across the fan-out. + - name: root_key_seg_rollup + type: rollup + dimensions: + - id + - source + segments: + - chain_cities.named_x + + - name: chain_checkins + sql: "SELECT * FROM visitor_checkins" + joins: + - name: chain_cities + relationship: many_to_one + sql: "{CUBE.city_id} = {chain_cities.id}" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: visitor_id + type: number + sql: visitor_id + - name: city_id + type: number + sql: city_id + + - name: chain_cities + sql: "SELECT * FROM cities" + segments: + - name: named_x + sql: "{CUBE}.name = 'X'" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: name + type: string + sql: name + + # Canonical star: fact many_to_one dimension cube, so the join cannot + # multiply rows and the fact key alone identifies every stored row. + - name: star_checkins + sql: "SELECT * FROM visitor_checkins" + joins: + - name: star_visitors + relationship: many_to_one + sql: "{CUBE.visitor_id} = {star_visitors.id}" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: visitor_id + type: number + sql: visitor_id + # Proxy for this cube's own primary key, which row identity does require. + - name: own_ref + type: number + sql: "{id}" + - name: created_at + type: time + sql: created_at + measures: + - name: cnt + type: count + pre_aggregations: + # Keyed on the primary key, so 1:1 with raw rows, but it stores the time + # dimension truncated to the day. + - name: star_day_rollup + type: rollup + measures: + - cnt + dimensions: + - id + time_dimension: created_at + granularity: day + + - name: star_rollup + type: rollup + measures: + - cnt + dimensions: + - id + - star_visitors.source + + # Stores the fact key only through a proxy dimension. + - name: proxy_rollup + type: rollup + measures: + - cnt + dimensions: + - own_ref + - star_visitors.source + + # 1:1 with raw rows: the segment groups the stored rows, but it reaches + # star_visitors over a many_to_one edge that cannot split a fact row. + - name: seg_rollup + type: rollup + measures: + - cnt + dimensions: + - id + - visitor_id + segments: + - star_visitors.source_google + + - name: star_visitors + sql: "SELECT * FROM visitors" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: source + type: string + sql: source + segments: + - name: source_google + sql: "{CUBE}.source = 'google'" + + # Pure many_to_one chain: the middle cube only bridges, so it cannot split a + # row and its key is not part of row identity. + - name: bridge_orders + sql: "SELECT * FROM orders" + joins: + - name: bridge_stores + relationship: many_to_one + sql: "{CUBE.store_id} = {bridge_stores.id}" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: store_id + type: number + sql: store_id + measures: + - name: cnt + type: count + pre_aggregations: + - name: bridge_rollup + type: rollup + measures: + - cnt + dimensions: + - id + - bridge_regions.name + + - name: bridge_stores + sql: "SELECT * FROM stores" + joins: + - name: bridge_regions + relationship: many_to_one + sql: "{CUBE.region_id} = {bridge_regions.id}" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: region_id + type: number + sql: region_id + + - name: bridge_regions + sql: "SELECT * FROM regions" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: name + type: string + sql: name + + # Non-additive stored state: a rollup row holds an HLL sketch, not a number. + - name: hll_orders + sql: "SELECT * FROM orders" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: status + type: string + sql: status + - name: user_id + type: number + sql: user_id + measures: + - name: uniq + type: count_distinct_approx + sql: user_id + pre_aggregations: + - name: hll_pk_rollup + type: rollup + measures: + - uniq + dimensions: + - id + - status + + # Fan-out measure: forces the multiplied-subquery planning path. + - name: mult_customers + sql: "SELECT * FROM customers" + joins: + - name: mult_orders + relationship: one_to_many + sql: "{CUBE.id} = {mult_orders.customer_id}" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: name + type: string + sql: name + measures: + - name: count + type: count + # Joins mult_orders inside its own sql; the rollup aggregates that away. + - name: orders_total + type: sum + sql: "{mult_orders.cnt}" + pre_aggregations: + # Grouped by the primary key, and carrying a measure whose sql reaches + # another cube without that cube touching the stored grain. + - name: mult_keyed + type: rollup + measures: + - orders_total + dimensions: + - id + - name + + - name: mult_collapsing + type: rollup + measures: + - count + dimensions: + - name + - mult_orders.status + + - name: mult_orders + sql: "SELECT * FROM orders" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: customer_id + type: number + sql: customer_id + - name: status + type: string + sql: status + measures: + - name: cnt + type: count + + # Non-additive rolling measure: its leaf stage is planned ungrouped for + # rendering reasons even though the user query is grouped. + - name: roll_orders + sql: "SELECT * FROM orders" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: category + type: string + sql: category + - name: created_at + type: time + sql: created_at + measures: + - name: rolling_avg_7d + type: avg + sql: amount + rolling_window: + trailing: 7 day + pre_aggregations: + - name: rolling_avg_rollup + type: rollup + measures: + - rolling_avg_7d + dimensions: + - category + time_dimension: created_at + granularity: day + + # rollupLambda union where the second branch groups by a non-unique column + # that merely shares the short name `id` with the first branch's key. + - name: lam_a + sql: "SELECT * FROM visitor_checkins" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: visitor_id + type: number + sql: visitor_id + - name: created_at + type: time + sql: created_at + measures: + - name: count + type: count + pre_aggregations: + - name: lam_base + type: rollup + measures: + - count + dimensions: + - id + - visitor_id + time_dimension: created_at + granularity: day + partition_granularity: day + + - name: lam_union + type: rollupLambda + rollups: + - lam_a.lam_base + - lam_b.lam_base + + - name: lam_b + sql: "SELECT * FROM visitor_checkins" + dimensions: + - name: row_id + type: number + sql: row_id + primary_key: true + # Shares the short name with lam_a's key but is not unique. + - name: id + type: number + sql: visitor_id + - name: visitor_id + type: number + sql: visitor_id + - name: created_at + type: time + sql: created_at + measures: + - name: count + type: count + pre_aggregations: + - name: lam_base + type: rollup + measures: + - count + dimensions: + - id + - visitor_id + time_dimension: created_at + granularity: day + partition_granularity: day + + # rollupJoin advertising a primary key that neither member rollup stores. + - name: rj_checkins + sql: "SELECT * FROM visitor_checkins" + joins: + - name: rj_visitors + relationship: many_to_one + sql: "{CUBE.visitor_id} = {rj_visitors.id}" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: visitor_id + type: number + sql: visitor_id + measures: + - name: count + type: count + pre_aggregations: + - name: rj_base + type: rollup + measures: + - count + dimensions: + - visitor_id + + - name: rj_keys_join + type: rollupJoin + measures: + - count + dimensions: + - id + - visitor_id + - rj_visitors.id + - rj_visitors.source + rollups: + - rj_checkins.rj_base + - rj_visitors.rj_base + + - name: rj_visitors + sql: "SELECT * FROM visitors" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: source + type: string + sql: source + pre_aggregations: + - name: rj_base + type: rollup + dimensions: + - id + - source diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/seeds/ungrouped_multiplied_dup_tables.sql b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/seeds/ungrouped_multiplied_dup_tables.sql new file mode 100644 index 0000000000000..14761d0e97ac0 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/seeds/ungrouped_multiplied_dup_tables.sql @@ -0,0 +1,23 @@ +DROP TABLE IF EXISTS dup_orders CASCADE; +DROP TABLE IF EXISTS dup_customers CASCADE; + +CREATE TABLE dup_customers ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL, + ltv NUMERIC(10, 2) NOT NULL +); + +-- Same name, different keys and different ltv. +INSERT INTO dup_customers (id, name, ltv) VALUES + (1, 'dup', 100), + (2, 'dup', 50); + +CREATE TABLE dup_orders ( + id INTEGER PRIMARY KEY, + customer_id INTEGER NOT NULL, + status TEXT NOT NULL +); + +INSERT INTO dup_orders (id, customer_id, status) VALUES + (10, 1, 'new'), + (11, 2, 'new'); diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_callback_column.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_callback_column.rs new file mode 100644 index 0000000000000..1e92c37d5ddea --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/filter_params_callback_column.rs @@ -0,0 +1,113 @@ +use crate::test_fixtures::cube_bridge::MockSchema; +use crate::test_fixtures::test_utils::TestContext; +use indoc::indoc; + +// The measure filter's only content is a `FILTER_PARAMS` binding whose column is +// a callback, and the time dimension is referenced from inside that callback +// alone — nothing else in the filter's SQL mentions it. +fn schema() -> MockSchema { + MockSchema::from_yaml(indoc! {" + cubes: + - name: commission + sql: \"SELECT * FROM commission\" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: partner + type: string + sql: partner + - name: pricing_duration + type: string + sql: pricing_duration + - name: reconciliation_date + type: time + sql: reconciliation_date + measures: + - name: daily_mrr + type: sum + sql: \"{CUBE}.total\" + filters: + - sql: \"{CUBE.pricing_duration} = 'DAILY'\" + - sql: \"{FILTER_PARAMS:commission.reconciliation_date:[CUBE.reconciliation_date] >= %0 AND [CUBE.reconciliation_date] < %1}\" + "}) + .unwrap() +} + +const QUERY: &str = indoc! {" + measures: + - commission.daily_mrr + dimensions: + - commission.partner + time_dimensions: + - dimension: commission.reconciliation_date + granularity: month + dateRange: + - \"2025-07-01\" + - \"2026-06-30\" +"}; + +#[test] +fn measure_filter_callback_column_renders_the_referenced_member() { + let ctx = TestContext::new(schema()).unwrap(); + + let (sql, _) = ctx.build_sql_and_params(QUERY).unwrap(); + + // Both halves of the callback's own predicate, with its member reference + // resolved to the dimension's SQL. The query's own time filter renders `<=` + // against the same column, so the strict `<` is what pins this to the + // callback. + assert!( + sql.contains( + "\"commission\".reconciliation_date >= $1 AND \"commission\".reconciliation_date < $2" + ), + "the filter param predicate must carry the referenced column\nsql: {}", + sql + ); + assert!( + !sql.contains("{arg:"), + "no dependency placeholder may survive into the SQL\nsql: {}", + sql + ); +} + +// A cube's `sql` builds the table the query reads from, so no member is in scope +// inside it. The reference is reported without resolving it, which is also what +// keeps the cube table and the dimension from resolving each other in a loop. +#[test] +fn cube_sql_referencing_a_member_is_rejected() { + let schema = MockSchema::from_yaml(indoc! {" + cubes: + - name: commission + sql: \"SELECT * FROM commission WHERE {CUBE.reconciliation_date} IS NOT NULL\" + dimensions: + - name: id + type: number + sql: id + primary_key: true + - name: reconciliation_date + type: time + sql: reconciliation_date + measures: + - name: count + type: count + "}) + .unwrap(); + + let err = TestContext::new(schema) + .and_then(|ctx| { + ctx.build_sql_and_params(indoc! {" + measures: + - commission.count + "}) + }) + .expect_err("a member reference in a cube's sql must be reported"); + + assert!( + err.message + .contains("references member `commission.reconciliation_date`"), + "unexpected error: {}", + err.message + ); +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/modifiers.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/modifiers.rs index a790679d4c1f1..78c582e486a90 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/modifiers.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/modifiers.rs @@ -15,6 +15,39 @@ fn create_multi_fact_context() -> TestContext { const BASIC_SEED: &str = "integration_basic_tables.sql"; const MULTI_FACT_SEED: &str = "integration_multi_fact_tables.sql"; +/// Body of the first ` AS ( ... )` block, in text order, whose text +/// contains `marker`, so an assertion can be aimed at one CTE instead of the +/// whole statement. `marker` must be lower-case. +fn cte_body_containing(sql: &str, marker: &str) -> Option { + let lower = sql.to_lowercase(); + let mut from = 0; + while let Some(rel) = lower[from..].find(" as (") { + let open = from + rel + " as (".len() - 1; + let mut depth = 0; + let mut close = None; + for (offset, ch) in sql[open..].char_indices() { + match ch { + '(' => depth += 1, + ')' => { + depth -= 1; + if depth == 0 { + close = Some(open + offset + 1); + break; + } + } + _ => {} + } + } + let close = close?; + let body = &sql[open..close]; + if body.to_lowercase().contains(marker) { + return Some(body.to_string()); + } + from = open + 1; + } + None +} + // 9.1: ORDER BY dimension ASC, measure DESC #[tokio::test(flavor = "multi_thread")] async fn test_order_by_dimension_and_measure() { @@ -279,3 +312,76 @@ async fn test_ungrouped_with_time_dimension() { insta::assert_snapshot!(result); } } + +const UNGROUPED_DUP_SEED: &str = "ungrouped_multiplied_dup_tables.sql"; + +// A measure whose own sql already aggregates cannot be projected row-level: the +// select it lands in must group, or the SQL is not valid at all. This holds for +// an ungrouped request too, so the multiplied CTE keeps its GROUP BY here. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_multiplied_keeps_group_by_for_aggregate_sql_measure() { + let schema = MockSchema::from_yaml_file("common/ungrouped_multiplied_dup.yaml"); + let ctx = TestContext::new(schema).unwrap(); + + let query = indoc! {" + measures: + - dup_customers.ltv_agg_number + dimensions: + - dup_customers.name + - dup_orders.status + ungrouped: true + "}; + + let sql = ctx.build_sql(query).unwrap(); + let lower = sql.to_lowercase(); + + assert!( + lower.contains("select distinct"), + "expected the keys subquery of a multiplied measure, got:\n{sql}" + ); + assert!( + lower.contains("group by"), + "a measure that aggregates in its own sql needs the enclosing select to \ + group, got:\n{sql}" + ); + + // Executing is the real assertion: without the grouping the engine rejects + // the statement outright. + if let Some(result) = ctx.try_execute_pg(query, UNGROUPED_DUP_SEED).await { + insta::assert_snapshot!(result); + } +} + +// A CTE hoisted out of a multi-stage leaf is consumed by the stage above it, +// which re-aggregates its rows. It projects row-level values for that reason, so +// it has to keep grouping to the leaf's own grain no matter what the request +// asked for — an ungrouped request does not make this CTE a raw-row scan. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_request_keeps_group_by_in_hoisted_multi_stage_leaf() { + let schema = + MockSchema::from_yaml_file("common/integration_multi_stage_multiplied_pre_agg.yaml") + .only_pre_aggregations(&[]); + let ctx = TestContext::new(schema).unwrap(); + + let query = indoc! {" + measures: + - customers.total_lifetime_value_prev_month_by_returns + time_dimensions: + - dimension: returns.created_at + granularity: month + ungrouped: true + "}; + + let sql = ctx.build_sql(query).unwrap(); + + // The hoisted leaf is the CTE holding the multiplied-measure keys subquery. + let leaf = cte_body_containing(&sql, "as \"keys\"").unwrap_or_else(|| { + panic!("expected a hoisted keys subquery, got:\n{sql}"); + }); + + assert!( + leaf.to_lowercase().contains("group by"), + "the hoisted leaf feeds a stage that re-aggregates it, so it must stay \ + grouped, got:\n{leaf}" + ); +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs index 1ea67ebdec057..2e5859b51b444 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs @@ -245,3 +245,294 @@ async fn test_mode_fixed_in_chain_diverges_from_relative() { insta::assert_snapshot!(result); } } + +// `grain.exclude` and `filter.exclude` naming the same dimension, which the +// query both groups by and filters on. The value ignores the filter and the +// partition; the row set stays the query's — the statuses the query filtered +// out may not reappear. +#[tokio::test(flavor = "multi_thread")] +async fn test_grain_and_filter_exclude_keeps_query_row_set() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.total_amount + - orders.amount_grain_and_filter_exclude_status + dimensions: + - orders.status + - orders.category + filters: + - dimension: orders.status + operator: equals + values: + - completed + order: + - id: orders.status + - id: orders.category + "#}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +// Share-of-total over a grand-total denominator that ignores the query filter. +// The denominator's own aggregation must span every status while the ratio is +// reported only for the rows the query asked for; a widened row set would show +// up as extra rows whose share coalesces to 0. +#[tokio::test(flavor = "multi_thread")] +async fn test_share_of_grand_total_keeps_query_row_set() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.amount_share_percent + dimensions: + - orders.status + - orders.category + filters: + - dimension: orders.status + operator: equals + values: + - completed + order: + - id: orders.status + - id: orders.category + "#}; + + let sql = ctx.build_sql(query).unwrap(); + // The window text comes from a `format!` in the physical plan, not from a + // dialect template, so `OVER (` is stable; whitespace is stripped only so + // the check does not depend on how the expression is laid out. + let dense: String = sql.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + !dense.contains("OVER("), + "the denominator has to hold the query's rows through a keys side, \ + which a window expression cannot do:\n{}", + sql + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +// A NULL dimension value is a grid key like any other: the keys side and the +// measure side must still meet on it. +#[tokio::test(flavor = "multi_thread")] +async fn test_grain_exclude_keeps_null_dimension_key() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.total_amount + - orders.amount_grain_and_filter_exclude_status + dimensions: + - orders.status + - orders.category_nullable + filters: + - dimension: orders.status + operator: equals + values: + - completed + order: + - id: orders.status + - id: orders.category_nullable + "#}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +// The dropped filter's dimension is not one the query groups by, so the grain +// reshape has nothing to remove and the measure side keeps the full grid — yet +// the rows within that grid still widen. Grouping by the primary key makes it +// visible: only the six completed orders may appear. +#[tokio::test(flavor = "multi_thread")] +async fn test_filter_exclude_keeps_row_set_when_member_not_grouped() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.total_amount + - orders.amount_exclude_status + - orders.amount_grain_and_filter_exclude_status + dimensions: + - orders.id + filters: + - dimension: orders.status + operator: equals + values: + - completed + order: + - id: orders.id + "#}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +// `keep_only` on a segment, with the dropped filter's dimension outside the +// query grid so the value differs from the plain measure: the segment still +// restricts to completed orders while the category filter is gone, so the +// measure spans every category and the plain measure only books. +#[tokio::test(flavor = "multi_thread")] +async fn test_keep_only_segment_value_ignores_dropped_filter() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.total_amount + - orders.amount_keep_only_segment + dimensions: + - orders.status + segments: + - orders.completed_orders + filters: + - dimension: orders.category + operator: equals + values: + - books + order: + - id: orders.status + "#}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +// A rolling window rewrites the date range it hands to its base state, so the +// filter the inner measure drops no longer carries the query's own bounds. The +// row set survives that regardless: a rolling window takes its rows from the +// time series built out of the query's `dateRange`, and its values through the +// frame condition derived from the same range — neither depends on the leaf +// keeping its date filter. +#[tokio::test(flavor = "multi_thread")] +async fn test_rolling_window_over_dropped_date_filter() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.total_amount + - orders.rolling_amount_no_date_bound + time_dimensions: + - dimension: orders.created_at + granularity: month + dateRange: + - "2024-03-01" + - "2024-03-31" + order: + - id: orders.created_at + "#}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +// The same shape with a dimension and a range narrow enough that the trailing +// window reaches months the leaf can see but the query did not ask for. +#[tokio::test(flavor = "multi_thread")] +async fn test_rolling_window_over_dropped_date_filter_by_dimension() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.total_amount + - orders.rolling_amount_no_date_bound + dimensions: + - orders.category + time_dimensions: + - dimension: orders.created_at + granularity: month + dateRange: + - "2024-01-01" + - "2024-01-31" + order: + - id: orders.created_at + - id: orders.category + "#}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +// The directive's `include` predicate shares operator and values with the query +// filter it drops but names a different member. The keys side is planned on the +// inherited state, so a member-blind CTE dedup key would hand back the widened +// measure CTE and leave the row set widened. +#[tokio::test(flavor = "multi_thread")] +async fn test_filter_exclude_with_lookalike_include_keeps_query_row_set() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.total_amount + - orders.amount_exclude_status_include_lookalike + dimensions: + - orders.status + - orders.category + filters: + - dimension: orders.status + operator: notEquals + values: + - pending + order: + - id: orders.status + - id: orders.category + "#}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +// Rank is out of the keys side on purpose: ranking within the query grid would +// leave one row per partition and collapse every rank to 1. The cost is that the +// statuses the query filtered out stay in the result, carrying a NULL for every +// measure that does honour the filter. Pins that trade rather than endorsing it. +#[tokio::test(flavor = "multi_thread")] +async fn test_rank_with_filter_exclude_ranks_over_whole_universe() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.total_amount + - orders.category_rank_all_statuses + dimensions: + - orders.status + - orders.category + filters: + - dimension: orders.status + operator: equals + values: + - completed + order: + - id: orders.status + - id: orders.category + "#}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__filter_exclude_keeps_row_set_when_member_not_grouped.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__filter_exclude_keeps_row_set_when_member_not_grouped.snap new file mode 100644 index 0000000000000..81ffe52b7ec36 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__filter_exclude_keeps_row_set_when_member_not_grouped.snap @@ -0,0 +1,12 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs +expression: result +--- +orders__id | orders__total_amount | orders__amount_exclude_status | orders__amount_grain_and_filter_exclude_status +-----------+----------------------+-------------------------------+----------------------------------------------- +1 | 100.00 | 100.00 | 100.00 +2 | 200.00 | 200.00 | 200.00 +6 | 300.00 | 300.00 | 300.00 +7 | 200.00 | 200.00 | 200.00 +11 | 400.00 | 400.00 | 400.00 +12 | 200.00 | 200.00 | 200.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__filter_exclude_with_lookalike_include_keeps_query_row_set.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__filter_exclude_with_lookalike_include_keeps_query_row_set.snap new file mode 100644 index 0000000000000..455c6f13cd472 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__filter_exclude_with_lookalike_include_keeps_query_row_set.snap @@ -0,0 +1,12 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs +expression: result +--- +orders__status | orders__category | orders__total_amount | orders__amount_exclude_status_include_lookalike +---------------+------------------+----------------------+------------------------------------------------ +cancelled | books | 100.00 | 100.00 +cancelled | clothing | 50.00 | 50.00 +cancelled | electronics | 50.00 | 50.00 +completed | books | 600.00 | 600.00 +completed | clothing | 300.00 | 300.00 +completed | electronics | 500.00 | 500.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__grain_and_filter_exclude_keeps_query_row_set.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__grain_and_filter_exclude_keeps_query_row_set.snap new file mode 100644 index 0000000000000..047115d46ed44 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__grain_and_filter_exclude_keeps_query_row_set.snap @@ -0,0 +1,9 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs +expression: result +--- +orders__status | orders__category | orders__total_amount | orders__amount_grain_and_filter_exclude_status +---------------+------------------+----------------------+----------------------------------------------- +completed | books | 600.00 | 880.00 +completed | clothing | 300.00 | 720.00 +completed | electronics | 500.00 | 650.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__grain_exclude_keeps_null_dimension_key.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__grain_exclude_keeps_null_dimension_key.snap new file mode 100644 index 0000000000000..0f4ea3599ec4a --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__grain_exclude_keeps_null_dimension_key.snap @@ -0,0 +1,9 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs +expression: result +--- +orders__status | orders__category_nullable | orders__total_amount | orders__amount_grain_and_filter_exclude_status +---------------+---------------------------+----------------------+----------------------------------------------- +completed | clothing | 300.00 | 720.00 +completed | electronics | 500.00 | 650.00 +completed | NULL | 600.00 | 880.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__keep_only_segment_drops_other_filters.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__keep_only_segment_drops_other_filters.snap index c2aa42fb27592..c334effc16da8 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__keep_only_segment_drops_other_filters.snap +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__keep_only_segment_drops_other_filters.snap @@ -5,6 +5,4 @@ expression: result --- orders__category | orders__total_amount | orders__amount_keep_only_segment -----------------+----------------------+--------------------------------- -books | 600.00 | 600.00 -clothing | NULL | 300.00 -electronics | NULL | 500.00 +books | 600.00 | 600.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__keep_only_segment_value_ignores_dropped_filter.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__keep_only_segment_value_ignores_dropped_filter.snap new file mode 100644 index 0000000000000..73b0d4be6ca1f --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__keep_only_segment_value_ignores_dropped_filter.snap @@ -0,0 +1,7 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs +expression: result +--- +orders__status | orders__total_amount | orders__amount_keep_only_segment +---------------+----------------------+--------------------------------- +completed | 600.00 | 1400.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__rank_with_filter_exclude_ranks_over_whole_universe.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__rank_with_filter_exclude_ranks_over_whole_universe.snap new file mode 100644 index 0000000000000..70a302bae503f --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__rank_with_filter_exclude_ranks_over_whole_universe.snap @@ -0,0 +1,15 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs +expression: result +--- +orders__status | orders__category | orders__total_amount | orders__category_rank_all_statuses +---------------+------------------+----------------------+----------------------------------- +cancelled | books | NULL | 3 +cancelled | clothing | NULL | 3 +cancelled | electronics | NULL | 3 +completed | books | 600.00 | 1 +completed | clothing | 300.00 | 2 +completed | electronics | 500.00 | 1 +pending | books | NULL | 2 +pending | clothing | NULL | 1 +pending | electronics | NULL | 2 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__rolling_window_over_dropped_date_filter.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__rolling_window_over_dropped_date_filter.snap new file mode 100644 index 0000000000000..fe0993a8777e6 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__rolling_window_over_dropped_date_filter.snap @@ -0,0 +1,7 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs +expression: result +--- +orders__created_at_month | orders__total_amount | orders__rolling_amount_no_date_bound +-------------------------+----------------------+------------------------------------- +2024-03-01 00:00:00 | 1000.00 | 2250.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__rolling_window_over_dropped_date_filter_by_dimension.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__rolling_window_over_dropped_date_filter_by_dimension.snap new file mode 100644 index 0000000000000..2eaacac3e13d8 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__rolling_window_over_dropped_date_filter_by_dimension.snap @@ -0,0 +1,9 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs +expression: result +--- +orders__category | orders__created_at_month | orders__total_amount | orders__rolling_amount_no_date_bound +-----------------+--------------------------+----------------------+------------------------------------- +books | 2024-01-01 00:00:00 | 230.00 | 230.00 +clothing | 2024-01-01 00:00:00 | 120.00 | 120.00 +electronics | 2024-01-01 00:00:00 | 150.00 | 150.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__share_of_grand_total_keeps_query_row_set.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__share_of_grand_total_keeps_query_row_set.snap new file mode 100644 index 0000000000000..aa86ff0523e33 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__filter_directive__share_of_grand_total_keeps_query_row_set.snap @@ -0,0 +1,9 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/filter_directive.rs +expression: result +--- +orders__status | orders__category | orders__amount_share_percent +---------------+------------------+----------------------------- +completed | books | 26.6666666666666667 +completed | clothing | 13.3333333333333333 +completed | electronics | 22.2222222222222222 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__views__view_keep_only_segment_at_leaf.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__views__view_keep_only_segment_at_leaf.snap index 890c172a1ec93..27996a5cdb088 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__views__view_keep_only_segment_at_leaf.snap +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__views__view_keep_only_segment_at_leaf.snap @@ -4,6 +4,4 @@ expression: result --- orders_ms_view__category | orders_ms_view__total_amount | orders_ms_view__amount_keep_only_segment -------------------------+------------------------------+----------------------------------------- -books | 600.00 | 600.00 -clothing | NULL | 300.00 -electronics | NULL | 500.00 +books | 600.00 | 600.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/mod.rs index ae99a03383f86..e27e269f61412 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/mod.rs @@ -1,3 +1,4 @@ mod multi_fact; mod multi_stage; mod sql_generation; +mod ungrouped_gate; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/snapshots/cubesqlplanner__tests__integration__pre_aggregations__sql_generation__ungrouped_cross_cube_view_joined_keys_rollup_cubestore_result.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/snapshots/cubesqlplanner__tests__integration__pre_aggregations__sql_generation__ungrouped_cross_cube_view_joined_keys_rollup_cubestore_result.snap new file mode 100644 index 0000000000000..8e0d97a48422d --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/snapshots/cubesqlplanner__tests__integration__pre_aggregations__sql_generation__ungrouped_cross_cube_view_joined_keys_rollup_cubestore_result.snap @@ -0,0 +1,19 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/sql_generation.rs +expression: result +--- +visitors_view__visitors_source | visitors_view__visitor_checkins_visitor_id +-------------------------------+------------------------------------------- +google | NULL +google | NULL +google | NULL +google | 1 +google | 1 +google | 1 +google | 8 +organic | NULL +organic | 6 +twitter | 4 +twitter | 4 +twitter | 5 +NULL | 7 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/snapshots/cubesqlplanner__tests__integration__pre_aggregations__sql_generation__ungrouped_pre_agg_measure_reads_rollup_column_cubestore_result.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/snapshots/cubesqlplanner__tests__integration__pre_aggregations__sql_generation__ungrouped_pre_agg_measure_reads_rollup_column_cubestore_result.snap index 559dcf5521533..4716ffabb09f2 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/snapshots/cubesqlplanner__tests__integration__pre_aggregations__sql_generation__ungrouped_pre_agg_measure_reads_rollup_column_cubestore_result.snap +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/snapshots/cubesqlplanner__tests__integration__pre_aggregations__sql_generation__ungrouped_pre_agg_measure_reads_rollup_column_cubestore_result.snap @@ -2,12 +2,15 @@ source: cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/sql_generation.rs expression: result --- -orders__status | orders__city | orders__total_amount ----------------+--------------+--------------------- -cancelled | Chicago | 25 -completed | Boston | 375 -completed | Chicago | 400 -completed | New York | 150 -pending | Boston | 150 -pending | Chicago | 175 -pending | New York | 260 +orders__id | orders__status | orders__city | orders__total_amount +-----------+----------------+--------------+--------------------- +1 | completed | New York | 100 +2 | pending | New York | 200 +3 | completed | New York | 50 +4 | completed | Boston | 300 +5 | pending | Boston | 150 +6 | completed | Boston | 75 +7 | cancelled | Chicago | 25 +8 | completed | Chicago | 400 +9 | pending | Chicago | 175 +10 | pending | New York | 60 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/snapshots/cubesqlplanner__tests__integration__pre_aggregations__sql_generation__ungrouped_view_query_for_join_cubestore_result.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/snapshots/cubesqlplanner__tests__integration__pre_aggregations__sql_generation__ungrouped_view_query_for_join_cubestore_result.snap new file mode 100644 index 0000000000000..a04059d459e0b --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/snapshots/cubesqlplanner__tests__integration__pre_aggregations__sql_generation__ungrouped_view_query_for_join_cubestore_result.snap @@ -0,0 +1,16 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/sql_generation.rs +expression: result +--- +visitors_view__visitors_id | visitors_view__visitors_source +---------------------------+------------------------------- +1 | google +2 | google +3 | google +4 | twitter +5 | twitter +6 | organic +7 | NULL +8 | google +9 | google +10 | organic diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/sql_generation.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/sql_generation.rs index a75cd162fcf87..8994412eda9a1 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/sql_generation.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/sql_generation.rs @@ -1438,25 +1438,25 @@ async fn test_order_by_only_measure_dropped_from_pre_agg() -> Result<(), CubeErr #[tokio::test(flavor = "multi_thread")] async fn test_ungrouped_pre_agg_measure_reads_rollup_column() -> Result<(), CubeError> { let schema = MockSchema::from_yaml_file("common/pre_aggregation_matching_test.yaml") - .only_pre_aggregations(&["main_rollup"]); + .only_pre_aggregations(&["primary_key_rollup"]); let ctx = TestContext::new(schema)?; let query_yaml = indoc! {" measures: - orders.total_amount dimensions: + - orders.id - orders.status - orders.city ungrouped: true order: - - id: orders.status - - id: orders.city + - id: orders.id "}; let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; assert_eq!(pre_aggrs.len(), 1); - assert_eq!(pre_aggrs[0].name(), "main_rollup"); + assert_eq!(pre_aggrs[0].name(), "primary_key_rollup"); assert!( sql.contains("\"orders__total_amount\" \"orders__total_amount\""), "ungrouped measure should project the rollup column, got:\n{sql}" @@ -1520,3 +1520,175 @@ async fn test_rollup_lambda_cross_cube_union_aliases() -> Result<(), CubeError> Ok(()) } + +// `joined_rollup` is a rollup on the fact cube `visitor_checkins` that includes +// a cross-cube dimension from the many_to_one joined `visitors` cube, plus a +// `time_dimension`. An ungrouped query with no measures and no time dimension +// requests raw rows, so it may only use a pre-aggregation whose dimensions +// cover the primary keys of every cube in the query. Here they don't, so +// reading flat rollup columns would silently drop the join between the two +// cubes: the query must fall back to base SQL. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_no_measures_cross_cube_query_should_not_match_rollup( +) -> Result<(), CubeError> { + let schema = MockSchema::from_yaml_file("common/pre_aggregations_test.yaml") + .only_pre_aggregations(&["joined_rollup"]); + let ctx = TestContext::new(schema)?; + + let query_yaml = indoc! {" + dimensions: + - visitor_checkins.visitor_id + - visitors.source + filters: + - dimension: visitors.source + operator: equals + values: + - google + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "expected no pre-aggregation, got `{}`. Generated SQL:\n{sql}", + pre_aggrs + .iter() + .map(|p| p.name().clone()) + .collect::>() + .join(", ") + ); + + assert!( + sql.to_lowercase().contains("join"), + "expected base SQL with a JOIN between visitor_checkins and visitors, got:\n{sql}" + ); + + Ok(()) +} + +// Same rejection as above, reached through a view. The view renames every +// member, so the ungrouped gate must compare the cubes and primary keys the +// view members resolve to rather than the names the query spells them with. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_cross_cube_view_query_should_not_match_rollup() -> Result<(), CubeError> { + let schema = MockSchema::from_yaml_file("common/pre_aggregations_test.yaml") + .only_pre_aggregations(&["joined_rollup"]); + let ctx = TestContext::new(schema)?; + + let query_yaml = indoc! {" + dimensions: + - visitors_view.visitor_checkins_visitor_id + - visitors_view.visitors_source + filters: + - dimension: visitors_view.visitors_source + operator: equals + values: + - google + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "expected no pre-aggregation, got `{}`. Generated SQL:\n{sql}", + pre_aggrs + .iter() + .map(|p| p.name().clone()) + .collect::>() + .join(", ") + ); + + assert!( + sql.to_lowercase().contains("join"), + "expected base SQL with a JOIN between visitor_checkins and visitors, got:\n{sql}" + ); + + Ok(()) +} + +// The mirror case: `for_join` covers the primary key of the only cube the query +// resolves to, so an ungrouped view query must still match it. Guards the gate +// against reading the view itself as a separate cube, which would make the cube +// sets differ and reject every ungrouped query issued through a view. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_view_query_matches_rollup_covering_primary_key() -> Result<(), CubeError> { + let schema = MockSchema::from_yaml_file("common/pre_aggregations_test.yaml") + .only_pre_aggregations(&["for_join"]); + let ctx = TestContext::new(schema)?; + + let query_yaml = indoc! {" + dimensions: + - visitors_view.visitors_id + - visitors_view.visitors_source + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert_eq!( + pre_aggrs.len(), + 1, + "expected `visitors.for_join` to be used, got SQL:\n{sql}" + ); + assert_eq!(pre_aggrs[0].name(), "for_join"); + assert_eq!(pre_aggrs[0].cube_name(), "visitors"); + + // The rows are the claim: one per raw visitor, not per (id, source) group. + if let Some(result) = ctx + .try_execute(query_yaml, "pre_aggregation_tables.sql") + .await + { + insta::assert_snapshot!("ungrouped_view_query_for_join_cubestore_result", result); + } + + Ok(()) +} + +// `joined_keys_rollup` has the same cross-cube shape as `joined_rollup` but is +// grouped by the primary keys of both cubes, so it does hold one row per raw +// joined row and an ungrouped view query spanning both cubes may use it. The +// contrast with the rejected `joined_rollup` is what the gate is discriminating. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_cross_cube_view_query_matches_rollup_covering_both_primary_keys( +) -> Result<(), CubeError> { + let schema = MockSchema::from_yaml_file("common/pre_aggregations_test.yaml") + .only_pre_aggregations(&["joined_keys_rollup"]); + let ctx = TestContext::new(schema)?; + + let query_yaml = indoc! {" + dimensions: + - visitors_view.visitors_source + - visitors_view.visitor_checkins_visitor_id + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert_eq!( + pre_aggrs.len(), + 1, + "expected `joined_keys_rollup` to be used, got SQL:\n{sql}" + ); + assert_eq!(pre_aggrs[0].name(), "joined_keys_rollup"); + // Tokenized so the check is immune to line breaks and to `join` appearing + // inside the rollup's own table name. + assert!( + !sql.to_lowercase().split_whitespace().any(|t| t == "join"), + "reading the rollup should not re-join the cubes, got:\n{sql}" + ); + + // The rows are the claim the gate rests on: one per raw joined row. + if let Some(result) = ctx + .try_execute(query_yaml, "pre_aggregation_tables.sql") + .await + { + insta::assert_snapshot!( + "ungrouped_cross_cube_view_joined_keys_rollup_cubestore_result", + result + ); + } + + Ok(()) +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/ungrouped_gate.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/ungrouped_gate.rs new file mode 100644 index 0000000000000..0cdf81ed88e52 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/pre_aggregations/ungrouped_gate.rs @@ -0,0 +1,567 @@ +//! Eligibility of pre-aggregations for ungrouped queries. +//! +//! An ungrouped query returns raw rows, so a pre-aggregation may serve it only +//! when each stored row maps to exactly one row of the query result. These tests +//! pin both directions of that rule: rollups whose grouping collapses rows must +//! be refused, and rollups that are provably 1:1 with raw rows must be kept. + +use crate::test_fixtures::cube_bridge::MockSchema; +use crate::test_fixtures::test_utils::TestContext; +use cubenativeutils::CubeError; +use indoc::indoc; + +fn ctx_with(pre_aggregations: &[&str]) -> Result { + let schema = MockSchema::from_yaml_file("common/ungrouped_pre_agg_gate.yaml") + .only_pre_aggregations(pre_aggregations); + TestContext::new(schema) +} + +fn used_names(pre_aggrs: &[crate::logical_plan::PreAggregationUsage]) -> String { + pre_aggrs + .iter() + .map(|p| p.name().clone()) + .collect::>() + .join(", ") +} + +// A cube may legally declare no primary key. Nothing then identifies a raw row, +// so no rollup can be read as raw rows and the query must fall back to the +// source table. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_rejects_rollup_when_cube_has_no_primary_key() -> Result<(), CubeError> { + let ctx = ctx_with(&["collapsing"])?; + + let query_yaml = indoc! {" + measures: + - no_pk_orders.total + dimensions: + - no_pk_orders.status + - no_pk_orders.city + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "a cube without a primary key has no raw-row identity, so `{}` must not \ + serve an ungrouped query. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// `chain_checkins` is not named by the query, but it sits between the two cubes +// that are, and its one_to_many edge multiplies rows. The rollup is grouped by +// the visitor and city keys only, so it stores one row per (visitor, city) pair +// while the raw join yields one row per checkin. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_rejects_rollup_missing_transit_cube_key() -> Result<(), CubeError> { + let ctx = ctx_with(&["chain_rollup"])?; + + let query_yaml = indoc! {" + dimensions: + - chain_visitors.source + - chain_cities.name + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "the fan-out transit cube `chain_checkins` is unaccounted for, so `{}` \ + must not serve an ungrouped query. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// A many_to_one join cannot multiply rows, so the fact primary key alone +// identifies every stored row and the dimension cube's key is not needed. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_accepts_star_rollup_without_dimension_cube_key() -> Result<(), CubeError> { + let ctx = ctx_with(&["star_rollup"])?; + + let query_yaml = indoc! {" + measures: + - star_checkins.cnt + dimensions: + - star_checkins.id + - star_visitors.source + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert_eq!( + pre_aggrs.len(), + 1, + "the fact key identifies each row across a many_to_one join, so \ + `star_rollup` should be used. Generated SQL:\n{sql}" + ); + assert_eq!(pre_aggrs[0].name(), "star_rollup"); + + Ok(()) +} + +// `own_ref` is a proxy for the fact cube's own primary key, which row identity +// does require, so the key is stored even though it is spelled under another +// name and only a resolved comparison can see it. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_accepts_rollup_storing_primary_key_by_reference() -> Result<(), CubeError> { + let ctx = ctx_with(&["proxy_rollup"])?; + + let query_yaml = indoc! {" + measures: + - star_checkins.cnt + dimensions: + - star_checkins.own_ref + - star_visitors.source + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert_eq!( + pre_aggrs.len(), + 1, + "`own_ref` resolves to `star_checkins.id`, so `proxy_rollup` does store \ + the identifying key and should be used. Generated SQL:\n{sql}" + ); + assert_eq!(pre_aggrs[0].name(), "proxy_rollup"); + + Ok(()) +} + +// A cross-cube segment only filters rows; it neither adds columns nor changes +// the stored grain, so the segment's cube must not be required to contribute a +// primary key. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_accepts_rollup_whose_extra_cube_comes_from_a_segment( +) -> Result<(), CubeError> { + let ctx = ctx_with(&["seg_rollup"])?; + + let query_yaml = indoc! {" + measures: + - star_checkins.cnt + dimensions: + - star_checkins.id + - star_checkins.visitor_id + segments: + - star_visitors.source_google + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert_eq!( + pre_aggrs.len(), + 1, + "a segment only filters, so `seg_rollup` stays 1:1 with raw rows and \ + should be used. Generated SQL:\n{sql}" + ); + assert_eq!(pre_aggrs[0].name(), "seg_rollup"); + + Ok(()) +} + +// The rollup is 1:1 with raw rows, but `uniq` is stored as an HLL sketch that +// only means anything after a merge. An ungrouped read projects the column +// as-is, so it would hand the client a binary sketch instead of a count. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_rejects_rollup_storing_non_additive_state() -> Result<(), CubeError> { + let ctx = ctx_with(&["hll_pk_rollup"])?; + + let query_yaml = indoc! {" + measures: + - hll_orders.uniq + dimensions: + - hll_orders.id + - hll_orders.status + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "an ungrouped read cannot finalize the stored HLL state, so `{}` must \ + not serve this query. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// A fan-out measure under `ungrouped` is rendered inline rather than through a +// multiplied subquery, so this stays a simple query. The rollup groups by +// non-key columns and must be refused. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_rejects_collapsing_rollup_with_fan_out_measure() -> Result<(), CubeError> { + let ctx = ctx_with(&["mult_collapsing"])?; + + let query_yaml = indoc! {" + measures: + - mult_customers.count + dimensions: + - mult_customers.name + - mult_orders.status + ungrouped: true + cubestoreSupportMultistage: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "`{}` groups by non-key columns, so it must not serve an ungrouped \ + query. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// A non-additive rolling measure makes the planner render its leaf stage +// ungrouped, but the user query is grouped and asks for aggregates, so the +// raw-row rule must not be applied to that leaf. +#[tokio::test(flavor = "multi_thread")] +async fn test_grouped_non_additive_rolling_query_still_uses_rollup() -> Result<(), CubeError> { + let ctx = ctx_with(&["rolling_avg_rollup"])?; + + let query_yaml = indoc! {r#" + measures: + - roll_orders.rolling_avg_7d + dimensions: + - roll_orders.category + time_dimensions: + - dimension: roll_orders.created_at + granularity: day + dateRange: + - "2024-01-10" + - "2024-01-25" + cubestoreSupportMultistage: true + "#}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert_eq!( + pre_aggrs.len(), + 1, + "the user query is grouped, so the internal ungrouped leaf must not cost \ + it `rolling_avg_rollup`. Generated SQL:\n{sql}" + ); + assert_eq!(pre_aggrs[0].name(), "rolling_avg_rollup"); + + Ok(()) +} + +// `lam_union` exposes only its first member rollup's symbols. The second branch +// groups by a non-unique column that merely shares the short name `id`, so the +// union as a whole is not 1:1 with raw rows. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_rejects_lambda_whose_other_branch_collapses() -> Result<(), CubeError> { + let ctx = ctx_with(&["lam_base", "lam_union"])?; + + let query_yaml = indoc! {" + measures: + - lam_a.count + dimensions: + - lam_a.id + - lam_a.visitor_id + time_dimensions: + - dimension: lam_a.created_at + granularity: day + ungrouped: true + pre_aggregation_id: lam_a.lam_union + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "the union's second branch is grouped on a non-key column, so `{}` must \ + not serve an ungrouped query. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// `rj_keys_join` declares both primary keys, but the rollups it actually reads +// store neither `rj_checkins.id` nor a per-checkin grain, so its rows are one +// per visitor. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_rejects_rollup_join_advertising_unstored_keys() -> Result<(), CubeError> { + let ctx = ctx_with(&["rj_base", "rj_keys_join"])?; + + let query_yaml = indoc! {" + measures: + - rj_checkins.count + dimensions: + - rj_checkins.visitor_id + - rj_visitors.source + ungrouped: true + pre_aggregation_id: rj_checkins.rj_keys_join + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "the member rollups do not store `rj_checkins.id`, so `{}` must not serve \ + an ungrouped query. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// The single-cube shape: the cube has a key, the rollup omits it, so its rows +// are collapsed and an ungrouped query must read the source table instead. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_single_cube_rejects_rollup_missing_primary_key() -> Result<(), CubeError> { + let ctx = ctx_with(&["sc_coarse"])?; + + let query_yaml = indoc! {" + measures: + - sc_orders.total + dimensions: + - sc_orders.status + - sc_orders.city + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "`{}` groups by non-key columns, so it must not serve an ungrouped \ + single-cube query. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// A filter is enough to pull a fan-out cube into the join even though the query +// selects none of its members, so row identity has to account for it. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_rejects_rollup_when_a_filter_widens_the_join() -> Result<(), CubeError> { + let ctx = ctx_with(&["chain_rollup"])?; + + let query_yaml = indoc! {" + dimensions: + - chain_visitors.id + - chain_visitors.source + filters: + - dimension: chain_cities.name + operator: equals + values: + - X + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "the filter joins in the fan-out cube `chain_checkins`, so `{}` must not \ + serve an ungrouped query. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// The mirror direction of the rule: the rollup is grouped across a fan-out cube +// the query never joins, so it holds more rows than the query asks for and would +// return each visitor once per city. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_rejects_rollup_stored_at_a_finer_grain() -> Result<(), CubeError> { + let ctx = ctx_with(&["chain_rollup"])?; + + let query_yaml = indoc! {" + dimensions: + - chain_visitors.id + - chain_visitors.source + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "`{}` is stored per (visitor, city) while the query reads one row per \ + visitor, so it must not serve it. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// A segment only filters, but reaching it joins in the fan-out cube, so a rollup +// keyed on the root alone no longer holds one row per output row. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_rejects_rollup_whose_segment_crosses_a_fan_out() -> Result<(), CubeError> { + let ctx = ctx_with(&["root_key_seg_rollup"])?; + + let query_yaml = indoc! {" + dimensions: + - chain_visitors.id + - chain_visitors.source + segments: + - chain_cities.named_x + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "the segment joins in the fan-out cube `chain_checkins`, so `{}` must not \ + serve an ungrouped query. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// `bridge_stores` only bridges two many_to_one edges, so it cannot split a row +// and its key is no part of row identity — the fact key alone identifies the +// output row even though the bridge sits in the middle of the join. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_accepts_rollup_across_a_many_to_one_bridge() -> Result<(), CubeError> { + let ctx = ctx_with(&["bridge_rollup"])?; + + let query_yaml = indoc! {" + measures: + - bridge_orders.cnt + dimensions: + - bridge_orders.id + - bridge_regions.name + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert_eq!( + pre_aggrs.len(), + 1, + "a many_to_one bridge cannot split a row, so `bridge_rollup` should be \ + used. Generated SQL:\n{sql}" + ); + assert_eq!(pre_aggrs[0].name(), "bridge_rollup"); + + Ok(()) +} + +// A measure joins whatever its own sql references, but the rollup aggregates +// that join away, so the referenced cube is no part of the stored grain and must +// not make the rollup ineligible. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_accepts_rollup_with_a_measure_reaching_another_cube( +) -> Result<(), CubeError> { + let ctx = ctx_with(&["mult_keyed"])?; + + let query_yaml = indoc! {" + dimensions: + - mult_customers.id + - mult_customers.name + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert_eq!( + pre_aggrs.len(), + 1, + "`mult_keyed` is grouped by the primary key, so it should be used. \ + Generated SQL:\n{sql}" + ); + assert_eq!(pre_aggrs[0].name(), "mult_keyed"); + + Ok(()) +} + +// A rollup's segments are appended to its dimension list when the table is +// materialized, so a segment groups the stored rows just like a dimension. One +// reaching across a row-splitting join therefore stores a row per (entity, +// segment value) even though the query never asks for the segment. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_rejects_rollup_whose_unrequested_segment_splits_rows( +) -> Result<(), CubeError> { + let ctx = ctx_with(&["root_key_seg_rollup"])?; + + let query_yaml = indoc! {" + dimensions: + - chain_visitors.id + - chain_visitors.source + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert!( + pre_aggrs.is_empty(), + "`{}` groups by a segment of `chain_cities`, reached over the fan-out to \ + `chain_checkins`, so it holds more than one row per visitor. Generated \ + SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} + +// A rollup stores its time dimension truncated to `granularity`, so that column +// must never reach a raw-row result. It cannot: a time dimension requested +// without a granularity is a filter, not an output member, and one requested +// with a granularity only matches a rollup storing that same granularity. +#[tokio::test(flavor = "multi_thread")] +async fn test_ungrouped_never_reads_a_truncated_time_dimension() -> Result<(), CubeError> { + let ctx = ctx_with(&["star_day_rollup"])?; + + let query_yaml = indoc! {" + measures: + - star_checkins.cnt + dimensions: + - star_checkins.id + time_dimensions: + - dimension: star_checkins.created_at + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(query_yaml)?; + + assert_eq!( + pre_aggrs.len(), + 1, + "`star_day_rollup` is grouped by the primary key, so it holds one row per checkin and may serve raw rows. Generated SQL:\n{sql}" + ); + assert!( + !sql.contains("created_at"), + "the stored time dimension is truncated to the day, so no raw-row result may read it. Generated SQL:\n{sql}" + ); + + // Asking for the raw value as a plain dimension finds no match at all: the + // rollup groups by `id` alone and stores no untruncated timestamp. + let raw_value_query = indoc! {" + dimensions: + - star_checkins.id + - star_checkins.created_at + ungrouped: true + "}; + + let (sql, pre_aggrs) = ctx.build_sql_with_used_pre_aggregations(raw_value_query)?; + + assert!( + pre_aggrs.is_empty(), + "`{}` stores no untruncated timestamp. Generated SQL:\n{sql}", + used_names(&pre_aggrs) + ); + + Ok(()) +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/snapshots/cubesqlplanner__tests__integration__modifiers__ungrouped_multiplied_keeps_group_by_for_aggregate_sql_measure.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/snapshots/cubesqlplanner__tests__integration__modifiers__ungrouped_multiplied_keeps_group_by_for_aggregate_sql_measure.snap new file mode 100644 index 0000000000000..8900c105826d9 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/snapshots/cubesqlplanner__tests__integration__modifiers__ungrouped_multiplied_keeps_group_by_for_aggregate_sql_measure.snap @@ -0,0 +1,7 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/modifiers.rs +expression: result +--- +dup_customers__name | dup_orders__status | dup_customers__ltv_agg_number +--------------------+--------------------+------------------------------ +dup | new | 150.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs index 5cb0406fda012..fef9974441c84 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs @@ -6,6 +6,7 @@ mod cube_names_collector; mod date_filters; mod dimension_symbol; mod filter; +mod filter_params_callback_column; mod join_hints_collector; mod measure_symbol; mod member_expressions_on_views;