Skip to content

Commit 9aa4718

Browse files
mcheshkovmcheshkov
committed
[FIXUP] extract move insert and create table handling to separate methods
1 parent 29f15fc commit 9aa4718

File tree

2 files changed

+40
-19
lines changed

2 files changed

+40
-19
lines changed

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

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
UnloadOptions,
2424
} from '@cubejs-backend/base-driver';
2525

26+
import * as process from "node:process";
2627
import { Readable } from 'node:stream';
2728
import { ClickHouseClient, createClient } from '@clickhouse/client';
2829
import type { ResponseJSON } from '@clickhouse/client';
@@ -283,6 +284,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
283284
}
284285

285286
protected queryResponse(query: string, values: unknown[]): Promise<ResponseJSON<Record<string, unknown>>> {
287+
const queryResponseCompat = process.env.CUBE_CLICKHOUSE_QUERY_COMPAT === 'true';
286288
const isInsert = query.trim().match(/^INSERT/i);
287289

288290
console.log('ClickHouse queryResponse call', query);
@@ -296,7 +298,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
296298
//
297299
// }
298300

299-
if (isInsert) {
301+
if (queryResponseCompat && isInsert) {
300302
// TODO why do we even have inserts coming to query? is it accepted contract, or just tests misbehaviour?
301303
// INSERT queries does not work with `query` method due to query preparation (like adding FORMAT clause)
302304
// And `insert` wants to construct query on its own
@@ -338,7 +340,7 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
338340
});
339341
console.log("queryResponse resultSet", query, resultSet.query_id, resultSet.response_headers);
340342

341-
if (resultSet.response_headers['x-clickhouse-format'] !== 'JSON') {
343+
if (queryResponseCompat && resultSet.response_headers['x-clickhouse-format'] !== 'JSON') {
342344
// TODO why do we even have CREATE TABLE coming to query? is it accepted contract, or just tests misbehaviour?
343345
// Result set for query like CREATE TABLE would be empty, and would not even send x-clickhouse-format header field
344346
// Parsing response like that as JSON would fail
@@ -704,4 +706,24 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
704706
incrementalSchemaLoading: true,
705707
};
706708
}
709+
710+
// This is not part of a driver interface, and should be used only for testing
711+
public async command(query: string): Promise<void> {
712+
await this.withConnection(async (connection) => {
713+
await connection.command({
714+
query,
715+
});
716+
});
717+
}
718+
719+
// This is not part of a driver interface, and should be used only for testing
720+
public async insert(table: string, values: Array<Array<unknown>>): Promise<void> {
721+
await this.withConnection(async (connection) => {
722+
await connection.insert({
723+
table,
724+
values,
725+
format: 'JSONCompactEachRow',
726+
});
727+
});
728+
}
707729
}

