-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(tesseract): push a segment named in FILTER_PARAMS into the cube's sql #11517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| import { getEnv } from '@cubejs-backend/shared'; | ||
| import { PostgresQuery } from '../../../src/adapter/PostgresQuery'; | ||
| import { prepareJsCompiler } from '../../unit/PrepareCompiler'; | ||
| import { dbRunner } from './PostgresDBRunner'; | ||
|
|
||
| // A cube wrapping a large scan pushes predicates into its own `sql` through | ||
| // FILTER_PARAMS. A segment can be named there like any other member: its | ||
| // binding renders the column it was given whenever the query selects that | ||
| // segment, and `1 = 1` when it does not. | ||
| describe('FILTER_PARAMS referencing a segment', () => { | ||
| jest.setTimeout(200000); | ||
|
|
||
| const events = ` | ||
| SELECT * FROM ( | ||
| SELECT 1 as id, 115 as evid, 'load' as action_group, 'us' as region | ||
| union all | ||
| SELECT 2 as id, 115 as evid, 'load' as action_group, 'eu' as region | ||
| union all | ||
| SELECT 3 as id, 200 as evid, 'click' as action_group, 'us' as region | ||
| ) AS t | ||
| `; | ||
|
|
||
| const compilers = prepareJsCompiler(` | ||
| cube('events', { | ||
| sql: \`${events} WHERE \${FILTER_PARAMS.events.start_load.filter("evid = 115 AND action_group = 'load'")} | ||
| AND \${FILTER_PARAMS.events.region.filter('region')}\`, | ||
| measures: { | ||
| count: { type: 'count' }, | ||
| }, | ||
| dimensions: { | ||
| id: { sql: 'id', type: 'number', primaryKey: true }, | ||
| region: { sql: 'region', type: 'string' }, | ||
| }, | ||
| segments: { | ||
| start_load: { sql: \`\${CUBE}.evid = 115 AND \${CUBE}.action_group = 'load'\` }, | ||
| us_only: { sql: \`\${CUBE}.region = 'us'\` }, | ||
| }, | ||
| }); | ||
|
|
||
| cube('grouped_events', { | ||
| sql: \`${events} WHERE \${FILTER_GROUP( | ||
| FILTER_PARAMS.grouped_events.start_load.filter("evid = 115 AND action_group = 'load'"), | ||
| FILTER_PARAMS.grouped_events.region.filter('region') | ||
| )}\`, | ||
| measures: { | ||
| count: { type: 'count' }, | ||
| }, | ||
| dimensions: { | ||
| id: { sql: 'id', type: 'number', primaryKey: true }, | ||
| region: { sql: 'region', type: 'string' }, | ||
| }, | ||
| segments: { | ||
| start_load: { sql: \`\${CUBE}.evid = 115 AND \${CUBE}.action_group = 'load'\` }, | ||
| }, | ||
| }); | ||
|
|
||
| cube('callback_events', { | ||
| sql: \`${events} WHERE \${FILTER_PARAMS.callback_events.start_load.filter(() => "evid = 115")} | ||
| AND \${FILTER_PARAMS.callback_events.needs_value.filter((v) => 'evid = ' + v)}\`, | ||
| measures: { | ||
| count: { type: 'count' }, | ||
| }, | ||
| dimensions: { | ||
| id: { sql: 'id', type: 'number', primaryKey: true }, | ||
| }, | ||
| segments: { | ||
| start_load: { sql: \`\${CUBE}.evid = 115\` }, | ||
| needs_value: { sql: \`\${CUBE}.evid = 115\` }, | ||
| }, | ||
| }); | ||
| `); | ||
|
|
||
| // The cube's `sql` is a subquery aliased as the cube, so everything before | ||
| // that alias is what the pushdown produced. | ||
| const baseSql = (sql: string, cube: string) => { | ||
| const alias = sql.indexOf(`AS "${cube}"`); | ||
| // Without the alias the slice would be the whole query, and the negative | ||
| // assertions would silently stop testing the pushed-down part. | ||
| expect(alias).toBeGreaterThan(-1); | ||
| return sql.slice(0, alias); | ||
| }; | ||
|
|
||
| const buildSql = async (query: any) => { | ||
| await compilers.compiler.compile(); | ||
| return new PostgresQuery(compilers, { timezone: 'UTC', ...query }).buildSqlAndParams()[0]; | ||
| }; | ||
|
|
||
| if (getEnv('nativeSqlPlanner')) { | ||
| it('pushes the segment predicate into the cube sql when the segment is selected', async () => { | ||
| const sql = await buildSql({ | ||
| measures: ['events.count'], | ||
| segments: ['events.start_load'], | ||
| }); | ||
|
|
||
| expect(baseSql(sql, 'events')).toMatch(/evid = 115 AND action_group = 'load'/); | ||
| }); | ||
|
|
||
| it('leaves the binding always-true when the segment is not selected', async () => { | ||
| const sql = await buildSql({ measures: ['events.count'] }); | ||
|
|
||
| expect(baseSql(sql, 'events')).not.toMatch(/evid = 115/); | ||
| expect(baseSql(sql, 'events')).toMatch(/1\s*=\s*1/); | ||
| }); | ||
|
|
||
| it('does not activate the binding for a different segment', async () => { | ||
| const sql = await buildSql({ | ||
| measures: ['events.count'], | ||
| segments: ['events.us_only'], | ||
| }); | ||
|
|
||
| expect(baseSql(sql, 'events')).not.toMatch(/evid = 115/); | ||
| }); | ||
|
|
||
| it('pushes the segment down alongside a dimension filter', async () => { | ||
| const sql = await buildSql({ | ||
| measures: ['events.count'], | ||
| segments: ['events.start_load'], | ||
| filters: [{ member: 'events.region', operator: 'equals', values: ['us'] }], | ||
| }); | ||
|
|
||
| const base = baseSql(sql, 'events'); | ||
| expect(base).toMatch(/evid = 115 AND action_group = 'load'/); | ||
| expect(base).toMatch(/region = \$\d/); | ||
| expect(base).not.toMatch(/1\s*=\s*1/); | ||
| }); | ||
|
|
||
| it('renders the segment as one member of a FILTER_GROUP', async () => { | ||
| const sql = await buildSql({ | ||
| measures: ['grouped_events.count'], | ||
| segments: ['grouped_events.start_load'], | ||
| }); | ||
|
|
||
| expect(baseSql(sql, 'grouped_events')).toMatch(/evid = 115 AND action_group = 'load'/); | ||
| }); | ||
|
|
||
| it('renders a callback column that takes no filter values', async () => { | ||
| const sql = await buildSql({ | ||
| measures: ['callback_events.count'], | ||
| segments: ['callback_events.start_load'], | ||
| }); | ||
|
|
||
| // Nothing else in this cube's sql states the predicate, so it can only | ||
| // have come from the callback the binding compiled. | ||
| expect(baseSql(sql, 'callback_events')).toMatch(/WHERE \(evid = 115\)/); | ||
| }); | ||
|
|
||
| // A segment supplies no values, so a column that takes one cannot render. | ||
| // Dropping only its restatement is narrower than binding a value the | ||
| // segment never gave — the segment still filters the query on its own. | ||
| it('leaves a value-taking callback column always-true', async () => { | ||
| const sql = await buildSql({ | ||
| measures: ['callback_events.count'], | ||
| segments: ['callback_events.needs_value'], | ||
| }); | ||
|
|
||
| // The parentheses are what the filter renderer adds around a binding it | ||
| // reached, so they tell an activated-then-dropped column apart from the | ||
| // bare `1 = 1` of a binding whose segment was never selected. | ||
| expect(baseSql(sql, 'callback_events')).toMatch(/AND \(1\s*=\s*1\)/); | ||
| expect(baseSql(sql, 'callback_events')).not.toMatch(/undefined|\{fpv:/); | ||
| }); | ||
|
|
||
| // The predicate now applies both inside the cube's sql and in the outer | ||
| // WHERE the segment always produced. Both restrict the same rows, so the | ||
| // result must be what the segment alone selected. | ||
| it('counts the segment rows once', async () => dbRunner.runQueryTest({ | ||
| measures: ['events.count'], | ||
| segments: ['events.start_load'], | ||
| timezone: 'UTC', | ||
| }, [ | ||
| { events__count: '2' }, | ||
| ], compilers)); | ||
|
|
||
| it('counts every row when no segment is selected', async () => dbRunner.runQueryTest({ | ||
| measures: ['events.count'], | ||
| timezone: 'UTC', | ||
| }, [ | ||
| { events__count: '3' }, | ||
| ], compilers)); | ||
| } else { | ||
| // Segment pushdown is implemented in the Tesseract planner only. | ||
| test.skip('FILTER_PARAMS referencing a segment', () => { expect(1).toBe(1); }); | ||
| } | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,36 @@ impl BaseSegment { | |
| pub fn is_member_expression(&self) -> bool { | ||
| self.is_member_expression | ||
| } | ||
|
|
||
| /// Whether `member` names this segment, as a `FILTER_PARAMS` binding or a | ||
| /// filter-tree target does. A view exposes a segment under its own path | ||
| /// while a binding in the underlying cube's sql names the cube's, so every | ||
| /// segment in the reference chain counts, not only the name the query asked | ||
| /// for. The chain stops at the first non-segment: a segment whose sql is a | ||
| /// bare reference resolves on to that dimension, whose own binding states a | ||
| /// column to compare a value against rather than a predicate. | ||
| pub fn matches_member_name(&self, member: &str) -> bool { | ||
| if self.is_member_expression { | ||
| return false; | ||
| } | ||
| if self.full_name == member { | ||
| return true; | ||
| } | ||
| let mut current = Some(self.member_evaluator.clone()); | ||
| while let Some(symbol) = current { | ||
| if symbol.as_member_expression().is_err() { | ||
| return false; | ||
| } | ||
| // A segment symbol lives in the `expr:` namespace, so the path is | ||
| // reassembled from the cube and member names it was compiled under. | ||
| if format!("{}.{}", symbol.cube_name(), symbol.name()) == member { | ||
| return true; | ||
| } | ||
| current = symbol.reference_member(); | ||
| } | ||
|
Comment on lines
+75
to
+86
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: the first iteration of the loop re-checks what line 72 already covered — for a cube-level segment The chain walk itself reads correctly to me, and the One asymmetry worth being aware of: |
||
| false | ||
| } | ||
|
|
||
| pub fn full_name(&self) -> String { | ||
| self.full_name.clone() | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: the
eventscube uses twoFILTER_PARAMSin onesqlwithout wrapping them inFILTER_GROUP, which the docs page this PR edits states is required ("If you useFILTER_PARAMSin your query more than once, you must wrap them withFILTER_GROUP"). It works here because the two are ANDed explicitly, and thegrouped_eventscube covers the grouped form — but a fixture that contradicts the documented rule is an odd thing to point future readers at. Splitting the second binding into its own cube (or wrapping both) keeps the fixture on-spec without losing any assertion.