Skip to content
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

feat(cubesql): Group By Rollup support #8281

Merged
merged 29 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/drivers-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -268,4 +268,4 @@ jobs:
run: |
cd ./packages/cubejs-testing-drivers
export DEBUG=testcontainers
yarn ${{ matrix.database }}-full
yarn ${{ matrix.database }}-full
8 changes: 8 additions & 0 deletions packages/cubejs-api-gateway/src/gateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@

protected readonly playgroundAuthSecret?: string;

protected readonly event: (name: string, props?: object) => void;

Check warning on line 148 in packages/cubejs-api-gateway/src/gateway.ts

View workflow job for this annotation

GitHub Actions / lint

'name' is defined but never used. Allowed unused args must match /^_.*/u

Check warning on line 148 in packages/cubejs-api-gateway/src/gateway.ts

View workflow job for this annotation

GitHub Actions / lint

'props' is defined but never used. Allowed unused args must match /^_.*/u

public constructor(
protected readonly apiSecret: string,
Expand Down Expand Up @@ -178,7 +178,7 @@
this.contextRejectionMiddleware = options.contextRejectionMiddleware || (async (req, res, next) => next());
this.wsContextAcceptor = options.wsContextAcceptor || (() => ({ accepted: true }));
// eslint-disable-next-line @typescript-eslint/no-empty-function
this.event = options.event || function () {};

Check warning on line 181 in packages/cubejs-api-gateway/src/gateway.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected unnamed function
}

public initApp(app: ExpressApplication) {
Expand Down Expand Up @@ -1274,12 +1274,20 @@
const obj = JSON.parse(memberExpression);
const args = obj.cube_params;
args.push(`return \`${obj.expr}\``);

const groupingSet = obj.grouping_set ? {
groupType: obj.grouping_set.group_type,
id: obj.grouping_set.id,
subId: obj.grouping_set.sub_id ? obj.grouping_set.sub_id : undefined
} : undefined;

return {
cubeName: obj.cube_name,
name: obj.alias,
expressionName: obj.alias,
expression: Function.constructor.apply(null, args),
definition: memberExpression,
groupingSet,
};
} else {
return memberExpression;
Expand Down Expand Up @@ -1726,7 +1734,7 @@
const {
context,
res,
apiType = 'sql',

Check warning on line 1737 in packages/cubejs-api-gateway/src/gateway.ts

View workflow job for this annotation

GitHub Actions / lint

'apiType' is assigned a value but never used. Allowed unused vars must match /^_.*/u
} = request;
const requestStarted = new Date();

Expand Down
5 changes: 5 additions & 0 deletions packages/cubejs-api-gateway/src/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ const memberExpression = Joi.object().keys({
name: Joi.string().required(),
expressionName: Joi.string(),
definition: Joi.string(),
groupingSet: Joi.object().keys({
groupType: Joi.valid('Rollup', 'Cube').required(),
id: Joi.number().required(),
subId: Joi.number()
})
});

const operators = [
Expand Down
7 changes: 7 additions & 0 deletions packages/cubejs-api-gateway/src/types/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
TimeMember,
FilterOperator,
QueryTimeDimensionGranularity,
QueryOrderType,

Check warning on line 13 in packages/cubejs-api-gateway/src/types/query.ts

View workflow job for this annotation

GitHub Actions / lint

'QueryOrderType' is defined but never used. Allowed unused vars must match /^_.*/u
} from './strings';
import { ResultType } from './enums';

Expand Down Expand Up @@ -39,12 +39,19 @@
or: (QueryFilter | LogicalAndFilter)[]
};

type GroupingSet = {
groupType: string,
id: number,
subId?: null | number
};

type MemberExpression = {
expression: Function;
cubeName: string;
name: string;
expressionName: string;
definition: string;
groupingSet?: GroupingSet
};

