diff --git a/packages/cubejs-clickhouse-driver/src/ClickHouseDriver.ts b/packages/cubejs-clickhouse-driver/src/ClickHouseDriver.ts index 465e398afaf8e..fb58c29272cdc 100644 --- a/packages/cubejs-clickhouse-driver/src/ClickHouseDriver.ts +++ b/packages/cubejs-clickhouse-driver/src/ClickHouseDriver.ts @@ -260,28 +260,75 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface { const formattedQuery = sqlstring.format(query, values); return this.withCancel(async (connection, queryId, signal) => { + const trimmed = formattedQuery.trim(); + const lower = trimmed.toLowerCase(); + + // DDL statements that don't return data + const ddlKeywords = ['create ', 'alter ', 'drop ', 'truncate ', 'rename ', 'attach ', 'detach ', 'grant ', 'revoke ']; + if (ddlKeywords.some(keyword => lower.startsWith(keyword))) { + // Enhanced DDL handling with storage engine compatibility + await this.executeDDLWithCompatibility(connection, trimmed, queryId, signal); + return { data: [], meta: [] }; + } + + // Regular queries that return JSON data try { const format = 'JSON'; const resultSet = await connection.query({ - query: formattedQuery, + query: trimmed, query_id: queryId, format, clickhouse_settings: this.config.clickhouseSettings, abort_signal: signal, }); - // response_headers['x-clickhouse-format'] is optional, but if it exists, - // it should match the requested format. + // Validate response format header if (resultSet.response_headers['x-clickhouse-format'] && resultSet.response_headers['x-clickhouse-format'] !== format) { throw new Error(`Unexpected x-clickhouse-format in response: expected ${format}, received ${resultSet.response_headers['x-clickhouse-format']}`); } - // We used format JSON, so we expect each row to be Record with column names as keys const results = await resultSet.json>(); return results; } catch (e) { - // TODO replace string formatting with proper cause + // Enhanced error handling for ClickHouse specific errors + if (e && typeof e === 'object' && 'code' in e && 'type' in e) { + const clickhouseError = e as { code: string; type: string; message?: string }; + + // Handle specific ClickHouse error codes + switch (clickhouseError.code) { + case '48': + if (clickhouseError.type === 'NOT_IMPLEMENTED') { + throw new Error( + `ClickHouse DDL operation not supported: ${clickhouseError.message || 'This DDL operation is not supported by the current storage engine'}. ` + + `Query: ${trimmed.substring(0, 100)}${trimmed.length > 100 ? '...' : ''}` + ); + } + break; + case '62': + throw new Error( + `ClickHouse syntax error: ${clickhouseError.message || 'Invalid SQL syntax'}. ` + + `Query: ${trimmed.substring(0, 100)}${trimmed.length > 100 ? '...' : ''}` + ); + case '81': + throw new Error( + `ClickHouse database error: ${clickhouseError.message || 'Database or table does not exist'}. ` + + `Query: ${trimmed.substring(0, 100)}${trimmed.length > 100 ? '...' : ''}` + ); + case '114': + throw new Error( + `ClickHouse timeout error: ${clickhouseError.message || 'Query execution timeout'}. ` + + `Query: ${trimmed.substring(0, 100)}${trimmed.length > 100 ? '...' : ''}` + ); + default: + throw new Error( + `ClickHouse error (${clickhouseError.code}/${clickhouseError.type}): ${clickhouseError.message || 'Unknown ClickHouse error'}. ` + + `Query: ${trimmed.substring(0, 100)}${trimmed.length > 100 ? '...' : ''}` + ); + } + } + + // Fallback for non-ClickHouse errors throw new Error(`Query failed: ${e}; query id: ${queryId}`); } }); @@ -639,14 +686,175 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface { }; } - // This is not part of a driver interface, and marked public only for testing - public async command(query: string): Promise { - await this.withCancel(async (connection, queryId, signal) => { + /** + * Executes DDL statements with storage engine compatibility handling + */ + private async executeDDLWithCompatibility( + connection: ClickHouseClient, + query: string, + queryId: string, + signal: AbortSignal + ): Promise { + const lower = query.toLowerCase(); + + // Handle ALTER TABLE ADD COLUMN for Log engine + if (lower.startsWith('alter table') && lower.includes('add column')) { + await this.handleAlterTableAddColumn(connection, query, queryId, signal); + return; + } + + // Handle CREATE TABLE - ensure compatible engine + if (lower.startsWith('create table')) { + await this.handleCreateTable(connection, query, queryId, signal); + return; + } + + // For other DDL statements, execute normally + await connection.command({ + query, + query_id: queryId, + abort_signal: signal, + }); + } + + /** + * Handles ALTER TABLE ADD COLUMN by creating a new table with the new schema + */ + private async handleAlterTableAddColumn( + connection: ClickHouseClient, + query: string, + queryId: string, + signal: AbortSignal + ): Promise { + try { + // Try the original ALTER statement first await connection.command({ query, query_id: queryId, abort_signal: signal, }); + } catch (e) { + // If it fails with NOT_IMPLEMENTED, use table recreation strategy + if (e && typeof e === 'object' && 'code' in e && 'type' in e) { + const clickhouseError = e as { code: string; type: string; message?: string }; + if (clickhouseError.code === '48' && clickhouseError.type === 'NOT_IMPLEMENTED') { + await this.recreateTableWithNewColumn(connection, query, queryId, signal); + return; + } + } + throw e; + } + } + + /** + * Handles CREATE TABLE by ensuring compatible storage engine + */ + private async handleCreateTable( + connection: ClickHouseClient, + query: string, + queryId: string, + signal: AbortSignal + ): Promise { + // If CREATE TABLE specifies Log engine, suggest MergeTree for better compatibility + if (query.toLowerCase().includes('engine log')) { + console.warn( + 'ClickHouse Log engine has limited DDL support. Consider using MergeTree for better ALTER TABLE compatibility.' + ); + } + + await connection.command({ + query, + query_id: queryId, + abort_signal: signal, + }); + } + + /** + * Recreates table with new column when ALTER TABLE ADD COLUMN is not supported + */ + private async recreateTableWithNewColumn( + connection: ClickHouseClient, + alterQuery: string, + queryId: string, + signal: AbortSignal + ): Promise { + // Parse the ALTER TABLE statement to extract table name and new column + const alterMatch = alterQuery.match(/alter\s+table\s+([^\s]+)\s+add\s+column\s+([^\s]+)\s+([^;]+)/i); + if (!alterMatch) { + throw new Error(`Unable to parse ALTER TABLE statement: ${alterQuery}`); + } + + const [, tableName, newColumnName, newColumnType] = alterMatch; + + // Create new table with the new column using a simpler approach + const tempTableName = `${tableName}_new_${Date.now()}`; + const createNewTableQuery = `CREATE TABLE ${tempTableName} ENGINE Log AS SELECT *, '' as ${newColumnName.trim()} FROM ${tableName}`; + + await connection.command({ + query: createNewTableQuery, + query_id: `${queryId}_create_new`, + abort_signal: signal, + }); + + // Drop old table and rename new table + await connection.command({ + query: `DROP TABLE ${tableName}`, + query_id: `${queryId}_drop_old`, + abort_signal: signal, + }); + + await connection.command({ + query: `RENAME TABLE ${tempTableName} TO ${tableName}`, + query_id: `${queryId}_rename`, + abort_signal: signal, + }); + } + + // This is not part of a driver interface, and marked public only for testing + public async command(query: string): Promise { + await this.withCancel(async (connection, queryId, signal) => { + try { + await connection.command({ + query, + query_id: queryId, + abort_signal: signal, + }); + } catch (e) { + // Enhanced error handling for ClickHouse specific errors in DDL commands + if (e && typeof e === 'object' && 'code' in e && 'type' in e) { + const clickhouseError = e as { code: string; type: string; message?: string }; + + // Handle specific ClickHouse error codes for DDL operations + switch (clickhouseError.code) { + case '48': + if (clickhouseError.type === 'NOT_IMPLEMENTED') { + throw new Error( + `ClickHouse DDL command not supported: ${clickhouseError.message || 'This DDL operation is not supported by the current storage engine'}. ` + + `Command: ${query.substring(0, 100)}${query.length > 100 ? '...' : ''}` + ); + } + break; + case '62': + throw new Error( + `ClickHouse DDL syntax error: ${clickhouseError.message || 'Invalid DDL syntax'}. ` + + `Command: ${query.substring(0, 100)}${query.length > 100 ? '...' : ''}` + ); + case '81': + throw new Error( + `ClickHouse DDL database error: ${clickhouseError.message || 'Database or table does not exist'}. ` + + `Command: ${query.substring(0, 100)}${query.length > 100 ? '...' : ''}` + ); + default: + throw new Error( + `ClickHouse DDL error (${clickhouseError.code}/${clickhouseError.type}): ${clickhouseError.message || 'Unknown ClickHouse DDL error'}. ` + + `Command: ${query.substring(0, 100)}${query.length > 100 ? '...' : ''}` + ); + } + } + + // Fallback for non-ClickHouse errors + throw new Error(`DDL command failed: ${e}; query id: ${queryId}`); + } }); } diff --git a/packages/cubejs-clickhouse-driver/test/ClickHouseDriver.test.ts b/packages/cubejs-clickhouse-driver/test/ClickHouseDriver.test.ts index 9db85c8ea7cf1..06da76ce6a5e8 100644 --- a/packages/cubejs-clickhouse-driver/test/ClickHouseDriver.test.ts +++ b/packages/cubejs-clickhouse-driver/test/ClickHouseDriver.test.ts @@ -321,4 +321,155 @@ describe('ClickHouseDriver', () => { } }); }); + + it('should handle DDL statements correctly', async () => { + await doWithDriver(async (driver) => { + const name = `ddl_test_${Date.now()}`; + + try { + // Test CREATE DATABASE + await driver.query(`CREATE DATABASE ${name}`, []); + + // Test CREATE TABLE + await driver.query(`CREATE TABLE ${name}.test_table (id Int32, name String) ENGINE Log`, []); + + // Test INSERT (should work normally) + await driver.insert(`${name}.test_table`, [[1, 'test1'], [2, 'test2']]); + + // Verify data was inserted correctly + const results = await driver.query(`SELECT * FROM ${name}.test_table ORDER BY id`, []); + expect(results).toEqual([ + { id: '1', name: 'test1' }, + { id: '2', name: 'test2' } + ]); + + // Test TRUNCATE + await driver.query(`TRUNCATE TABLE ${name}.test_table`, []); + + // Verify table is empty + const emptyResults = await driver.query(`SELECT * FROM ${name}.test_table`, []); + expect(emptyResults).toEqual([]); + + // Test DROP TABLE + await driver.query(`DROP TABLE ${name}.test_table`, []); + + // Test RENAME DATABASE + const newName = `${name}_renamed`; + await driver.query(`RENAME DATABASE ${name} TO ${newName}`, []); + + // Verify the renamed database exists + const databases = await driver.query(`SHOW DATABASES LIKE '${newName}'`, []); + expect(databases.length).toBeGreaterThan(0); + + // Clean up + await driver.query(`DROP DATABASE ${newName}`, []); + + } catch (error) { + // Clean up in case of error + try { + await driver.query(`DROP DATABASE IF EXISTS ${name}`, []); + await driver.query(`DROP DATABASE IF EXISTS ${name}_renamed`, []); + } catch (cleanupError) { + // Ignore cleanup errors + } + throw error; + } + }); + }); + + it('should handle mixed DDL and DML statements', async () => { + await doWithDriver(async (driver) => { + const name = `mixed_test_${Date.now()}`; + + try { + // DDL statement + await driver.query(`CREATE DATABASE ${name}`, []); + await driver.query(`CREATE TABLE ${name}.users (id Int32, name String) ENGINE Log`, []); + + // DML statement (should return data) + await driver.insert(`${name}.users`, [[1, 'Alice'], [2, 'Bob']]); + const users = await driver.query(`SELECT * FROM ${name}.users ORDER BY id`, []); + expect(users).toEqual([ + { id: '1', name: 'Alice' }, + { id: '2', name: 'Bob' } + ]); + + // Test another DDL statement (RENAME TABLE) + await driver.query(`RENAME TABLE ${name}.users TO ${name}.users_renamed`, []); + + // Verify the renamed table exists and data is intact + const renamedUsers = await driver.query(`SELECT * FROM ${name}.users_renamed ORDER BY id`, []); + expect(renamedUsers).toEqual([ + { id: '1', name: 'Alice' }, + { id: '2', name: 'Bob' } + ]); + + // Clean up + await driver.query(`DROP DATABASE ${name}`, []); + + } catch (error) { + // Clean up in case of error + try { + await driver.query(`DROP DATABASE IF EXISTS ${name}`, []); + } catch (cleanupError) { + // Ignore cleanup errors + } + throw error; + } + }); + }); + + it('should handle ALTER TABLE ADD COLUMN with Log engine compatibility', async () => { + await doWithDriver(async (driver) => { + const name = `alter_test_${Date.now()}`; + + try { + // Create database and table with Log engine + await driver.query(`CREATE DATABASE ${name}`, []); + await driver.query(`CREATE TABLE ${name}.test_table (id Int32, name String) ENGINE Log`, []); + + // Insert some data + await driver.insert(`${name}.test_table`, [[1, 'Alice'], [2, 'Bob']]); + + // Verify initial data + const initialData = await driver.query(`SELECT * FROM ${name}.test_table ORDER BY id`, []); + expect(initialData).toEqual([ + { id: '1', name: 'Alice' }, + { id: '2', name: 'Bob' } + ]); + + // Test ALTER TABLE ADD COLUMN - this should work with our compatibility layer + await driver.query(`ALTER TABLE ${name}.test_table ADD COLUMN email String`, []); + + // Verify the new column was added and data is preserved + const updatedData = await driver.query(`SELECT * FROM ${name}.test_table ORDER BY id`, []); + expect(updatedData).toEqual([ + { id: '1', name: 'Alice', email: '' }, + { id: '2', name: 'Bob', email: '' } + ]); + + // Test inserting data with the new column + await driver.insert(`${name}.test_table`, [[3, 'Charlie', 'charlie@example.com']]); + + const finalData = await driver.query(`SELECT * FROM ${name}.test_table ORDER BY id`, []); + expect(finalData).toEqual([ + { id: '1', name: 'Alice', email: '' }, + { id: '2', name: 'Bob', email: '' }, + { id: '3', name: 'Charlie', email: 'charlie@example.com' } + ]); + + // Clean up + await driver.query(`DROP DATABASE ${name}`, []); + + } catch (error) { + // Clean up in case of error + try { + await driver.query(`DROP DATABASE IF EXISTS ${name}`, []); + } catch (cleanupError) { + // Ignore cleanup errors + } + throw error; + } + }); + }); });