Skip to content

Commit e25e65f

Browse files
authored
feat(clickhouse-driver): Switch from apla-clickhouse to @clickhouse/client (#8928)
Switch client library used to talk to ClickHouse to upstream one. Lots of related changes: * Streaming now does not use streaming JSON parser, so we can't rely on `meta` field in JSON format. Instread it relies on `JSONCompactEachRowWithNamesAndTypes`: first two rows returned should contain names and types. https://clickhouse.com/docs/en/sql-reference/formats#jsoncompacteachrowwithnamesandtypes * Streaming now use async iterators instread of Node.js streams internally. External API returns stream, as before * Pooling moved completely to client library. `generic-pool` is not used at all, `dbMaxPoolSize` is passed to client library to limit open sockets. New client maintains `http.Agent` internally, and have it's own idle timers, looks fine for us. * Queries now does not send `session_id`, as we anyway expect queries to be independent, and don't use session-bound stuff, like temporary tables. Previous behaviour was kind of weird: session ids were attached to client in pool, but for every query it would acquire new client from pool, so nothing could actually utilize same session. * `KILL QUERY` on cancellation now uses separate client instance, to avoid getting stuck on busy pool * `query` method supports only `SELECT` queries, or other queries that return result sets. For DDL queries on this client library one have to use other methods. Because of that more overrides were necessary, like `dropTable`, `createSchemaIfNotExists` or `createTable`. * Driver now respects per-datasource `dbQueryTimeout` config * fix(backend-shared): Rename `convertTimeStrToMs` to `convertTimeStrToSeconds`. It returns input number (which should be seconds) as it is, and 5 for '5s'
1 parent 137de67 commit e25e65f

File tree

9 files changed

+383
-259
lines changed

9 files changed

+383
-259
lines changed

packages/cubejs-backend-shared/src/env.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export class InvalidConfiguration extends Error {
99
}
1010
}
1111