‎packages/cubejs-clickhouse-driver/test/ClickHouseDriver.test.ts‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe('ClickHouseDriver', () => {
3535

3636
await doWithDriver(async (driver) => {
3737
await driver.createSchemaIfNotExists('test');
38-
await driver.query(
38+
await driver.command(
3939
`
4040
CREATE TABLE test.types_test (
4141
date Date,
@@ -59,18 +59,17 @@ describe('ClickHouseDriver', () => {
5959
enum8 Enum('hello' = 1, 'world' = 2),
6060
enum16 Enum('hello' = 1, 'world' = 1000)
6161
) ENGINE Log
62-
`,
63-
[]
62+
`
6463
);
6564

66-
await driver.query('INSERT INTO test.types_test VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)', [
67-
'2020-01-01', '2020-01-01 00:00:00', '2020-01-01 00:00:00.000', '2020-01-01 00:00:00.000000', '2020-01-01 00:00:00.000000000', 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1.01, 1.01, 1.01, 'hello', 'world'
65+
await driver.insert('test.types_test', [
66+
['2020-01-01', '2020-01-01 00:00:00', '2020-01-01 00:00:00.000', '2020-01-01 00:00:00.000000', '2020-01-01 00:00:00.000000000', 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1.01, 1.01, 1.01, 'hello', 'world']
6867
]);
69-
await driver.query('INSERT INTO test.types_test VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)', [
70-
'2020-01-02', '2020-01-02 00:00:00', '2020-01-02 00:00:00.123', '2020-01-02 00:00:00.123456', '2020-01-02 00:00:00.123456789', 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2.02, 2.02, 2.02, 'hello', 'world'
68+
await driver.insert('test.types_test', [
69+
['2020-01-02', '2020-01-02 00:00:00', '2020-01-02 00:00:00.123', '2020-01-02 00:00:00.123456', '2020-01-02 00:00:00.123456789', 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2.02, 2.02, 2.02, 'hello', 'world']
7170
]);
72-
await driver.query('INSERT INTO test.types_test VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)', [
73-
'2020-01-03', '2020-01-03 00:00:00', '2020-01-03 00:00:00.234', '2020-01-03 00:00:00.234567', '2020-01-03 00:00:00.234567890', 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3.03, 3.03, 3.03, 'hello', 'world'
71+
await driver.insert('test.types_test', [
72+
['2020-01-03', '2020-01-03 00:00:00', '2020-01-03 00:00:00.234', '2020-01-03 00:00:00.234567', '2020-01-03 00:00:00.234567890', 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3.03, 3.03, 3.03, 'hello', 'world']
7473
]);
7574
});
7675
}, 30 * 1000);
@@ -190,8 +189,8 @@ describe('ClickHouseDriver', () => {
190189
const name = `temp_${Date.now()}`;
191190
try {
192191
await driver.createSchemaIfNotExists(name);
193-
await driver.query(`CREATE TABLE ${name}.a (dateTime DateTime, date Date) ENGINE Log`, []);
194-
await driver.query(`INSERT INTO ${name}.a VALUES ('2019-04-30 11:55:00', '2019-04-30')`, []);
192+
await driver.command(`CREATE TABLE ${name}.a (dateTime DateTime, date Date) ENGINE Log`);
193+
await driver.insert(`${name}.a`, [['2019-04-30 11:55:00', '2019-04-30']]);
195194

196195
const values = await driver.query(`SELECT * FROM ${name}.a`, []);
197196
expect(values).toEqual([{
@@ -209,8 +208,8 @@ describe('ClickHouseDriver', () => {
209208
const name = `temp_${Date.now()}`;
210209
try {
211210
await driver.createSchemaIfNotExists(name);
212-
await driver.query(`CREATE TABLE ${name}.test (x Int32, s String) ENGINE Log`, []);
213-
await driver.query(`INSERT INTO ${name}.test VALUES (?, ?), (?, ?), (?, ?)`, [1, 'str1', 2, 'str2', 3, 'str3']);
211+
await driver.command(`CREATE TABLE ${name}.test (x Int32, s String) ENGINE Log`);
212+
await driver.insert(`${name}.test`, [[1, 'str1'], [2, 'str2'], [3, 'str3']]);
214213
const values = await driver.query(`SELECT * FROM ${name}.test WHERE x = ?`, [2]);
215214
expect(values).toEqual([{ x: '2', s: 'str2' }]);
216215
} finally {
@@ -224,11 +223,11 @@ describe('ClickHouseDriver', () => {
224223
const name = `temp_${Date.now()}`;
225224
try {
226225
await driver.createSchemaIfNotExists(name);
227-
await driver.query(`CREATE TABLE ${name}.a (x Int32, s String) ENGINE Log`, []);
228-
await driver.query(`INSERT INTO ${name}.a VALUES (?, ?), (?, ?), (?, ?)`, [1, 'str1', 2, 'str2', 3, 'str3']);
226+
await driver.command(`CREATE TABLE ${name}.a (x Int32, s String) ENGINE Log`);
227+
await driver.insert(`${name}.a`, [[1, 'str1'], [2, 'str2'], [3, 'str3']]);
229228

230-
await driver.query(`CREATE TABLE ${name}.b (x Int32, s String) ENGINE Log`, []);
231-
await driver.query(`INSERT INTO ${name}.b VALUES (?, ?), (?, ?), (?, ?)`, [2, 'str2', 3, 'str3', 4, 'str4']);
229+
await driver.command(`CREATE TABLE ${name}.b (x Int32, s String) ENGINE Log`);
230+
await driver.insert(`${name}.b`, [[2, 'str2'], [3, 'str3'], [4, 'str4']]);
232231

233232
const values = await driver.query(`SELECT * FROM ${name}.a LEFT OUTER JOIN ${name}.b ON a.x = b.x`, []);
234233
expect(values).toEqual([

0 commit comments

Comments
 (0)