Skip to content

Commit 3b94b2c

Browse files
authored
fix(@cubejs-backend/prestodb-driver): Wrong OFFSET/LIMIT order (#1135)
* Feature: Simplify tests by async/await * Fix: PrestodbQuery - overide groupByDimensionLimit to right OFFSET LIMIT order, #988 * test: Add test for #988 Fixes #988
1 parent 77e0f32 commit 3b94b2c

File tree

3 files changed

+434
-386
lines changed

3 files changed

+434
-386
lines changed

packages/cubejs-schema-compiler/adapter/PrestodbQuery.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class PrestodbFilter extends BaseFilter {
2323
// TODO here can be measure type of string actually
2424
return 'CAST(? AS DOUBLE)';
2525
}
26+
2627
return '?';
2728
}
2829
}
@@ -91,6 +92,13 @@ class PrestodbQuery extends BaseQuery {
9192
countDistinctApprox(sql) {
9293
return `approx_distinct(${sql})`;
9394
}
95+
96+
groupByDimensionLimit() {
97+
const limitClause = this.rowLimit === null ? '' : ` LIMIT ${this.rowLimit && parseInt(this.rowLimit, 10) || 10000}`;
98+
const offsetClause = this.offset ? ` OFFSET ${parseInt(this.offset, 10)}` : '';
99+
100+
return `${offsetClause}${limitClause}`;
101+
}
94102
}
95103

96104
module.exports = PrestodbQuery;

packages/cubejs-schema-compiler/test/integration/postgres/SQLGenerationLogicTest.js

