Skip to content

Commit 350a438

Browse files
authored
fix(server-core): Handle empty query in getSqlGenerator (#9270)
api-gateway calls getSqlGenerator with empty query and concrete data source to initialize SQL API But because query is empty `sqlGenerator.dataSource` can be undefined, and it would trigger re-creating query with new data source, which would be `default`
1 parent 5c47335 commit 350a438

File tree

3 files changed

+98
-11
lines changed

3 files changed

+98
-11
lines changed

packages/cubejs-server-core/src/core/CompilerApi.js

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -137,19 +137,26 @@ export class CompilerApi {
137137
throw new Error(`Unknown dbType: ${dbType}`);
138138
}
139139

140+
// sqlGenerator.dataSource can return undefined for query without members
141+
// Queries like this are used by api-gateway to initialize SQL API
142+
// At the same time, those queries should use concrete dataSource, so we should be good to go with it
140143
dataSource = compilers.compiler.withQuery(sqlGenerator, () => sqlGenerator.dataSource);
141-
const _dbType = await this.getDbType(dataSource);
142-
if (dataSource !== 'default' && dbType !== _dbType) {
143-
// TODO consider more efficient way than instantiating query
144-
sqlGenerator = await this.createQueryByDataSource(
145-
compilers,
146-
query,
147-
dataSource,
148-
_dbType
149-
);
144+
if (dataSource !== undefined) {
145+
const _dbType = await this.getDbType(dataSource);
146+
if (dataSource !== 'default' && dbType !== _dbType) {
147+
// TODO consider more efficient way than instantiating query
148+
sqlGenerator = await this.createQueryByDataSource(
149+
compilers,
150+
query,
151+
dataSource,
152+
_dbType
153+
);
150154

151-
if (!sqlGenerator) {
152-
throw new Error(`Can't find dialect for '${dataSource}' data source: ${_dbType}`);
155+
if (!sqlGenerator) {
156+
throw new Error(
157+
`Can't find dialect for '${dataSource}' data source: ${_dbType}`
158+
);
159+
}
153160
}
154161
}
155162

packages/cubejs-testing/test/__snapshots__/smoke-multidb.test.ts.snap

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,21 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`multidb SQL pushdown queries to different data sources: Products 1`] = `
4+
Array [
5+
Object {
6+
"name": "apples",
7+
},
8+
]
9+
`;
10+
11+
exports[`multidb SQL pushdown queries to different data sources: Suppliers 1`] = `
12+
Array [
13+
Object {
14+
"company": "Fruits Inc",
15+
},
16+
]
17+
`;
18+
319
exports[`multidb query: query 1`] = `
420
Array [
521
Object {

packages/cubejs-testing/test/smoke-multidb.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { MysqlDBRunner, PostgresDBRunner } from '@cubejs-backend/testing-shared'
33
import cubejs, { CubeApi } from '@cubejs-client/core';
44
// eslint-disable-next-line import/no-extraneous-dependencies
55
import { afterAll, beforeAll, expect, jest } from '@jest/globals';
6+
import { Client as PgClient } from 'pg';
67
import { BirdBox, getBirdbox } from '../src';
78
import {
89
DEFAULT_API_TOKEN,
@@ -11,12 +12,43 @@ import {
1112
JEST_BEFORE_ALL_DEFAULT_TIMEOUT,
1213
} from './smoke-tests';
1314

15+
// TODO: Random port?
16+
const pgPort = 5656;
17+
let connectionId = 0;
18+
19+
async function createPostgresClient(user: string, password: string) {
20+
connectionId++;
21+
const currentConnId = connectionId;
22+
23+
console.debug(`[pg] new connection ${currentConnId}`);
24+
25+
const conn = new PgClient({
26+
database: 'db',
27+
port: pgPort,
28+
host: '127.0.0.1',
29+
user,
30+
password,
31+
ssl: false,
32+
});
33+
conn.on('error', (err) => {
34+
console.log(err);
35+
});
36+
conn.on('end', () => {
37+
console.debug(`[pg] end ${currentConnId}`);
38+
});
39+
40+
await conn.connect();
41+
42+
return conn;
43+
}
44+
1445
describe('multidb', () => {
1546
jest.setTimeout(60 * 5 * 1000);
1647
let db: StartedTestContainer;
1748
let db2: StartedTestContainer;
1849
let birdbox: BirdBox;
1950
let client: CubeApi;
51+
let connection: PgClient;
2052

2153
beforeAll(async () => {
2254
db = await PostgresDBRunner.startContainer({});
@@ -39,6 +71,9 @@ describe('multidb', () => {
3971
CUBEJS_DB_USER2: 'root',
4072
CUBEJS_DB_PASS2: 'Test1test',
4173

74+
CUBEJS_PG_SQL_PORT: `${pgPort}`,
75+
CUBESQL_SQL_PUSH_DOWN: 'true',
76+
4277
...DEFAULT_CONFIG,
4378
},
4479
{
@@ -49,6 +84,7 @@ describe('multidb', () => {
4984
client = cubejs(async () => DEFAULT_API_TOKEN, {
5085
apiUrl: birdbox.configuration.apiUrl,
5186
});
87+
connection = await createPostgresClient('admin', 'admin_password');
5288
}, JEST_BEFORE_ALL_DEFAULT_TIMEOUT);
5389

5490
afterAll(async () => {
@@ -69,4 +105,32 @@ describe('multidb', () => {
69105
});
70106
expect(response.rawData()).toMatchSnapshot('query');
71107
});
108+
109+
test('SQL pushdown queries to different data sources: Products', async () => {
110+
const res = await connection.query(`
111+
SELECT
112+
name
113+
FROM
114+
Products
115+
WHERE
116+
LOWER(name) = 'apples'
117+
GROUP BY
118+
1
119+
`);
120+
expect(res.rows).toMatchSnapshot();
121+
});
122+
123+
test('SQL pushdown queries to different data sources: Suppliers', async () => {
124+
const res = await connection.query(`
125+
SELECT
126+
company
127+
FROM
128+
Suppliers
129+
WHERE
130+
LOWER(company) = 'fruits inc'
131+
GROUP BY
132+
1
133+
`);
134+
expect(res.rows).toMatchSnapshot();
135+
});
72136
});

0 commit comments

Comments
 (0)