Skip to content

Commit 79d6620

Browse files
committed
[FIXUP] more fixes re config compat
1 parent 10ba31d commit 79d6620

File tree

1 file changed

+26
-88
lines changed

1 file changed

+26
-88
lines changed

packages/cubejs-clickhouse-driver/src/ClickHouseDriver.ts

Lines changed: 26 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import {
2828
import * as process from "node:process";
2929
import { Readable } from 'node:stream';
3030
import { ClickHouseClient, createClient } from '@clickhouse/client';
31-
import type { ResponseJSON } from '@clickhouse/client';
31+
import type { ClickHouseSettings, ResponseJSON } from '@clickhouse/client';
3232
import genericPool from 'generic-pool';
3333
import type { Factory, Pool } from 'generic-pool';
3434
import { v4 as uuidv4 } from 'uuid';
@@ -64,12 +64,11 @@ const ClickhouseTypeToGeneric: Record<string, string> = {
6464
export interface ClickHouseDriverOptions {
6565
host?: string,
6666
port?: string,
67-
auth?: string,
67+
username?: string,
68+
password?: string,
6869
protocol?: string,
6970
database?: string,
7071
readOnly?: boolean,
71-
// TODO how to treat this in a BC way?
72-
// queryOptions?: object,
7372

7473
/**
7574
* Data source name.
@@ -107,11 +106,9 @@ type ClickHouseDriverConfig = {
107106
username: string,
108107
password: string,
109108
readOnly: boolean,
110-
// TODO why separate object?
111-
queryOptions: {
112-
database: string,
113-
},
109+
database: string,
114110
exportBucket: ClickhouseDriverExportAWS | null,
111+
clickhouseSettings: ClickHouseSettings,
115112
};
116113

117114
export class ClickHouseDriver extends BaseDriver implements DriverInterface {
@@ -140,75 +137,44 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
140137
testConnectionTimeout: config.testConnectionTimeout,
141138
});
142139

143-
const dataSource =
144-
config.dataSource ||
145-
assertDataSource('default');
146-
147-
// TODO recheck everything in config for new driver
140+
const dataSource = config.dataSource ?? assertDataSource('default');
148141
const host = config.host ?? getEnv('dbHost', { dataSource });
149142
const port = config.port ?? getEnv('dbPort', { dataSource }) ?? 8123;
150143
const protocol = config.protocol ?? getEnv('dbSsl', { dataSource }) ? 'https:' : 'http:';
151-
// TODO proper value here, with proper back compat, and treating protocol
152144
const url = `${protocol}//${host}:${port}`;
153145
// TODO drop this
154146
console.log('ClickHouseDriver will use url', url);
155147

156-
// TODO parse username and apssword from config.auth, from a string like this:
157-
// `${getEnv('dbUser', { dataSource })}:${getEnv('dbPass', { dataSource })}`
158-
159148
const username = getEnv('dbUser', { dataSource });
160149
const password = getEnv('dbPass', { dataSource });
161-
const database = (getEnv('dbName', { dataSource }) as string) ||
162-
config && config.database ||
163-
'default';
150+
const database = config.database ?? (getEnv('dbName', { dataSource }) as string) ?? 'default';
151+
152+
// TODO this is a bit inconsistent with readOnly
153+
this.readOnlyMode =
154+
getEnv('clickhouseReadOnly', { dataSource }) === 'true';
155+
164156
this.config = {
165-
// host: getEnv('dbHost', { dataSource }),
166-
// port: getEnv('dbPort', { dataSource }),
167157
url,
168-
// auth:
169-
// getEnv('dbUser', { dataSource }) ||
170-
// getEnv('dbPass', { dataSource })
171-
// ? `${
172-
// getEnv('dbUser', { dataSource })
173-
// }:${
174-
// getEnv('dbPass', { dataSource })
175-
// }`
176-
// : '',
177158
username,
178159
password,
179-
// protocol: getEnv('dbSsl', { dataSource }) ? 'https:' : 'http:',
180-
queryOptions: {
181-
database,
182-
},
160+
database,
183161
exportBucket: this.getExportBucket(dataSource),
184162
readOnly: !!config.readOnly,
185-
...config
163+
clickhouseSettings: {
164+
// If ClickHouse user's permissions are restricted with "readonly = 1",
165+
// change settings queries are not allowed. Thus, "join_use_nulls" setting
166+
// can not be changed
167+
...(this.readOnlyMode ? {} : { join_use_nulls: 1 }),
168+
},
186169
};
187170

188-
this.readOnlyMode =
189-
getEnv('clickhouseReadOnly', { dataSource }) === 'true';
190-
191171
this.connectionFactory = {
192172
create: async () => createClient({
193-
// ...this.config,
194-
195173
url: this.config.url,
196174
username: this.config.username,
197175
password: this.config.password,
198-
199-
database: this.config.queryOptions.database,
200-
session_id: uuidv4(),
201-
clickhouse_settings: {
202-
//
203-
//
204-
// If ClickHouse user's permissions are restricted with "readonly = 1",
205-
// change settings queries are not allowed. Thus, "join_use_nulls" setting
206-
// can not be changed
207-
//
208-
//
209-
...(this.readOnlyMode ? {} : { join_use_nulls: 1 }),
210-
},
211-
176+
database: this.config.database,
177+
clickhouse_settings: this.config.clickhouseSettings,
212178
// TODO max_open_connections vs generic pool
213179
max_open_connections: 1,
214180
}),
@@ -320,17 +286,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
320286
await connection.command({
321287
query: formattedQuery,
322288
query_id: queryId,
323-
clickhouse_settings: {
324-
//
325-
//
326-
// If ClickHouse user's permissions are restricted with "readonly = 1",
327-
// change settings queries are not allowed. Thus, "join_use_nulls" setting
328-
// can not be changed
329-
//
330-
//
331-
// TODO extract join_use_nulls preparation to function
332-
...(this.readOnlyMode ? {} : { join_use_nulls: 1 }),
333-
},
289+
clickhouse_settings: this.config.clickhouseSettings,
334290
});
335291

336292
return {
@@ -342,16 +298,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
342298
query: formattedQuery,
343299
query_id: queryId,
344300
format: 'JSON',
345-
clickhouse_settings: {
346-
//
347-
//
348-
// If ClickHouse user's permissions are restricted with "readonly = 1",
349-
// change settings queries are not allowed. Thus, "join_use_nulls" setting
350-
// can not be changed
351-
//
352-
//
353-
...(this.readOnlyMode ? {} : { join_use_nulls: 1 }),
354-
},
301+
clickhouse_settings: this.config.clickhouseSettings,
355302
});
356303
console.log("queryResponse resultSet", query, resultSet.query_id, resultSet.response_headers);
357304

@@ -407,7 +354,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
407354
database as table_schema,
408355
type as data_type
409356
FROM system.columns
410-
WHERE database = '${this.config.queryOptions.database}'
357+
WHERE database = '${this.config.database}'
411358
`;
412359
}
413360

@@ -442,7 +389,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
442389
}
443390

444391
public override async getSchemas(): Promise<QuerySchemasResult[]> {
445-
return [{ schema_name: this.config.queryOptions.database }];
392+
return [{ schema_name: this.config.database }];
446393
}
447394

448395
public async stream(
@@ -464,16 +411,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
464411
query: formattedQuery,
465412
query_id: uuidv4(),
466413
format: 'JSONCompactEachRowWithNamesAndTypes',
467-
clickhouse_settings: {
468-
//
469-
//
470-
// If ClickHouse user's permissions are restricted with "readonly = 1",
471-
// change settings queries are not allowed. Thus, "join_use_nulls" setting
472-
// can not be changed
473-
//
474-
//
475-
...(this.readOnlyMode ? {} : { join_use_nulls: 1 }),
476-
}
414+
clickhouse_settings: this.config.clickhouseSettings,
477415
});
478416
// Array<unknown> is okay, because we use fixed JSONCompactEachRowWithNamesAndTypes format
479417
// And each row after first two will look like this: [42, "hello", [0,1]]

0 commit comments

Comments
 (0)