Skip to content

Commit 41ed9cc

Browse files
authored
fix(schema-compiler): fix FILTER_PARAMS flow in pre aggregations filtering (#8761)
* fix(schema-compiler): fix FILTER_PARAMS flow in pre aggregations filtering * add tests
1 parent a1c63ac commit 41ed9cc

File tree

2 files changed

+102
-13
lines changed

2 files changed

+102
-13
lines changed

packages/cubejs-schema-compiler/src/adapter/BaseQuery.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2866,8 +2866,11 @@ export class BaseQuery {
28662866

28672867
newSubQueryForCube(cube, options) {
28682868
if (this.options.queryFactory) {
2869-
options.paramAllocator = null;
2870-
return this.options.queryFactory.createQuery(cube, this.compilers, this.subQueryOptions(options));
2869+
// When dealing with rollup joins, it's crucial to use the correct parameter allocator for the specific cube in use.
2870+
// By default, we'll use BaseQuery, but it's important to note that different databases (Oracle, PostgreSQL, MySQL, Druid, etc.)
2871+
// have unique parameter allocator symbols. Using the wrong allocator can break the query, especially when rollup joins involve
2872+
// different cubes that require different allocators.
2873+
return this.options.queryFactory.createQuery(cube, this.compilers, { ...this.subQueryOptions(options), paramAllocator: null });
28712874
}
28722875

28732876
return this.newSubQuery(options);
@@ -3522,7 +3525,7 @@ export class BaseQuery {
35223525
if (preAggregation.refreshKey) {
35233526
if (preAggregation.refreshKey.sql) {
35243527
return [
3525-
this.paramAllocator.buildSqlAndParams(
3528+
preAggregationQueryForSql.paramAllocator.buildSqlAndParams(
35263529
preAggregationQueryForSql.evaluateSql(cube, preAggregation.refreshKey.sql)
35273530
).concat({
35283531
external: false,

packages/cubejs-schema-compiler/test/unit/pre-aggregations.test.ts

Lines changed: 96 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { prepareCompiler, prepareYamlCompiler } from './PrepareCompiler';
22
import { createECommerceSchema, createSchemaYaml } from './utils';
3-
import { PostgresQuery } from '../../src';
3+
import { PostgresQuery, queryClass, QueryFactory } from '../../src';
44

55
describe('pre-aggregations', () => {
66
it('rollupJoin scheduledRefresh', async () => {
@@ -9,36 +9,36 @@ describe('pre-aggregations', () => {
99
`
1010
cube(\`Users\`, {
1111
sql: \`SELECT * FROM public.users\`,
12-
12+
1313
preAggregations: {
1414
usersRollup: {
1515
dimensions: [CUBE.id],
1616
},
1717
},
18-
18+
1919
measures: {
2020
count: {
2121
type: \`count\`,
2222
},
2323
},
24-
24+
2525
dimensions: {
2626
id: {
2727
sql: \`id\`,
2828
type: \`string\`,
2929
primaryKey: true,
3030
},
31-
31+
3232
name: {
3333
sql: \`name\`,
3434
type: \`string\`,
3535
},
3636
},
3737
});
38-
38+
3939
cube('Orders', {
4040
sql: \`SELECT * FROM orders\`,
41-
41+
4242
preAggregations: {
4343
ordersRollup: {
4444
measures: [CUBE.count],
@@ -52,20 +52,20 @@ describe('pre-aggregations', () => {
5252
rollups: [Users.usersRollup, CUBE.ordersRollup],
5353
},
5454
},
55-
55+
5656
joins: {
5757
Users: {
5858
relationship: \`belongsTo\`,
5959
sql: \`\${CUBE.userId} = \${Users.id}\`,
6060
},
6161
},
62-
62+
6363
measures: {
6464
count: {
6565
type: \`count\`,
6666
},
6767
},
68-
68+
6969
dimensions: {
7070
id: {
7171
sql: \`id\`,
@@ -232,4 +232,90 @@ describe('pre-aggregations', () => {
232232
expect(indexesSql[0].indexName).toEqual('orders_indexes_orders_by_day_with_day_by_status_regular_index');
233233
expect(indexesSql[1].indexName).toEqual('orders_indexes_orders_by_day_with_day_by_status_agg_index');
234234
});
235+
236+
it('pre-aggregation with FILTER_PARAMS', async () => {
237+
const { compiler, cubeEvaluator, joinGraph } = prepareYamlCompiler(
238+
createSchemaYaml({
239+
cubes: [
240+
{
241+
name: 'orders',
242+
sql_table: 'orders',
243+
measures: [{
244+
name: 'count',
245+
type: 'count',
246+
}],
247+
dimensions: [
248+
{
249+
name: 'created_at',
250+
sql: 'created_at',
251+
type: 'time',
252+
},
253+
{
254+
name: 'updated_at',
255+
sql: '{created_at}',
256+
type: 'time',
257+
},
258+
{
259+
name: 'status',
260+
sql: 'status',
261+
type: 'string',
262+
}
263+
],
264+
preAggregations: [
265+
{
266+
name: 'orders_by_day_with_day',
267+
measures: ['count'],
268+
dimensions: ['status'],
269+
timeDimension: 'CUBE.created_at',
270+
granularity: 'day',
271+
partition_granularity: 'month',
272+
build_range_start: {
273+
sql: 'SELECT \'2022-01-01\'::timestamp',
274+
},
275+
build_range_end: {
276+
sql: 'SELECT \'2024-01-01\'::timestamp'
277+
},
278+
refresh_key: {
279+
every: '4 hours',
280+
sql: `
281+
SELECT max(created_at) as max_created_at
282+
FROM orders
283+
WHERE {FILTER_PARAMS.orders.created_at.filter('date(created_at)')}`,
284+
},
285+
},
286+
]
287+
}
288+
]
289+
})
290+
);
291+
292+
await compiler.compile();
293+
294+
// It's important to provide a queryFactory, as it triggers flow
295+
// with paramAllocator reset in BaseQuery->newSubQueryForCube()
296+
const queryFactory = new QueryFactory(
297+
{
298+
orders: PostgresQuery
299+
}
300+
);
301+
302+
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
303+
measures: [
304+
'orders.count'
305+
],
306+
timeDimensions: [{
307+
dimension: 'orders.created_at',
308+
granularity: 'day',
309+
dateRange: ['2023-01-01', '2023-01-10']
310+
}],
311+
dimensions: ['orders.status'],
312+
queryFactory
313+
});
314+
315+
const preAggregationsDescription: any = query.preAggregations?.preAggregationsDescription();
316+
expect(preAggregationsDescription[0].loadSql[0].includes('WHERE ("orders".created_at >= $1::timestamptz AND "orders".created_at <= $2::timestamptz)')).toBeTruthy();
317+
expect(preAggregationsDescription[0].loadSql[1]).toEqual(['__FROM_PARTITION_RANGE', '__TO_PARTITION_RANGE']);
318+
expect(preAggregationsDescription[0].invalidateKeyQueries[0][0].includes('WHERE ((date(created_at) >= $1::timestamptz AND date(created_at) <= $2::timestamptz))')).toBeTruthy();
319+
expect(preAggregationsDescription[0].invalidateKeyQueries[0][1]).toEqual(['__FROM_PARTITION_RANGE', '__TO_PARTITION_RANGE']);
320+
});
235321
});

0 commit comments

Comments
 (0)