/**
Expand Down
12 changes: 6 additions & 6 deletions packages/cubejs-backend-native/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

58 changes: 56 additions & 2 deletions packages/cubejs-schema-compiler/src/adapter/BaseQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -1899,7 +1899,11 @@ export class BaseQuery {
return '';
}
const dimensionColumns = this.dimensionColumns();
return dimensionColumns.length ? ` GROUP BY ${dimensionColumns.map((c, i) => `${i + 1}`).join(', ')}` : '';
if (!dimensionColumns.length) {
return '';
}
const dimensionNames = dimensionColumns.map((c, i) => `${i + 1}`);
return this.rollupGroupByClause(dimensionNames);
}

getFieldIndex(id) {
Expand Down Expand Up @@ -1993,6 +1997,53 @@ export class BaseQuery {
return this.limitOffsetClause(limit, offset);
}

/**
* @protected
* @param {Array<string>} dimensionNames
* @returns {string}
*/
rollupGroupByClause(dimensionNames) {
if (this.ungrouped) {
return '';
}
const dimensionColumns = this.dimensionColumns();
if (!dimensionColumns.length) {
return '';
}

const groupingSets = R.flatten(this.dimensionsForSelect().map(d => d.dimension).filter(d => !!d)).map(d => d.groupingSet);

let result = ' GROUP BY ';

dimensionColumns.forEach((c, i) => {
const groupingSet = groupingSets[i];
const comma = i > 0 ? ', ' : '';
const prevId = i > 0 ? (groupingSets[i - 1] || { id: null }).id : null;
const currId = (groupingSet || { id: null }).id;

if (prevId !== null && currId !== prevId) {
result += ')';
}

if ((prevId === null || currId !== prevId) && groupingSet != null) {
if (groupingSet.groupType === 'Rollup') {
result += `${comma}ROLLUP(`;
} else if (groupingSet.groupType === 'Cube') {
result += `${comma}CUBE(`;
}
} else {
result += `${comma}`;
}

result += dimensionNames[i];
});
if (groupingSets[groupingSets.length - 1] != null) {
result += ')';
}

return result;
}