12-
export function convertTimeStrToMs(
12+
export function convertTimeStrToSeconds(
1313
input: string,
1414
envName: string,
1515
description: string = 'Must be a number in seconds or duration string (1s, 1m, 1h).',
@@ -126,7 +126,7 @@ function asBoolOrTime(input: string, envName: string): number | boolean {
126126
return false;
127127
}
128128

129-
return convertTimeStrToMs(
129+
return convertTimeStrToSeconds(
130130
input,
131131
envName,
132132
'Should be boolean or number (in seconds) or string in time format (1s, 1m, 1h)'
@@ -510,7 +510,7 @@ const variables: Record<string, (...args: any) => any> = {
510510
}) => {
511511
const key = keyByDataSource('CUBEJS_DB_POLL_MAX_INTERVAL', dataSource);
512512
const value = process.env[key] || '5s';
513-
return convertTimeStrToMs(value, key);
513+
return convertTimeStrToSeconds(value, key);
514514
},
515515

516516
/**
@@ -525,14 +525,14 @@ const variables: Record<string, (...args: any) => any> = {
525525
const key = keyByDataSource('CUBEJS_DB_POLL_TIMEOUT', dataSource);
526526
const value = process.env[key];
527527
if (value) {
528-
return convertTimeStrToMs(value, key);
528+
return convertTimeStrToSeconds(value, key);
529529
} else {
530530
return null;
531531
}
532532
},
533533

534534
/**
535-
* Query timeout. Currently used in BigQuery, Dremio, Postgres, Snowflake
535+
* Query timeout. Currently used in BigQuery, ClickHouse, Dremio, Postgres, Snowflake
536536
* and Athena drivers and the orchestrator (queues, pre-aggs). For the
537537
* orchestrator this variable did not split by the datasource.
538538
*
@@ -546,7 +546,7 @@ const variables: Record<string, (...args: any) => any> = {
546546
} = {}) => {
547547
const key = keyByDataSource('CUBEJS_DB_QUERY_TIMEOUT', dataSource);
548548
const value = process.env[key] || '10m';
549-
return convertTimeStrToMs(value, key);
549+
return convertTimeStrToSeconds(value, key);
550550
},
551551

552552
/**

packages/cubejs-backend-shared/test/env.test.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
1-
import { getEnv, convertTimeStrToMs } from '../src/env';
1+
import { getEnv, convertTimeStrToSeconds } from '../src/env';
22

33
test('convertTimeStrToMs', () => {
4-
expect(convertTimeStrToMs('1', 'VARIABLE_ENV')).toBe(1);
5-
expect(convertTimeStrToMs('1s', 'VARIABLE_ENV')).toBe(1);
6-
expect(convertTimeStrToMs('5s', 'VARIABLE_ENV')).toBe(5);
7-
expect(convertTimeStrToMs('1m', 'VARIABLE_ENV')).toBe(1 * 60);
8-
expect(convertTimeStrToMs('10m', 'VARIABLE_ENV')).toBe(10 * 60);
9-
expect(convertTimeStrToMs('1h', 'VARIABLE_ENV')).toBe(60 * 60);
10-
expect(convertTimeStrToMs('2h', 'VARIABLE_ENV')).toBe(2 * 60 * 60);
4+
expect(convertTimeStrToSeconds('1', 'VARIABLE_ENV')).toBe(1);
5+
expect(convertTimeStrToSeconds('1s', 'VARIABLE_ENV')).toBe(1);
6+
expect(convertTimeStrToSeconds('5s', 'VARIABLE_ENV')).toBe(5);
7+
expect(convertTimeStrToSeconds('1m', 'VARIABLE_ENV')).toBe(1 * 60);
8+
expect(convertTimeStrToSeconds('10m', 'VARIABLE_ENV')).toBe(10 * 60);
9+
expect(convertTimeStrToSeconds('1h', 'VARIABLE_ENV')).toBe(60 * 60);
10+
expect(convertTimeStrToSeconds('2h', 'VARIABLE_ENV')).toBe(2 * 60 * 60);
1111
});
1212

1313
test('convertTimeStrToMs(exception)', () => {
14-
expect(() => convertTimeStrToMs('', 'VARIABLE_ENV')).toThrowError(
14+
expect(() => convertTimeStrToSeconds('', 'VARIABLE_ENV')).toThrowError(
1515
`Value "" is not valid for VARIABLE_ENV. Must be a number in seconds or duration string (1s, 1m, 1h).`
1616
);
1717
});

packages/cubejs-base-driver/src/BaseDriver.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -614,9 +614,14 @@ export abstract class BaseDriver implements DriverInterface {
614614
return [];
615615
}
616616

617-
public createTable(quotedTableName: string, columns: TableColumn[]) {
617+
// This is only for use in tests
618+
public async createTableRaw(query: string): Promise<void> {
619+
await this.query(query);
620+
}
621+
622+
public async createTable(quotedTableName: string, columns: TableColumn[]): Promise<void> {
618623
const createTableSql = this.createTableSql(quotedTableName, columns);
619-
return this.query(createTableSql, []).catch(e => {
624+
await this.query(createTableSql, []).catch(e => {
620625
e.message = `Error during create table: ${createTableSql}: ${e.message}`;
621626
throw e;
622627
});

packages/cubejs-clickhouse-driver/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,9 @@
2727
"integration:clickhouse": "jest dist/test"
2828
},
2929
"dependencies": {
30-
"@cubejs-backend/apla-clickhouse": "^1.7",
30+
"@clickhouse/client": "^1.7.0",
3131
"@cubejs-backend/base-driver": "1.1.4",
3232
"@cubejs-backend/shared": "1.1.4",
33-
"generic-pool": "^3.6.0",
3433
"moment": "^2.24.0",
3534
"sqlstring": "^2.3.1",
3635
"uuid": "^8.3.2"

0 commit comments

Comments
 (0)