Skip to content

Commit

Permalink
fix(schema-compiler): fix Maximum call stack size exceeded if FILTER_…
Browse files Browse the repository at this point in the history
…PARAMS are used inside dimensions/measures (#8867)

* fix(schema-compiler): fix Maximum call stack size exceeded if FILTER_PARAMS are used inside dimensions/measures

* add tests

* move guard to the filter_params proxy
  • Loading branch information
KSDaemon authored Nov 13, 2024
1 parent 211d1c1 commit 8e7c5c7
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 12 deletions.
16 changes: 13 additions & 3 deletions packages/cubejs-schema-compiler/src/adapter/BaseQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -2240,6 +2240,7 @@ export class BaseQuery {
this.safeEvaluateSymbolContext().memberChildren[parentMember].push(memberPath);
}
}

this.safeEvaluateSymbolContext().currentMember = memberPath;
try {
if (type === 'measure') {
Expand Down Expand Up @@ -3882,9 +3883,16 @@ export class BaseQuery {
// collectFrom() -> traverseSymbol() -> evaluateSymbolSql() ->
// evaluateSql() -> resolveSymbolsCall() -> cubeReferenceProxy->toString() ->
// evaluateSymbolSql() -> evaluateSql()... -> and got here again
//
// When FILTER_PARAMS is used in dimension/measure SQL - we also hit recursive loop:
// allBackAliasMembersExceptSegments() -> collectFrom() -> traverseSymbol() -> evaluateSymbolSql() ->
// autoPrefixAndEvaluateSql() -> evaluateSql() -> filterProxyFromAllFilters->Proxy->toString()
// and so on...
// For this case aliasGathering flag is added to the context in first iteration and
// is checked below to prevent looping.
const aliases = allFilters ?
allFilters
.map(v => (v.query ? v.query.allBackAliasMembersExceptSegments() : {}))
.map(v => (v.query && !v.query.safeEvaluateSymbolContext().aliasGathering ? v.query.allBackAliasMembersExceptSegments() : {}))
.reduce((a, b) => ({ ...a, ...b }), {})
: {};
// Filtering aliases that somehow relate to this group member
Expand Down Expand Up @@ -3932,8 +3940,10 @@ export class BaseQuery {
const query = this;
return members.map(
member => {
const collectedMembers = query
.collectFrom([member], query.collectMemberNamesFor.bind(query), 'collectMemberNamesFor');
const collectedMembers = query.evaluateSymbolSqlWithContext(
() => query.collectFrom([member], query.collectMemberNamesFor.bind(query), 'collectMemberNamesFor'),
{ aliasGathering: true }
);
const memberPath = member.expressionPath();
let nonAliasSeen = false;
return collectedMembers
Expand Down
87 changes: 78 additions & 9 deletions packages/cubejs-schema-compiler/test/unit/base-query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1586,15 +1586,37 @@ describe('SQL Generation', () => {
cubes: [{
name: 'Order',
sql: 'select * from order where {FILTER_PARAMS.Order.type.filter(\'type\')}',
measures: [{
name: 'count',
type: 'count',
}],
dimensions: [{
name: 'type',
sql: 'type',
type: 'string'
}]
measures: [
{
name: 'count',
type: 'count',
},
{
name: 'avg_filtered',
sql: 'product_id',
type: 'avg',
filters: [
{ sql: `{FILTER_PARAMS.Order.category.filter('category')}` }
]
}
],
dimensions: [
{
name: 'type',
sql: 'type',
type: 'string'
},
{
name: 'category',
sql: 'category',
type: 'string'
},
{
name: 'proxied',
sql: `{FILTER_PARAMS.Order.type.filter("x => type = 'online'")}`,
type: 'boolean',
}
]
}],
views: [{
name: 'orders_view',
Expand Down Expand Up @@ -1829,6 +1851,53 @@ describe('SQL Generation', () => {
const queryString = queryAndParams[0];
expect(/select\s+\*\s+from\s+order\s+where\s+\(\(type\s=\s\?\)\)/.test(queryString)).toBeTruthy();
});

it('correctly substitute filter params in cube\'s query dimension used in filter', async () => {
await compilers.compiler.compile();
const query = new BaseQuery(compilers, {
measures: ['Order.count'],
dimensions: ['Order.proxied'],
filters: [
{
member: 'Order.proxied',
operator: 'equals',
values: [true],
},
],
});
const queryAndParams = query.buildSqlAndParams();
const queryString = queryAndParams[0];
expect(queryString).toContain(`SELECT
(1 = 1) "order__proxied", count(*) "order__count"
FROM
(select * from order where (1 = 1)) AS "order" WHERE ((1 = 1) = ?)`);
});

it('correctly substitute filter params in cube\'s query measure used in filter', async () => {
await compilers.compiler.compile();
const query = new BaseQuery(compilers, {
measures: ['Order.avg_filtered'],
dimensions: ['Order.type'],
filters: [
{
member: 'Order.type',
operator: 'equals',
values: ['online'],
},
{
member: 'Order.category',
operator: 'equals',
values: ['category'],
},
],
});
const queryAndParams = query.buildSqlAndParams();
const queryString = queryAndParams[0];
expect(queryString).toContain(`SELECT
"order".type "order__type", avg(CASE WHEN ((category = ?)) THEN "order".product_id END) "order__avg_filtered"
FROM
(select * from order where (type = ?)) AS "order" WHERE ("order".type = ?) AND ("order".category = ?)`);
});
});

describe('FILTER_GROUP', () => {
Expand Down

0 comments on commit 8e7c5c7

Please sign in to comment.