/**
* @protected
* @param limit
Expand Down Expand Up @@ -2937,10 +2988,11 @@ export class BaseQuery {
'{{ from | indent(2, true) }}\n' +
') AS {{ from_alias }}{% endif %}' +
'{% if filter %}\nWHERE {{ filter }}{% endif %}' +
'{% if group_by %}\nGROUP BY {{ group_by | map(attribute=\'index\') | join(\', \') }}{% endif %}' +
'{% if group_by %}\nGROUP BY {{ group_by }}{% endif %}' +
'{% if order_by %}\nORDER BY {{ order_by | map(attribute=\'expr\') | join(\', \') }}{% endif %}' +
'{% if limit %}\nLIMIT {{ limit }}{% endif %}' +
'{% if offset %}\nOFFSET {{ offset }}{% endif %}',
group_by_exprs: '{{ group_by | map(attribute=\'index\') | join(\', \') }}',
},
expressions: {
column_aliased: '{{expr}} {{quoted_alias}}',
Expand All @@ -2954,6 +3006,8 @@ export class BaseQuery {
in_list: '{{ expr }} {% if negated %}NOT {% endif %}IN ({{ in_exprs_concat }})',
subquery: '({{ expr }})',
in_subquery: '{{ expr }} {% if negated %}NOT {% endif %}IN {{ subquery_expr }}',
rollup: 'ROLLUP({{ exprs_concat }})',
cube: 'CUBE({{ exprs_concat }})',
negative: '-({{ expr }})',
not: 'NOT ({{ expr }})',
true: 'TRUE',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class PrestodbQuery extends BaseQuery {
templates.functions.DATEPART = 'DATE_PART({{ args_concat }})';
templates.statements.select = 'SELECT {{ select_concat | map(attribute=\'aliased\') | join(\', \') }} \n' +
'FROM (\n {{ from }}\n) AS {{ from_alias }} \n' +
'{% if group_by %} GROUP BY {{ group_by | map(attribute=\'index\') | join(\', \') }}{% endif %}' +
'{% if group_by %} GROUP BY {{ group_by }}{% endif %}' +
'{% if order_by %} ORDER BY {{ order_by | map(attribute=\'expr\') | join(\', \') }}{% endif %}' +
'{% if offset %}\nOFFSET {{ offset }}{% endif %}' +
'{% if limit %}\nLIMIT {{ limit }}{% endif %}';
Expand Down
9 changes: 8 additions & 1 deletion packages/cubejs-testing-drivers/fixtures/athena.json
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,15 @@
"querying BigECommerce: null sum",
"querying BigECommerce: null boolean",
"--------------------",

"week granularity is not supported for intervals",
"--------------------",
"querying BigECommerce: rolling window by 2 week"
"querying BigECommerce: rolling window by 2 week",

"SKIPPED SQL API (Need work)",
"---------------------------------------",
"SQL API: Simple Rollup",
"SQL API: Complex Rollup",
"SQL API: Nested Rollup"
]
}
3 changes: 2 additions & 1 deletion packages/cubejs-testing-drivers/fixtures/bigquery.json
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@

"SKIPPED SQL API (Need work)",
"---------------------------------------",
"SQL API: reuse params"
"SQL API: reuse params",
"SQL API: Complex Rollup"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@
"querying Products: dimensions -- doesn't work wo ordering",
"querying ECommerce: total quantity, avg discount, total sales, total profit by product + order + total -- rounding in athena",
"querying ECommerce: total sales, total profit by month + order (date) + total -- doesn't work with the BigQuery",
"querying ECommerce: total quantity, avg discount, total sales, total profit by product + order + total -- noisy test"
"querying ECommerce: total quantity, avg discount, total sales, total profit by product + order + total -- noisy test",

"SKIPPED SQL API (Need work)",
"---------------------------------------",
"SQL API: Nested Rollup"
]
}
}
5 changes: 4 additions & 1 deletion packages/cubejs-testing-drivers/fixtures/mssql.json
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@
"SQL API: reuse params",
"SQL API: post-aggregate percentage of total",
"SQL API: powerbi min max ungrouped flag",
"SQL API: powerbi min max push down"
"SQL API: powerbi min max push down",
"SQL API: Simple Rollup",
"SQL API: Complex Rollup",
"SQL API: Nested Rollup"
]
}
5 changes: 4 additions & 1 deletion packages/cubejs-testing-drivers/fixtures/mysql.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,9 @@

"SKIPPED SQL API (Need work)",
"---------------------------------------",
"SQL API: reuse params"
"SQL API: reuse params",
"SQL API: Simple Rollup",
"SQL API: Complex Rollup",
"SQL API: Nested Rollup"
]
}
49 changes: 49 additions & 0 deletions packages/cubejs-testing-drivers/src/tests/testQueries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1628,5 +1628,54 @@ from
`);
expect(res.rows).toMatchSnapshot('reuse_params');
});

executePg('SQL API: Simple Rollup', async () => {
const connection = await createPostgresClient();
const res = await connection.query(`
select
rowId, orderId, orderDate, sum(count)
from
"ECommerce" as "ECommerce"
group by
ROLLUP(1, 2, 3)
order by 1, 2, 3

`);
expect(res.rows).toMatchSnapshot('simple_rollup');
});

executePg('SQL API: Complex Rollup', async () => {
const connection = await createPostgresClient();
const res = await connection.query(`
select
rowId, orderId, orderDate, city, sum(count)
from
"ECommerce" as "ECommerce"
group by
ROLLUP(1, 2), 3, ROLLUP(4)
order by 1, 2, 3, 4

`);
expect(res.rows).toMatchSnapshot('complex_rollup');
});
executePg('SQL API: Nested Rollup', async () => {
const connection = await createPostgresClient();
const res = await connection.query(`
select rowId, orderId, orderDate, sum(cnt)
from (
select
rowId, orderId, orderDate, sum(count) as cnt
from
"ECommerce" as "ECommerce"
group by 1, 2, 3

) a
group by
ROLLUP(1, 2, 3)
order by 1, 2, 3

`);
expect(res.rows).toMatchSnapshot('nested_rollup');
});
});
}