Fix command and related tests, use more flexible test env, rename configs

Author: slvrtrn

.docker/clickhouse/cluster/config1.xml renamed to .docker/clickhouse/cluster/server1_config.xml

@@ -52,6 +52,6 @@ jobs:
           CLICKHOUSE_CLOUD_HOST: ${{ secrets.CLICKHOUSE_CLOUD_HOST }}
           CLICKHOUSE_CLOUD_USERNAME: ${{ secrets.CLICKHOUSE_CLOUD_USERNAME }}
           CLICKHOUSE_CLOUD_PASSWORD: ${{ secrets.CLICKHOUSE_CLOUD_PASSWORD }}
-          CLICKHOUSE_CLOUD_ENABLED: 'true'
+          CLICKHOUSE_TEST_ENVIRONMENT: 'cloud'
         run: |
           npm test

.docker/clickhouse/cluster/macros1.xml renamed to .docker/clickhouse/cluster/server1_macros.xml

@@ -1,65 +1,71 @@
 import { expect } from 'chai';
-import { createClient, type ClickHouseClient } from '../../src';
-import type { ResponseJSON } from '../../src/clickhouse_types';
+import type { ResponseJSON } from '../../src';
+import { type ClickHouseClient } from '../../src';
+import {
+  createTestClient,
+  getClickHouseTestEnvironment,
+  TestEnv,
+} from '../utils/client';
+import { guid } from '../utils';
 
 describe('command', () => {
   let client: ClickHouseClient;
+  beforeEach(() => {
+    client = createTestClient();
+  });
   afterEach(async () => {
-    await client.command({ query: 'DROP TABLE example' });
     await client.close();
   });
 
   it('sends a command to execute', async () => {
-    client = createClient();
-    const ddl =
-      'CREATE TABLE example (id UInt64, name String, sku Array(UInt8), timestamp DateTime) Engine = Memory';
+    const { ddl, tableName, engine } = getDDL();
 
-    await client.command({ query: ddl });
+    const commandResult = await client.command({
+      query: ddl,
+      format: 'TabSeparated',
+    });
+    await commandResult.text();
 
-    const result = await client.select({
-      query: `SELECT * from system.tables where name = 'example'`,
+    const selectResult = await client.select({
+      query: `SELECT * from system.tables where name = '${tableName}'`,
       format: 'JSON',
     });
 
-    const { data, rows } = await result.json<
+    const { data, rows } = await selectResult.json<
       ResponseJSON<{ name: string; engine: string; create_table_query: string }>
     >();
 
     expect(rows).to.equal(1);
     const table = data[0];
-    expect(table.name).equal('example');
-    expect(table.engine).equal('Memory');
+    expect(table.name).equal(tableName);
+    expect(table.engine).equal(engine);
     expect(table.create_table_query).to.be.a('string');
   });
 
   it('does not swallow ClickHouse error', (done) => {
-    client = createClient();
-
-    const ddl =
-      'CREATE TABLE example (id UInt64, name String, sku Array(UInt8), timestamp DateTime) Engine = Memory';
-
+    const { ddl, tableName } = getDDL();
     Promise.resolve()
       .then(() => client.command({ query: ddl }))
       .then(() => client.command({ query: ddl }))
       .catch((e: any) => {
         expect(e.code).to.equal('57');
         expect(e.type).to.equal('TABLE_ALREADY_EXISTS');
         // TODO remove whitespace from end
-        expect(e.message).equal('Table default.example already exists. ');
+        expect(e.message).equal(`Table default.${tableName} already exists. `);
         done();
       });
   });
 
   it.skip('can specify a parameterized query', async () => {
-    client = createClient();
-    await client.command({
-      query:
-        'CREATE TABLE {table_name: String} (id UInt64, name String, sku Array(UInt8), timestamp DateTime) Engine = Memory',
+    const commandResult = await client.command({
+      query: '',
       query_params: {
         table_name: 'example',
       },
     });
+    await commandResult.text();
 
+    // FIXME: use different DDL based on the TestEnv
     const result = await client.select({
       query: `SELECT * from system.tables where name = 'example'`,
       format: 'JSON',
@@ -74,3 +80,43 @@ describe('command', () => {
     expect(table.name).to.equal('example');
   });
 });
+
+function getDDL(): {
+  ddl: string;
+  tableName: string;
+  engine: string;
+} {
+  const env = getClickHouseTestEnvironment();
+  const tableName = `command_test_${guid()}`;
+  switch (env) {
+    // ENGINE can be omitted in the cloud statements:
+    // it will use ReplicatedMergeTree and will add ON CLUSTER as well
+    case TestEnv.Cloud: {
+      const ddl = `
+        CREATE TABLE ${tableName}
+        (id UInt64, name String, sku Array(UInt8), timestamp DateTime)
+        ORDER BY (id)
+      `;
+      return { ddl, tableName, engine: 'ReplicatedMergeTree' };
+    }
+    case TestEnv.LocalSingleNode: {
+      const ddl = `
+        CREATE TABLE ${tableName}
+        (id UInt64, name String, sku Array(UInt8), timestamp DateTime)
+        ENGINE = MergeTree()
+        ORDER BY (id)
+      `;
+      return { ddl, tableName, engine: 'MergeTree' };
+    }
+
+    case TestEnv.LocalCluster: {
+      const ddl = `
+        CREATE TABLE ${tableName} ON CLUSTER '{cluster}'
+        (id UInt64, name String, sku Array(UInt8), timestamp DateTime)
+        ENGINE ReplicatedMergeTree('/clickhouse/{cluster}/tables/{database}/{table}/{shard}', '{replica}')
+        ORDER BY (id)
+      `;
+      return { ddl, tableName, engine: 'ReplicatedMergeTree' };
+    }
+  }
+}

.docker/clickhouse/cluster/config2.xml renamed to .docker/clickhouse/cluster/server2_config.xml

@@ -2,6 +2,7 @@ import { expect } from 'chai';
 import type { ResponseJSON } from '../../src';
 import { type ClickHouseClient } from '../../src';
 import { createTable, createTestClient, guid } from '../utils';
+import { TestEnv } from '../utils/client';
 
 describe('insert', () => {
   let client: ClickHouseClient;
@@ -13,18 +14,32 @@ describe('insert', () => {
       },
     });
     tableName = `test_table_${guid()}`;
-    await createTable(
-      client,
-      (engine) => {
-        return `
-          CREATE TABLE ${tableName} ON CLUSTER '{cluster}'
-          (id UInt64, name String, sku Array(UInt8))
-          ${engine}
-          ORDER BY (id)
-        `;
-      },
-      `ReplicatedMergeTree('/clickhouse/{cluster}/tables/{database}/{table}/{shard}', '{replica}')`
-    );
+    await createTable(client, (env) => {
+      switch (env) {
+        // ENGINE can be omitted in the cloud statements:
+        // it will use ReplicatedMergeTree and will add ON CLUSTER as well
+        case TestEnv.Cloud:
+          return `
+            CREATE TABLE ${tableName}
+            (id UInt64, name String, sku Array(UInt8))
+            ORDER BY (id)
+          `;
+        case TestEnv.LocalSingleNode:
+          return `
+            CREATE TABLE ${tableName}
+            (id UInt64, name String, sku Array(UInt8))
+            ENGINE MergeTree()
+            ORDER BY (id)
+          `;
+        case TestEnv.LocalCluster:
+          return `
+            CREATE TABLE ${tableName} ON CLUSTER '{cluster}'
+            (id UInt64, name String, sku Array(UInt8))
+            ENGINE ReplicatedMergeTree('/clickhouse/{cluster}/tables/{database}/{table}/{shard}', '{replica}')
+            ORDER BY (id)
+          `;
+      }
+    });
   });
   afterEach(async () => {
     await client.close();

.docker/clickhouse/cluster/macros2.xml renamed to .docker/clickhouse/cluster/server2_macros.xml

@@ -1,26 +1,39 @@
 import {
   ClickHouseClient,
   ClickHouseClientConfigOptions,
+  ClickHouseSettings,
   createClient,
 } from '../../src';
 import { guid } from './guid';
 
+export enum TestEnv {
+  Cloud = 'CLOUD',
+  LocalSingleNode = 'LOCAL_SINGLE_NODE',
+  LocalCluster = 'LOCAL_CLUSTER',
+}
+
 export function createTestClient(
-  config?: ClickHouseClientConfigOptions
+  config: ClickHouseClientConfigOptions = {}
 ): ClickHouseClient {
-  if (isClickHouseCloudEnabled()) {
+  const env = getClickHouseTestEnvironment();
+  const clickHouseSettings: ClickHouseSettings = {};
+  if (env === TestEnv.LocalCluster || env === TestEnv.Cloud) {
+    clickHouseSettings.insert_quorum = 2;
+  }
+  if (env === TestEnv.Cloud) {
     console.log('Using ClickHouse Cloud client');
     return createClient({
       host: getFromEnv('CLICKHOUSE_CLOUD_HOST'),
       username: getFromEnv('CLICKHOUSE_CLOUD_USERNAME'),
       password: getFromEnv('CLICKHOUSE_CLOUD_PASSWORD'),
-      clickhouse_settings: {
-        insert_quorum: 2,
-      },
+      ...clickHouseSettings,
       ...config,
     });
   } else {
-    return createClient(config);
+    return createClient({
+      ...clickHouseSettings,
+      ...config,
+    });
   }
 }
 
@@ -37,12 +50,10 @@ export async function createRandomDatabase(
 
 export async function createTable(
   client: ClickHouseClient,
-  definition: (engine: string) => string,
-  engine = 'MergeTree()'
+  definition: (environment: TestEnv) => string
 ) {
-  const ddl = isClickHouseCloudEnabled()
-    ? definition('')
-    : definition(`ENGINE ${engine}`);
+  const env = getClickHouseTestEnvironment();
+  const ddl = definition(env);
   await client.command({
     query: ddl,
   });
@@ -57,6 +68,19 @@ function getFromEnv(key: string): string {
   return value;
 }
 
-function isClickHouseCloudEnabled() {
-  return process.env['CLICKHOUSE_CLOUD_ENABLED'] === 'true';
+export function getClickHouseTestEnvironment(): TestEnv {
+  let env;
+  switch (process.env['CLICKHOUSE_TEST_ENVIRONMENT']) {
+    case 'CLOUD':
+      env = TestEnv.Cloud;
+      break;
+    case 'LOCAL_CLUSTER':
+      env = TestEnv.LocalCluster;
+      break;
+    default:
+      env = TestEnv.LocalSingleNode;
+      break;
+  }
+  console.log(`Using ${env} test environment`);
+  return env;
 }

.github/workflows/run-tests.yml

@@ -14,8 +14,8 @@ services:
       - '9000:9000'
       - '9181:9181'
     volumes:
-      - './.docker/clickhouse/cluster/config1.xml:/etc/clickhouse-server/config.xml'
-      - './.docker/clickhouse/cluster/macros1.xml:/etc/clickhouse-server/config.d/macros.xml'
+      - './.docker/clickhouse/cluster/server1_config.xml:/etc/clickhouse-server/config.xml'
+      - './.docker/clickhouse/cluster/server1_macros.xml:/etc/clickhouse-server/config.d/macros.xml'
       - './.docker/clickhouse/cluster/users.xml:/etc/clickhouse-server/users.xml'
 
   clickhouse2:
@@ -31,8 +31,8 @@ services:
       - '9001:9000'
       - '9182:9181'
     volumes:
-      - './.docker/clickhouse/cluster/config2.xml:/etc/clickhouse-server/config.xml'
-      - './.docker/clickhouse/cluster/macros2.xml:/etc/clickhouse-server/config.d/macros.xml'
+      - './.docker/clickhouse/cluster/server2_config.xml:/etc/clickhouse-server/config.xml'
+      - './.docker/clickhouse/cluster/server2_macros.xml:/etc/clickhouse-server/config.d/macros.xml'
       - './.docker/clickhouse/cluster/users.xml:/etc/clickhouse-server/users.xml'
 
   # Using Nginx as a cluster entrypoint and a round-robin load balancer for HTTP requests

__tests__/integration/command.test.ts

@@ -44,6 +44,7 @@ export interface SelectParams extends BaseParams {
 
 export interface CommandParams extends BaseParams {
   query: string;
+  format?: DataFormat;
 }
 
 export interface InsertParams extends BaseParams {
@@ -140,13 +141,16 @@ export class ClickHouseClient {
     return new Rows(stream, format);
   }
 
-  async command(params: CommandParams): Promise<void> {
-    const query = params.query.trim();
+  async command(params: CommandParams): Promise<Rows> {
+    const format = params.format ?? 'JSON';
+    const query = formatCommandQuery(params.query, format);
 
-    await this.connection.command({
+    const stream = await this.connection.command({
       query,
       ...this.getBaseParams(params),
     });
+
+    return new Rows(stream, format);
   }
 
   async insert(params: InsertParams): Promise<void> {
@@ -184,6 +188,12 @@ function formatSelectQuery(query: string, format: DataFormat): string {
   return query + ' \nFORMAT ' + format;
 }
 
+// it is a duplicate of `formatSelectQuery`, but it might differ in the future
+function formatCommandQuery(query: string, format: DataFormat): string {
+  query = query.trim();
+  return query + ' \nFORMAT ' + format;
+}
+
 function validateInsertValues(
   values: ReadonlyArray<any> | Stream.Readable
 ): void {

__tests__/integration/insert.test.ts

@@ -15,7 +15,6 @@ import type {
 import { toSearchParams } from './http_search_params';
 import { transformUrl } from './transform_url';
 import { getAsText, isStream } from '../../utils';
-import { Rows } from '../../result';
 
 export interface RequestParams {
   method: 'GET' | 'POST';
@@ -253,23 +252,18 @@ export abstract class BaseHttpAdapter implements Connection {
     });
   }
 
-  async command(params: BaseParams): Promise<void> {
+  async command(params: BaseParams): Promise<Stream.Readable> {
     const searchParams = toSearchParams(
       params.clickhouse_settings,
       params.query_params
     );
 
-    const stream = await this.request({
+    return await this.request({
       method: 'POST',
       url: transformUrl({ url: this.config.host, pathname: '/', searchParams }),
       body: params.query,
       abort_signal: params.abort_signal,
     });
-
-    const rows = new Rows(stream, 'TabSeparated');
-    const text = await rows.text();
-    console.log(`Command returned:\n${text}`);
-    // return await getAsText(result);
   }
 
   async insert(params: InsertParams): Promise<void> {

__tests__/utils/client.ts

@@ -34,7 +34,7 @@ export interface Connection {
   ping(): Promise<boolean>;
   close(): Promise<void>;
   select(params: BaseParams): Promise<Stream.Readable>;
-  command(params: BaseParams): Promise<void>;
+  command(params: BaseParams): Promise<Stream.Readable>;
   insert(params: InsertParams): Promise<void>;
 }