From 39eeccf8320caed0c71dacc5dd944763f865d2f5 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Thu, 21 Nov 2024 13:55:50 +0200 Subject: [PATCH 1/5] fix(schema-compiler): Fix type annotation for buildSqlAndParams --- packages/cubejs-schema-compiler/src/adapter/BaseQuery.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index d22716b4e10b4..4323604d5a829 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -577,9 +577,9 @@ export class BaseQuery { } /** - * Returns an array of SQL query strings for the query. + * Returns a pair of SQL query string and parameter values for the query. * @param {boolean} [exportAnnotatedSql] - returns annotated sql with not rendered params if true - * @returns {Array} + * @returns {[string, Array]} */ buildSqlAndParams(exportAnnotatedSql) { if (getEnv('nativeSqlPlanner')) { From dff63a576b124a5dec8f53a9d4b7b8b0ce53612a Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Thu, 21 Nov 2024 14:01:57 +0200 Subject: [PATCH 2/5] test(schema-compiler): Rework TEST_CLICKHOUSE_HOST handling --- .../clickhouse/ClickHouseDbRunner.js | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.js b/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.js index d7cb4a295cc85..bfb619f4f2ff3 100644 --- a/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.js +++ b/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.js @@ -103,15 +103,24 @@ export class ClickHouseDbRunner extends BaseDbRunner { }; testQueries = async (queries, prepareDataSet) => { - if (!this.container && !process.env.TEST_CLICKHOUSE_HOST) { - this.container = await new GenericContainer(`clickhouse/clickhouse-server:${this.clickHouseVersion}`) - .withExposedPorts(this.port()) - .start(); + let host; + let port; + if (process.env.TEST_CLICKHOUSE_HOST) { + host = process.env.TEST_CLICKHOUSE_HOST; + port = 8123; + } else { + if (!this.container) { + this.container = await new GenericContainer(`clickhouse/clickhouse-server:${this.clickHouseVersion}`) + .withExposedPorts(this.port()) + .start(); + } + host = 'localhost'; + port = this.container.getMappedPort(8123); } const clickHouse = new ClickHouse({ - host: 'localhost', - port: process.env.TEST_CLICKHOUSE_HOST ? 8123 : this.container.getMappedPort(8123), + host, + port, }); clickHouse.sessionId = uuidv4(); // needed for tests to use temporary tables From 9f49f97d9ba069d6d5d1c23ef2429068b71f5e06 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Thu, 21 Nov 2024 13:44:14 +0200 Subject: [PATCH 3/5] test(schema-compiler): Port ClickHouseDbRunner to TypeScript --- ...HouseDbRunner.js => ClickHouseDbRunner.ts} | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) rename packages/cubejs-schema-compiler/test/integration/clickhouse/{ClickHouseDbRunner.js => ClickHouseDbRunner.ts} (84%) diff --git a/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.js b/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts similarity index 84% rename from packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.js rename to packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts index bfb619f4f2ff3..b12a15321d7a6 100644 --- a/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.js +++ b/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts @@ -1,28 +1,37 @@ /* eslint-disable */ +// @ts-ignore import ClickHouse from '@cubejs-backend/apla-clickhouse'; import { GenericContainer } from 'testcontainers'; +import type { StartedTestContainer } from 'testcontainers'; import { format as formatSql } from 'sqlstring'; import { v4 as uuidv4 } from 'uuid'; import { ClickHouseQuery } from '../../../src/adapter/ClickHouseQuery'; import { BaseDbRunner } from "../utils/BaseDbRunner"; +// Just a placeholder for now +type ClickHouseClient = any; + process.env.TZ = 'GMT'; export class ClickHouseDbRunner extends BaseDbRunner { - adapter = 'clickhouse'; - container = null; - clickHouseVersion = process.env.TEST_CLICKHOUSE_VERSION || '23.11'; - supportsExtendedDateTimeResults = this.clickHouseVersion >= '22.9'; - allowExperimentalJoinCondition = this.clickHouseVersion >= '24.5'; + public adapter: string = 'clickhouse'; + + protected container: StartedTestContainer | null = null; + + protected clickHouseVersion: string = process.env.TEST_CLICKHOUSE_VERSION || '23.11'; + + public supportsExtendedDateTimeResults: boolean = this.clickHouseVersion >= '22.9'; - tearDown = async () => { + protected allowExperimentalJoinCondition: boolean = this.clickHouseVersion >= '24.5'; + + public override async tearDown(): Promise { if (this.container) { await this.container.stop(); this.container = null; } } - gutterDataSet = async function (clickHouse) { + protected async gutterDataSet(clickHouse: ClickHouseClient): Promise { // let engine = 'MergeTree PARTITION BY id ORDER BY (id) SETTINGS index_granularity = 8192' const engine = 'Memory'; @@ -100,9 +109,9 @@ export class ClickHouseDbRunner extends BaseDbRunner { (40), (41), (42), (43), (44), (45), (46), (47), (48), (49), (50), (51), (52), (53), (54), (55), (56), (57), (58), (59) `, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); - }; + } - testQueries = async (queries, prepareDataSet) => { + public override async testQueries(queries: Array<[string, Array]>, prepareDataSet?: ((client: ClickHouseClient) => Promise) | null): Promise>>> { let host; let port; if (process.env.TEST_CLICKHOUSE_HOST) { @@ -128,7 +137,7 @@ export class ClickHouseDbRunner extends BaseDbRunner { prepareDataSet = prepareDataSet || this.gutterDataSet; await prepareDataSet(clickHouse); - const requests = []; + const requests: Array = []; // Controls whether functions return results with extended date and time ranges. // @@ -158,16 +167,18 @@ export class ClickHouseDbRunner extends BaseDbRunner { const results = await Promise.all(requests); return results.map(_normaliseResponse); - }; + } - testQuery = async (queryAndParams, prepareDataSet) => this.testQueries([queryAndParams], prepareDataSet) - .then(res => res[0]); + public async testQuery(queryAndParams: [string, Array], prepareDataSet?: ((client: ClickHouseClient) => Promise) | null): Promise>> { + const res = await this.testQueries([queryAndParams], prepareDataSet); + return res[0]; + } - port() { + public override port(): number { return 8123; } - newTestQuery(compilers, query) { + protected override newTestQuery(compilers: unknown, query: unknown): ClickHouseQuery { return new ClickHouseQuery(compilers, query); } } @@ -179,7 +190,7 @@ export class ClickHouseDbRunner extends BaseDbRunner { // // https://github.com/statsbotco/cube.js/pull/98#discussion_r279698399 // -function _normaliseResponse(res) { +function _normaliseResponse(res: any): Array> { if (process.env.DEBUG_LOG === 'true') console.log(res); if (res.data) { res.data.forEach(row => { From 05984a2176208cb0345faf8f8f2b674bbc9642a0 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Thu, 21 Nov 2024 14:00:23 +0200 Subject: [PATCH 4/5] refactor(schema-compiler): Move _normaliseResponse to static method --- .../clickhouse/ClickHouseDbRunner.ts | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts b/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts index b12a15321d7a6..40ae3991f8b29 100644 --- a/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts +++ b/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts @@ -166,7 +166,7 @@ export class ClickHouseDbRunner extends BaseDbRunner { const results = await Promise.all(requests); - return results.map(_normaliseResponse); + return results.map(ClickHouseDbRunner._normaliseResponse); } public async testQuery(queryAndParams: [string, Array], prepareDataSet?: ((client: ClickHouseClient) => Promise) | null): Promise>> { @@ -181,34 +181,34 @@ export class ClickHouseDbRunner extends BaseDbRunner { protected override newTestQuery(compilers: unknown, query: unknown): ClickHouseQuery { return new ClickHouseQuery(compilers, query); } -} -// -// -// ClickHouse returns DateTime as strings in format "YYYY-DD-MM HH:MM:SS" -// cube.js expects them in format "YYYY-DD-MMTHH:MM:SS.000", so translate them based on the metadata returned -// -// https://github.com/statsbotco/cube.js/pull/98#discussion_r279698399 -// -function _normaliseResponse(res: any): Array> { - if (process.env.DEBUG_LOG === 'true') console.log(res); - if (res.data) { - res.data.forEach(row => { - for (const field in row) { - const value = row[field]; - if (value !== null) { - const meta = res.meta.find(m => m.name == field); - if (meta.type.includes('DateTime')) { - row[field] = `${value.substring(0, 10)}T${value.substring(11, 22)}.000`; - } else if (meta.type.includes('Date')) { - row[field] = `${value}T00:00:00.000`; - } else if (meta.type.includes('Int') || meta.type.includes('Float')) { - // convert all numbers into strings - row[field] = `${value}`; + // + // + // ClickHouse returns DateTime as strings in format "YYYY-DD-MM HH:MM:SS" + // cube.js expects them in format "YYYY-DD-MMTHH:MM:SS.000", so translate them based on the metadata returned + // + // https://github.com/statsbotco/cube.js/pull/98#discussion_r279698399 + // + protected static _normaliseResponse(res: any): Array> { + if (process.env.DEBUG_LOG === 'true') console.log(res); + if (res.data) { + res.data.forEach(row => { + for (const field in row) { + const value = row[field]; + if (value !== null) { + const meta = res.meta.find(m => m.name == field); + if (meta.type.includes('DateTime')) { + row[field] = `${value.substring(0, 10)}T${value.substring(11, 22)}.000`; + } else if (meta.type.includes('Date')) { + row[field] = `${value}T00:00:00.000`; + } else if (meta.type.includes('Int') || meta.type.includes('Float')) { + // convert all numbers into strings + row[field] = `${value}`; + } } } - } - }); + }); + } + return res.data; } - return res.data; } From 753141c153950ac036fbd93d5e160e9b96c4f9c9 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Thu, 21 Nov 2024 12:21:33 +0200 Subject: [PATCH 5/5] test(schema-compiler): Replace apla-clickhouse with @clickhouse/client --- packages/cubejs-schema-compiler/package.json | 2 +- .../clickhouse/ClickHouseDbRunner.ts | 153 ++++++++++-------- yarn.lock | 5 - 3 files changed, 85 insertions(+), 75 deletions(-) diff --git a/packages/cubejs-schema-compiler/package.json b/packages/cubejs-schema-compiler/package.json index a156bb516e726..ee8d87e0c7be1 100644 --- a/packages/cubejs-schema-compiler/package.json +++ b/packages/cubejs-schema-compiler/package.json @@ -57,7 +57,7 @@ "uuid": "^8.3.2" }, "devDependencies": { - "@cubejs-backend/apla-clickhouse": "^1.7.0", + "@clickhouse/client": "^1.7.0", "@cubejs-backend/linter": "^1.0.0", "@cubejs-backend/query-orchestrator": "1.1.7", "@types/babel__code-frame": "^7.0.6", diff --git a/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts b/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts index 40ae3991f8b29..f399bcfa72a7b 100644 --- a/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts +++ b/packages/cubejs-schema-compiler/test/integration/clickhouse/ClickHouseDbRunner.ts @@ -1,6 +1,5 @@ -/* eslint-disable */ -// @ts-ignore -import ClickHouse from '@cubejs-backend/apla-clickhouse'; +import { createClient } from '@clickhouse/client'; +import type { ClickHouseClient, ResponseJSON } from '@clickhouse/client'; import { GenericContainer } from 'testcontainers'; import type { StartedTestContainer } from 'testcontainers'; import { format as formatSql } from 'sqlstring'; @@ -8,9 +7,6 @@ import { v4 as uuidv4 } from 'uuid'; import { ClickHouseQuery } from '../../../src/adapter/ClickHouseQuery'; import { BaseDbRunner } from "../utils/BaseDbRunner"; -// Just a placeholder for now -type ClickHouseClient = any; - process.env.TZ = 'GMT'; export class ClickHouseDbRunner extends BaseDbRunner { @@ -35,27 +31,32 @@ export class ClickHouseDbRunner extends BaseDbRunner { // let engine = 'MergeTree PARTITION BY id ORDER BY (id) SETTINGS index_granularity = 8192' const engine = 'Memory'; - await clickHouse.querying(` - CREATE TEMPORARY TABLE visitors (id UInt64, amount UInt64, created_at DateTime, updated_at DateTime, status UInt64, source Nullable(String), latitude Float64, longitude Float64) - ENGINE = ${engine}`, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); + await clickHouse.command({ query: ` + CREATE TEMPORARY TABLE visitors (id UInt64, amount UInt64, created_at DateTime, updated_at DateTime, status UInt64, source Nullable(String), latitude Float64, longitude Float64) + ENGINE = ${engine} + ` }); - await clickHouse.querying(` - CREATE TEMPORARY TABLE visitor_checkins (id UInt64, visitor_id UInt64, created_at DateTime, source Nullable(String)) - ENGINE = ${engine}`, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); + await clickHouse.command({ query: ` + CREATE TEMPORARY TABLE visitor_checkins (id UInt64, visitor_id UInt64, created_at DateTime, source Nullable(String)) + ENGINE = ${engine} + ` }); - await clickHouse.querying(` - CREATE TEMPORARY TABLE cards (id UInt64, visitor_id UInt64, visitor_checkin_id UInt64) - ENGINE = ${engine}`, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); + await clickHouse.command({ query: ` + CREATE TEMPORARY TABLE cards (id UInt64, visitor_id UInt64, visitor_checkin_id UInt64) + ENGINE = ${engine} + ` }); - await clickHouse.querying(` - CREATE TEMPORARY TABLE events (id UInt64, type String, name String, started_at DateTime64, ended_at Nullable(DateTime64)) - ENGINE = ${engine}`, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); + await clickHouse.command({ query: ` + CREATE TEMPORARY TABLE events (id UInt64, type String, name String, started_at DateTime64, ended_at Nullable(DateTime64)) + ENGINE = ${engine} + ` }); - await clickHouse.querying(` - CREATE TEMPORARY TABLE numbers (num Int) - ENGINE = ${engine}`, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); + await clickHouse.command({ query: ` + CREATE TEMPORARY TABLE numbers (num Int) + ENGINE = ${engine} + ` }); - await clickHouse.querying(` + await clickHouse.command({ query: ` INSERT INTO visitors (id, amount, created_at, updated_at, status, source, latitude, longitude) VALUES @@ -65,9 +66,9 @@ export class ClickHouseDbRunner extends BaseDbRunner { (4, 400, '2017-01-06 16:00:00', '2017-01-24 16:00:00', 2, null, 120.120, 10.60), (5, 500, '2017-01-06 16:00:00', '2017-01-24 16:00:00', 2, null, 120.120, 58.10), (6, 500, '2016-09-06 16:00:00', '2016-09-06 16:00:00', 2, null, 120.120, 58.10) - `, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); + ` }); - await clickHouse.querying(` + await clickHouse.command({ query: ` INSERT INTO visitor_checkins (id, visitor_id, created_at, source) VALUES @@ -77,18 +78,18 @@ export class ClickHouseDbRunner extends BaseDbRunner { (4, 2, '2017-01-04 16:00:00', null), (5, 2, '2017-01-04 16:00:00', null), (6, 3, '2017-01-05 16:00:00', null) - `, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); + ` }); - await clickHouse.querying(` + await clickHouse.command({ query: ` INSERT INTO cards (id, visitor_id, visitor_checkin_id) VALUES (1, 1, 1), (2, 1, 2), (3, 3, 6) - `, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); + ` }); - await clickHouse.querying(` + await clickHouse.command({ query: ` INSERT INTO events (id, type, name, started_at, ended_at) VALUES @@ -96,9 +97,9 @@ export class ClickHouseDbRunner extends BaseDbRunner { (2, 'moon_missions', 'Apollo 11', '1969-07-16 13:32:00', '1969-07-24 16:50:35'), (3, 'moon_missions', 'Artemis I', '2021-11-16 06:32:00', '2021-12-11 18:50:00'), (4, 'private_missions', 'Axiom Mission 1', '2022-04-08 15:17:12', '2022-04-25 17:06:00') - `, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); + ` }); - await clickHouse.querying(` + await clickHouse.command({ query: ` INSERT INTO numbers (num) VALUES @@ -108,7 +109,7 @@ export class ClickHouseDbRunner extends BaseDbRunner { (30), (31), (32), (33), (34), (35), (36), (37), (38), (39), (40), (41), (42), (43), (44), (45), (46), (47), (48), (49), (50), (51), (52), (53), (54), (55), (56), (57), (58), (59) - `, { queryOptions: { session_id: clickHouse.sessionId, join_use_nulls: '1' } }); + ` }); } public override async testQueries(queries: Array<[string, Array]>, prepareDataSet?: ((client: ClickHouseClient) => Promise) | null): Promise>>> { @@ -127,18 +128,17 @@ export class ClickHouseDbRunner extends BaseDbRunner { port = this.container.getMappedPort(8123); } - const clickHouse = new ClickHouse({ - host, - port, - }); + const clickHouse = createClient({ + url: `http://${host}:${port}`, - clickHouse.sessionId = uuidv4(); // needed for tests to use temporary tables + // needed for tests to use temporary tables + session_id: uuidv4(), + max_open_connections: 1, + }); prepareDataSet = prepareDataSet || this.gutterDataSet; await prepareDataSet(clickHouse); - const requests: Array = []; - // Controls whether functions return results with extended date and time ranges. // // 0 — Functions return Date or DateTime for all arguments (default). @@ -150,19 +150,23 @@ export class ClickHouseDbRunner extends BaseDbRunner { // // https://clickhouse.com/docs/en/operations/settings/settings#enable-extended-results-for-datetime-functions const extendedDateTimeResultsOptions = this.supportsExtendedDateTimeResults ? { - enable_extended_results_for_datetime_functions: '1' - } : {}; - - for (const [query, params] of queries) { - requests.push(clickHouse.querying(formatSql(query, params), { - dataObjects: true, - queryOptions: { - session_id: clickHouse.sessionId, - join_use_nulls: '1', - ...extendedDateTimeResultsOptions - } - })); - } + enable_extended_results_for_datetime_functions: 1 + } as const : {}; + + const requests = queries + .map(async ([query, params]) => { + const resultSet = await clickHouse.query({ + query: formatSql(query, params), + format: 'JSON', + clickhouse_settings: { + join_use_nulls: 1, + ...extendedDateTimeResultsOptions + } + }); + // Because we used JSON format we expect each row in result set to be a record of column name => value + const result = await resultSet.json>(); + return result; + }); const results = await Promise.all(requests); @@ -189,26 +193,37 @@ export class ClickHouseDbRunner extends BaseDbRunner { // // https://github.com/statsbotco/cube.js/pull/98#discussion_r279698399 // - protected static _normaliseResponse(res: any): Array> { - if (process.env.DEBUG_LOG === 'true') console.log(res); - if (res.data) { - res.data.forEach(row => { - for (const field in row) { - const value = row[field]; - if (value !== null) { - const meta = res.meta.find(m => m.name == field); - if (meta.type.includes('DateTime')) { - row[field] = `${value.substring(0, 10)}T${value.substring(11, 22)}.000`; - } else if (meta.type.includes('Date')) { - row[field] = `${value}T00:00:00.000`; - } else if (meta.type.includes('Int') || meta.type.includes('Float')) { - // convert all numbers into strings - row[field] = `${value}`; + protected static _normaliseResponse(res: ResponseJSON>): Array> { + if (process.env.DEBUG_LOG === 'true') { + console.log(res); + } + + const { meta, data } = res; + if (meta === undefined) { + throw new Error('Unexpected missing meta'); + } + + data.forEach(row => { + for (const [field, value] of Object.entries(row)) { + if (value !== null) { + const fieldMeta = meta.find(m => m.name === field); + if (fieldMeta === undefined) { + throw new Error(`Missing meta for field ${field}`); + } + if (fieldMeta.type.includes('DateTime')) { + if (typeof value !== 'string') { + throw new Error(`Unexpected value for ${field}`); } + row[field] = `${value.substring(0, 10)}T${value.substring(11, 22)}.000`; + } else if (fieldMeta.type.includes('Date')) { + row[field] = `${value}T00:00:00.000`; + } else if (fieldMeta.type.includes('Int') || fieldMeta.type.includes('Float')) { + // convert all numbers into strings + row[field] = `${value}`; } } - }); - } - return res.data; + } + }); + return data; } } diff --git a/yarn.lock b/yarn.lock index bbc90068904a5..a3b76c1851873 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4365,11 +4365,6 @@ tiny-invariant "^1.3.3" valid-url "^1.0.9" -"@cubejs-backend/apla-clickhouse@^1.7.0": - version "1.7.0" - resolved "https://registry.yarnpkg.com/@cubejs-backend/apla-clickhouse/-/apla-clickhouse-1.7.0.tgz#6359f46c56492d1704d18be0210c7546fdac5f5e" - integrity sha512-qwXapTC/qosA6RprElRjnl8gmlDQaxtJPtbgcdjyNvkmiyao1HI+w5QkjHWCiVm6aTzE0gjFr6/2y87TZ9fojg== - "@cubejs-backend/dotenv@^9.0.2": version "9.0.2" resolved "https://registry.yarnpkg.com/@cubejs-backend/dotenv/-/dotenv-9.0.2.tgz#c3679091b702f0fd38de120c5a63943fcdc0dcbf"