Lines changed: 88 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -315,40 +315,38 @@ describe('SQL Generation', function test() {
315315
});
316316
`);
317317

318-
319-
it('filter with operator OR', () => {
320-
const result = compiler.compile().then(() => {
321-
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
322-
measures: [
323-
'visitor_checkins.google_sourced_checkins'
324-
],
325-
timeDimensions: [],
326-
filters: [
327-
{
328-
or: [
329-
{ dimension: 'cards.id', operator: 'equals', values: ['3'] },
330-
{ dimension: 'cards.id', operator: 'equals', values: ['1'] }
331-
]
332-
},
333-
],
334-
timezone: 'America/Los_Angeles'
335-
});
336-
337-
console.log(query.buildSqlAndParams());
318+
it('filter with operator OR', async () => {
319+
await compiler.compile();
338320

339-
return dbRunner.testQuery(query.buildSqlAndParams()).then(res => {
340-
console.log(JSON.stringify(res));
341-
res.should.be.deepEqual(
342-
[{ 'visitor_checkins__google_sourced_checkins': '1' }]
343-
);
344-
});
321+
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
322+
measures: [
323+
'visitor_checkins.google_sourced_checkins'
324+
],
325+
timeDimensions: [],
326+
filters: [
327+
{
328+
or: [
329+
{ dimension: 'cards.id', operator: 'equals', values: ['3'] },
330+
{ dimension: 'cards.id', operator: 'equals', values: ['1'] }
331+
]
332+
},
333+
],
334+
timezone: 'America/Los_Angeles'
345335
});
346336

347-
return result;
337+
console.log(query.buildSqlAndParams());
338+
339+
return dbRunner.testQuery(query.buildSqlAndParams()).then(res => {
340+
console.log(JSON.stringify(res));
341+
res.should.be.deepEqual(
342+
[{ 'visitor_checkins__google_sourced_checkins': '1' }]
343+
);
344+
});
348345
});
349-
350346

351-
it('having and where filter in same operator OR', () => compiler.compile().then(() => {
347+
it('having and where filter in same operator OR', async () => {
348+
await compiler.compile();
349+
352350
try {
353351
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
354352
'measures': [
@@ -381,15 +379,17 @@ describe('SQL Generation', function test() {
381379
'visitors.source'
382380
]
383381
});
384-
382+
385383
throw new Error();
386384
} catch (error) {
387385
// You cannot use dimension and measure in same condition
388386
error.should.be.instanceof(UserError);
389387
}
390-
}));
391-
392-
it('having filter with operator OR', () => compiler.compile().then(() => {
388+
});
389+
390+
it('having filter with operator OR', async () => {
391+
await compiler.compile();
392+
393393
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
394394
measures: [
395395
'visitors.visitor_count',
@@ -438,10 +438,11 @@ describe('SQL Generation', function test() {
438438
}]
439439
);
440440
});
441-
}));
441+
});
442442

443+
it('having filter with operators OR & AND', async () => {
444+
await compiler.compile();
443445

444-
it('having filter with operators OR & AND', () => compiler.compile().then(() => {
445446
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
446447
measures: [
447448
'visitors.visitor_count',
@@ -508,9 +509,11 @@ describe('SQL Generation', function test() {
508509
}]
509510
);
510511
});
511-
}));
512+
});
513+
514+
it('having filter with operators OR & AND (with filter based on measures not from select part clause)', async () => {
515+
await compiler.compile();
512516

513-
it('having filter with operators OR & AND (with filter based on measures not from select part clause)', () => compiler.compile().then(() => {
514517
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
515518
measures: [
516519
'visitors.visitor_count',
@@ -577,9 +580,11 @@ describe('SQL Generation', function test() {
577580
}]
578581
);
579582
});
580-
}));
583+
});
584+
585+
it('where filter with operators OR & AND', async () => {
586+
await compiler.compile();
581587

582-
it('where filter with operators OR & AND', () => compiler.compile().then(() => {
583588
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
584589
measures: [
585590
'visitors.visitor_count'
@@ -643,9 +648,11 @@ describe('SQL Generation', function test() {
643648
}]
644649
);
645650
});
646-
}));
651+
});
652+
653+
it('where filter with operators OR & AND (with filter based on dimensions not from select part clause)', async () => {
654+
await compiler.compile();
647655

648-
it('where filter with operators OR & AND (with filter based on dimensions not from select part clause)', () => compiler.compile().then(() => {
649656
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
650657
measures: [
651658
'visitors.visitor_count'
@@ -706,9 +713,11 @@ describe('SQL Generation', function test() {
706713
}]
707714
);
708715
});
709-
}));
716+
});
717+
718+
it('where filter with only one argument', async () => {
719+
await compiler.compile();
710720

711-
it('where filter with only one argument', () => compiler.compile().then(() => {
712721
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
713722
measures: [
714723
'visitors.visitor_count'
@@ -766,9 +775,11 @@ describe('SQL Generation', function test() {
766775
}]
767776
);
768777
});
769-
}));
778+
});
779+
780+
it('where filter without arguments', async () => {
781+
await compiler.compile();
770782

771-
it('where filter without arguments', () => compiler.compile().then(() => {
772783
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
773784
measures: [
774785
'visitors.visitor_count'
@@ -812,9 +823,11 @@ describe('SQL Generation', function test() {
812823
console.log(JSON.stringify(res));
813824
res.should.be.deepEqual([{ 'visitors__source': 'some', 'visitors__visitor_count': '2' }]);
814825
});
815-
}));
826+
});
827+
828+
it('where filter without any arguments', async () => {
829+
await compiler.compile();
816830

817-
it('where filter without any arguments', () => compiler.compile().then(() => {
818831
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
819832
measures: [
820833
'visitors.visitor_count'
@@ -858,8 +871,11 @@ describe('SQL Generation', function test() {
858871
]
859872
);
860873
});
861-
}));
862-
it('where filter with incorrect one arguments', () => compiler.compile().then(() => {
874+
});
875+
876+
it('where filter with incorrect one arguments', async () => {
877+
await compiler.compile();
878+
863879
try {
864880
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
865881
measures: [
@@ -874,28 +890,28 @@ describe('SQL Generation', function test() {
874890
{
875891
and: [
876892
{ and: [
877-
{
878-
or: [
879-
{
880-
and: [
881-
{
882-
measure: 'visitors.source',
883-
operator: 'equals',
884-
values: ['some']
885-
}
886-
]
887-
},
888-
{
889-
and: [
890-
{
891-
dimension: 'visitors_source',
892-
operator: 'equals',
893-
values: ['google']
894-
}
895-
]
896-
}
897-
]
898-
}]
893+
{
894+
or: [
895+
{
896+
and: [
897+
{
898+
measure: 'visitors.source',
899+
operator: 'equals',
900+
values: ['some']
901+
}
902+
]
903+
},
904+
{
905+
and: [
906+
{
907+
dimension: 'visitors_source',
908+
operator: 'equals',
909+
values: ['google']
910+
}
911+
]
912+
}
913+
]
914+
}]
899915
}]
900916
}],
901917
order: [{
@@ -907,7 +923,7 @@ describe('SQL Generation', function test() {
907923
} catch (error) {
908924
error.should.be.instanceof(UserError);
909925
}
910-
}));
926+
});
911927

912928
// end of tests
913929
});

0 commit comments

Comments
 (0)