Skip to content

Commit 8ca5234

Browse files
paveltiunovpaveltiunov
authored
fix: LIMIT is not enforced (#6586)
* fix: Limit is not enforced * Fix unit test
1 parent 48af997 commit 8ca5234

File tree

7 files changed

+190
-32
lines changed

7 files changed

+190
-32
lines changed

‎packages/cubejs-api-gateway/src/gateway.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,13 +1146,13 @@ class ApiGateway {
11461146

11471147
if (!persistent) {
11481148
if (
1149-
typeof query.limit === 'number' &&
1150-
query.limit > getEnv('dbQueryLimit')
1149+
typeof query.rowLimit === 'number' &&
1150+
query.rowLimit > getEnv('dbQueryLimit')
11511151
) {
11521152
throw new Error('The query limit has been exceeded.');
11531153
}
1154-
query.limit = typeof query.limit === 'number'
1155-
? query.limit
1154+
query.rowLimit = typeof query.rowLimit === 'number'
1155+
? query.rowLimit
11561156
: def;
11571157
}
11581158
}

‎packages/cubejs-api-gateway/test/index.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ describe('API Gateway', () => {
259259
timezone: 'UTC',
260260
order: [],
261261
filters: [],
262-
limit: 10000,
262+
rowLimit: 10000,
263263
dimensions: [],
264264
timeDimensions: [],
265265
queryType: 'regularQuery'
@@ -271,7 +271,7 @@ describe('API Gateway', () => {
271271
timezone: 'UTC',
272272
order: [],
273273
filters: [],
274-
limit: 10000,
274+
rowLimit: 10000,
275275
dimensions: [],
276276
timeDimensions: [],
277277
queryType: 'regularQuery'

‎packages/cubejs-schema-compiler/src/adapter/PreAggregations.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,7 @@ export class PreAggregations {
961961
refreshRangeQuery() {
962962
return this.query.newSubQuery({
963963
rowLimit: null,
964+
offset: null,
964965
preAggregationQuery: true,
965966
});
966967
}
@@ -970,6 +971,7 @@ export class PreAggregations {
970971
cube,
971972
{
972973
rowLimit: null,
974+
offset: null,
973975
timeDimensions: aggregation.partitionTimeDimensions,
974976
preAggregationQuery: true,
975977
}
@@ -983,6 +985,7 @@ export class PreAggregations {
983985
cube,
984986
{
985987
rowLimit: null,
988+
offset: null,
986989
measures: references.measures,
987990
dimensions: references.dimensions,
988991
timeDimensions: this.mergePartitionTimeDimensions(references, aggregation.partitionTimeDimensions),
@@ -999,6 +1002,7 @@ export class PreAggregations {
9991002
cube,
10001003
{
10011004
rowLimit: null,
1005+
offset: null,
10021006
measures: aggregation.measures,
10031007
dimensions: aggregation.dimensions,
10041008
timeDimensions:

‎packages/cubejs-testing-drivers/fixtures/snowflake.json‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,37 @@
3535
},
3636
"preAggregations": {
3737
"Products": [],
38-
"Customers": [],
39-
"ECommerce": []
38+
"Customers": [
39+
{
40+
"name": "RA",
41+
"measures": ["CUBE.count", "CUBE.runningTotal"]
42+
}
43+
],
44+
"ECommerce": [
45+
{
46+
"name": "SA",
47+
"dimensions": ["CUBE.productName"],
48+
"measures": [
49+
"CUBE.totalQuantity",
50+
"CUBE.avgDiscount",
51+
"CUBE.totalSales",
52+
"CUBE.totalProfit"
53+
]
54+
},
55+
{
56+
"name": "TA",
57+
"time_dimension": "CUBE.orderDate",
58+
"granularity": "month",
59+
"partition_granularity": "month",
60+
"dimensions": ["CUBE.productName"],
61+
"measures": [
62+
"CUBE.totalQuantity",
63+
"CUBE.avgDiscount",
64+
"CUBE.totalSales",
65+
"CUBE.totalProfit"
66+
]
67+
}
68+
]
4069
},
4170
"skip": [
4271
"---------------------------------------",
@@ -53,13 +82,6 @@
5382
"for the ECommerce.TimeAnalysisInternal",
5483
"for the ECommerce.TimeAnalysisExternal",
5584

56-
"---------------------------------------",
57-
"Full tests ",
58-
"---------------------------------------",
59-
"must built pre-aggregations",
60-
"querying Customers: dimentions + order + total + offset",
61-
"querying ECommerce: dimentions + order + total + offset",
62-
6385
"---------------------------------------",
6486
"SKIPED FOR ALL ",
6587
"---------------------------------------",

‎packages/cubejs-testing-drivers/src/helpers/getComposePath.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { getFixtures } from './getFixtures';
77
/**
88
* Returns docker compose file by data source type.
99
*/
10-
export function getComposePath(type: string): [path: string, file: string] {
10+
export function getComposePath(type: string, isLocal: boolean): [path: string, file: string] {
1111
const _path = path.resolve(process.cwd(), './.temp');
1212
const _file = `${type}-compose.yaml`;
1313
const { cube, data } = getFixtures(type);
@@ -27,22 +27,24 @@ export function getComposePath(type: string): [path: string, file: string] {
2727
const compose: any = {
2828
version: '2.2',
2929
services: {
30-
cube: {
31-
...cube,
32-
container_name: 'cube',
33-
image: 'cubejs/cube:testing-drivers',
34-
depends_on,
35-
links,
36-
volumes,
37-
restart: 'always',
38-
},
30+
...(!isLocal ? {
31+
cube: {
32+
...cube,
33+
container_name: 'cube',
34+
image: 'cubejs/cube:testing-drivers',
35+
depends_on,
36+
links,
37+
volumes,
38+
restart: 'always',
39+
}
40+
} : {}),
3941
store: {
4042
container_name: 'store',
4143
image: 'cubejs/cubestore:latest',
4244
ports: ['3030'],
4345
restart: 'always',
4446
}
45-
},
47+
}
4648
};
4749
if (data) {
4850
compose.services.data = {

‎packages/cubejs-testing-drivers/src/helpers/runEnvironment.ts‎

Lines changed: 107 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import * as fs from 'fs-extra';
22
import * as path from 'path';
3+
import { ChildProcess, ChildProcessWithoutNullStreams, spawn } from 'child_process';
34
import { config } from 'dotenv';
5+
import yargs from 'yargs/yargs';
46
import { DockerComposeEnvironment } from 'testcontainers';
7+
import { pausePromise } from '@cubejs-backend/shared';
58
import { getFixtures } from './getFixtures';
69
import { getTempPath } from './getTempPath';
710
import { getComposePath } from './getComposePath';
@@ -10,13 +13,94 @@ import { getPackageJsonPath } from './getPackageJsonPath';
1013
import { getSchemaPath } from './getSchemaPath';
1114
import { Environment } from '../types/Environment';
1215

16+
interface CubeEnvironment {
17+
withStartupTimeout(startupTimeout: number): this;
18+
withEnvironment(environment: { [key in string]: string; }): this;
19+
up(): Promise<void>;
20+
down(): Promise<void>;
21+
}
22+
23+
class CubeCliEnvironment implements CubeEnvironment {
24+
public cli: ChildProcessWithoutNullStreams | null = null;
25+
26+
private env: any = {};
27+
28+
public constructor(private dir: string) {
29+
}
30+
31+
public async up(): Promise<void> {
32+
try {
33+
this.cli = spawn(
34+
path.resolve(process.cwd(), '../cubejs-server/bin/server'),
35+
[],
36+
{
37+
cwd: this.dir,
38+
shell: true,
39+
detached: true,
40+
stdio: [
41+
'pipe',
42+
'pipe',
43+
'pipe',
44+
],
45+
env: {
46+
...process.env, ...this.env
47+
},
48+
}
49+
);
50+
if (this.cli.stdout) {
51+
this.cli.stdout.on('data', (msg) => {
52+
process.stdout.write(msg);
53+
});
54+
}
55+
if (this.cli.stderr) {
56+
this.cli.stderr.on('data', (msg) => {
57+
process.stdout.write(msg);
58+
});
59+
}
60+
await pausePromise(10 * 1000);
61+
} catch (e) {
62+
process.stdout.write(`Error spawning cube: ${e}\n`);
63+
}
64+
}
65+
66+
public withEnvironment(environment: { [key in string]: string }): this {
67+
this.env = { ...this.env, ...environment };
68+
return this;
69+
}
70+
71+
public withStartupTimeout(startupTimeout: number): this {
72+
return this;
73+
}
74+
75+
public async down() {
76+
if (this.cli) {
77+
process.kill(-this.cli.pid, 'SIGINT');
78+
process.stdout.write('Cube Exited\n');
79+
}
80+
}
81+
}
82+
1383
export async function runEnvironment(type: string, suf?: string): Promise<Environment> {
1484
const fixtures = getFixtures(type);
1585
getTempPath();
1686
getSchemaPath(type, suf);
1787
getCubeJsPath(type);
1888
getPackageJsonPath(type);
19-
const [composePath, composeFile] = getComposePath(type);
89+
const { mode } = yargs(process.argv.slice(2))
90+
.exitProcess(false)
91+
.options({
92+
mode: {
93+
describe: 'Determines test mode',
94+
choices: [
95+
'local',
96+
'docker'
97+
],
98+
default: 'docker',
99+
}
100+
})
101+
.argv;
102+
const isLocal = mode === 'local';
103+
const [composePath, composeFile] = getComposePath(type, isLocal);
20104
const compose = new DockerComposeEnvironment(
21105
composePath,
22106
composeFile,
@@ -46,16 +130,30 @@ export async function runEnvironment(type: string, suf?: string): Promise<Enviro
46130
}
47131
});
48132
const environment = await compose.up();
49-
const cube = {
133+
134+
const store = {
135+
port: environment.getContainer('store').getMappedPort(3030),
136+
logs: await environment.getContainer('store').logs(),
137+
};
138+
139+
const cliEnv = isLocal ? new CubeCliEnvironment(composePath) : null;
140+
if (cliEnv) {
141+
cliEnv.withEnvironment({
142+
CUBEJS_CUBESTORE_HOST: '127.0.0.1',
143+
CUBEJS_CUBESTORE_PORT: `${store.port}`,
144+
});
145+
await cliEnv.up();
146+
}
147+
const cube = cliEnv ? {
148+
port: 4000,
149+
logs: cliEnv.cli?.stdout || process.stdout
150+
} : {
50151
port: environment.getContainer('cube').getMappedPort(
51152
parseInt(fixtures.cube.ports[0], 10),
52153
),
53154
logs: await environment.getContainer('cube').logs(),
54155
};
55-
const store = {
56-
port: environment.getContainer('store').getMappedPort(3030),
57-
logs: await environment.getContainer('store').logs(),
58-
};
156+
59157
if (fixtures.data) {
60158
const data = {
61159
port: environment.getContainer('data').getMappedPort(
@@ -77,6 +175,9 @@ export async function runEnvironment(type: string, suf?: string): Promise<Enviro
77175
store,
78176
stop: async () => {
79177
await environment.down({ timeout: 30 * 1000 });
178+
if (cliEnv) {
179+
await cliEnv.down();
180+
}
80181
},
81182
};
82183
}

‎packages/cubejs-testing-drivers/test/__snapshots__/snowflake-full.test.ts.snap‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2579,6 +2579,15 @@ Array [
25792579
]
25802580
`;
25812581

2582+
exports[`Queries with the @cubejs-backend/snowflake-driver querying Customers: dimentions + order + total + offset 1`] = `
2583+
Array [
2584+
Object {
2585+
"Customers.customerId": "BS-11380",
2586+
"Customers.customerName": "Customer 9",
2587+
},
2588+
]
2589+
`;
2590+
25822591
exports[`Queries with the @cubejs-backend/snowflake-driver querying Customers: dimentions + order 1`] = `
25832592
Array [
25842593
Object {
@@ -4156,6 +4165,26 @@ Array [
41564165
]
41574166
`;
41584167

4168+
exports[`Queries with the @cubejs-backend/snowflake-driver querying ECommerce: dimentions + order + total + offset 1`] = `
4169+
Array [
4170+
Object {
4171+
"ECommerce.category": "Office Supplies",
4172+
"ECommerce.city": "Bowling",
4173+
"ECommerce.customerId": "BS-11380",
4174+
"ECommerce.customerName": "Customer 9",
4175+
"ECommerce.discount": "0.2",
4176+
"ECommerce.orderDate": "2020-11-16T00:00:00.000",
4177+
"ECommerce.orderId": "CA-2017-160633",
4178+
"ECommerce.productName": "Panasonic KP-380BK Classic Electric Pencil Sharpener",
4179+
"ECommerce.profit": "5.3970",
4180+
"ECommerce.quantity": "3",
4181+
"ECommerce.rowId": "9619",
4182+
"ECommerce.sales": "86.352",
4183+
"ECommerce.subCategory": "Art",
4184+
},
4185+
]
4186+
`;
4187+
41594188
exports[`Queries with the @cubejs-backend/snowflake-driver querying ECommerce: dimentions + order 1`] = `
41604189
Array [
41614190
Object {

0 commit comments

Comments
 (0)