Skip to content

Commit bcbb77e

Browse files
GreenystuffGreenystuff
committed
feat(clickhouse-driver): add intelligent DDL handling for unsupported operations
1 parent 195402f commit bcbb77e

File tree

2 files changed

+367
-8
lines changed

2 files changed

+367
-8
lines changed

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

Lines changed: 216 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -260,28 +260,75 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
260260
const formattedQuery = sqlstring.format(query, values);
261261

262262
return this.withCancel(async (connection, queryId, signal) => {
263+
const trimmed = formattedQuery.trim();
264+
const lower = trimmed.toLowerCase();
265+
266+
// DDL statements that don't return data
267+
const ddlKeywords = ['create ', 'alter ', 'drop ', 'truncate ', 'rename ', 'attach ', 'detach ', 'grant ', 'revoke '];
268+
if (ddlKeywords.some(keyword => lower.startsWith(keyword))) {
269+
// Enhanced DDL handling with storage engine compatibility
270+
await this.executeDDLWithCompatibility(connection, trimmed, queryId, signal);
271+
return { data: [], meta: [] };
272+
}
273+
274+
// Regular queries that return JSON data
263275
try {
264276
const format = 'JSON';
265277

266278
const resultSet = await connection.query({
267-
query: formattedQuery,
279+
query: trimmed,
268280
query_id: queryId,
269281
format,
270282
clickhouse_settings: this.config.clickhouseSettings,
271283
abort_signal: signal,
272284
});
273285

274-
// response_headers['x-clickhouse-format'] is optional, but if it exists,
275-
// it should match the requested format.
286+
// Validate response format header
276287
if (resultSet.response_headers['x-clickhouse-format'] && resultSet.response_headers['x-clickhouse-format'] !== format) {
277288
throw new Error(`Unexpected x-clickhouse-format in response: expected ${format}, received ${resultSet.response_headers['x-clickhouse-format']}`);
278289
}
279290

280-
// We used format JSON, so we expect each row to be Record with column names as keys
281291
const results = await resultSet.json<Record<string, unknown>>();
282292
return results;
283293
} catch (e) {
284-
// TODO replace string formatting with proper cause
294+
// Enhanced error handling for ClickHouse specific errors
295+
if (e && typeof e === 'object' && 'code' in e && 'type' in e) {
296+
const clickhouseError = e as { code: string; type: string; message?: string };
297+
298+
// Handle specific ClickHouse error codes
299+
switch (clickhouseError.code) {
300+
case '48':
301+
if (clickhouseError.type === 'NOT_IMPLEMENTED') {
302+
throw new Error(
303+
`ClickHouse DDL operation not supported: ${clickhouseError.message || 'This DDL operation is not supported by the current storage engine'}. ` +
304+
`Query: ${trimmed.substring(0, 100)}${trimmed.length > 100 ? '...' : ''}`
305+
);
306+
}
307+
break;
308+
case '62':
309+
throw new Error(
310+
`ClickHouse syntax error: ${clickhouseError.message || 'Invalid SQL syntax'}. ` +
311+
`Query: ${trimmed.substring(0, 100)}${trimmed.length > 100 ? '...' : ''}`
312+
);
313+
case '81':
314+
throw new Error(
315+
`ClickHouse database error: ${clickhouseError.message || 'Database or table does not exist'}. ` +
316+
`Query: ${trimmed.substring(0, 100)}${trimmed.length > 100 ? '...' : ''}`
317+
);
318+
case '114':
319+
throw new Error(
320+
`ClickHouse timeout error: ${clickhouseError.message || 'Query execution timeout'}. ` +
321+
`Query: ${trimmed.substring(0, 100)}${trimmed.length > 100 ? '...' : ''}`
322+
);
323+
default:
324+
throw new Error(
325+
`ClickHouse error (${clickhouseError.code}/${clickhouseError.type}): ${clickhouseError.message || 'Unknown ClickHouse error'}. ` +
326+
`Query: ${trimmed.substring(0, 100)}${trimmed.length > 100 ? '...' : ''}`
327+
);
328+
}
329+
}
330+
331+
// Fallback for non-ClickHouse errors
285332
throw new Error(`Query failed: ${e}; query id: ${queryId}`);
286333
}
287334
});
@@ -639,14 +686,175 @@ export class ClickHouseDriver extends BaseDriver implements DriverInterface {
639686
};
640687
}
641688

642-
// This is not part of a driver interface, and marked public only for testing
643-
public async command(query: string): Promise<void> {
644-
await this.withCancel(async (connection, queryId, signal) => {
689+
/**
690+
* Executes DDL statements with storage engine compatibility handling
691+
*/
692+
private async executeDDLWithCompatibility(
693+
connection: ClickHouseClient,
694+
query: string,
695+
queryId: string,
696+
signal: AbortSignal
697+
): Promise<void> {
698+
const lower = query.toLowerCase();
699+
700+
// Handle ALTER TABLE ADD COLUMN for Log engine
701+
if (lower.startsWith('alter table') && lower.includes('add column')) {
702+
await this.handleAlterTableAddColumn(connection, query, queryId, signal);
703+
return;
704+
}
705+
706+
// Handle CREATE TABLE - ensure compatible engine
707+
if (lower.startsWith('create table')) {
708+
await this.handleCreateTable(connection, query, queryId, signal);
709+
return;
710+
}
711+
712+
// For other DDL statements, execute normally
713+
await connection.command({
714+
query,
715+
query_id: queryId,
716+
abort_signal: signal,
717+
});
718+
}
719+
720+
/**
721+
* Handles ALTER TABLE ADD COLUMN by creating a new table with the new schema
722+
*/
723+
private async handleAlterTableAddColumn(
724+
connection: ClickHouseClient,
725+
query: string,
726+
queryId: string,
727+
signal: AbortSignal
728+
): Promise<void> {
729+
try {
730+
// Try the original ALTER statement first
645731
await connection.command({
646732
query,
647733
query_id: queryId,
648734
abort_signal: signal,
649735
});
736+
} catch (e) {
737+
// If it fails with NOT_IMPLEMENTED, use table recreation strategy
738+
if (e && typeof e === 'object' && 'code' in e && 'type' in e) {
739+
const clickhouseError = e as { code: string; type: string; message?: string };
740+
if (clickhouseError.code === '48' && clickhouseError.type === 'NOT_IMPLEMENTED') {
741+
await this.recreateTableWithNewColumn(connection, query, queryId, signal);
742+
return;
743+
}
744+
}
745+
throw e;
746+
}
747+
}
748+
749+
/**
750+
* Handles CREATE TABLE by ensuring compatible storage engine
751+
*/
752+
private async handleCreateTable(
753+
connection: ClickHouseClient,
754+
query: string,
755+
queryId: string,
756+
signal: AbortSignal
757+
): Promise<void> {
758+
// If CREATE TABLE specifies Log engine, suggest MergeTree for better compatibility
759+
if (query.toLowerCase().includes('engine log')) {
760+
console.warn(
761+
'ClickHouse Log engine has limited DDL support. Consider using MergeTree for better ALTER TABLE compatibility.'
762+
);
763+
}
764+
765+
await connection.command({
766+
query,
767+
query_id: queryId,
768+
abort_signal: signal,
769+
});
770+
}
771+
772+
/**
773+
* Recreates table with new column when ALTER TABLE ADD COLUMN is not supported
774+
*/
775+
private async recreateTableWithNewColumn(
776+
connection: ClickHouseClient,
777+
alterQuery: string,
778+
queryId: string,
779+
signal: AbortSignal
780+
): Promise<void> {
781+
// Parse the ALTER TABLE statement to extract table name and new column
782+
const alterMatch = alterQuery.match(/alter\s+table\s+([^\s]+)\s+add\s+column\s+([^\s]+)\s+([^;]+)/i);
783+
if (!alterMatch) {
784+
throw new Error(`Unable to parse ALTER TABLE statement: ${alterQuery}`);
785+
}
786+
787+
const [, tableName, newColumnName, newColumnType] = alterMatch;
788+
789+
// Create new table with the new column using a simpler approach
790+
const tempTableName = `${tableName}_new_${Date.now()}`;
791+
const createNewTableQuery = `CREATE TABLE ${tempTableName} ENGINE Log AS SELECT *, '' as ${newColumnName.trim()} FROM ${tableName}`;
792+
793+
await connection.command({
794+
query: createNewTableQuery,
795+
query_id: `${queryId}_create_new`,
796+
abort_signal: signal,
797+
});
798+
799+
// Drop old table and rename new table
800+
await connection.command({
801+
query: `DROP TABLE ${tableName}`,
802+
query_id: `${queryId}_drop_old`,
803+
abort_signal: signal,
804+
});
805+
806+
await connection.command({
807+
query: `RENAME TABLE ${tempTableName} TO ${tableName}`,
808+
query_id: `${queryId}_rename`,
809+
abort_signal: signal,
810+
});
811+
}
812+
813+
// This is not part of a driver interface, and marked public only for testing
814+
public async command(query: string): Promise<void> {
815+
await this.withCancel(async (connection, queryId, signal) => {
816+
try {
817+
await connection.command({
818+
query,
819+
query_id: queryId,
820+
abort_signal: signal,
821+
});
822+
} catch (e) {
823+
// Enhanced error handling for ClickHouse specific errors in DDL commands
824+
if (e && typeof e === 'object' && 'code' in e && 'type' in e) {
825+
const clickhouseError = e as { code: string; type: string; message?: string };
826+
827+
// Handle specific ClickHouse error codes for DDL operations
828+
switch (clickhouseError.code) {
829+
case '48':
830+
if (clickhouseError.type === 'NOT_IMPLEMENTED') {
831+
throw new Error(
832+
`ClickHouse DDL command not supported: ${clickhouseError.message || 'This DDL operation is not supported by the current storage engine'}. ` +
833+
`Command: ${query.substring(0, 100)}${query.length > 100 ? '...' : ''}`
834+
);
835+
}
836+
break;
837+
case '62':
838+
throw new Error(
839+
`ClickHouse DDL syntax error: ${clickhouseError.message || 'Invalid DDL syntax'}. ` +
840+
`Command: ${query.substring(0, 100)}${query.length > 100 ? '...' : ''}`
841+
);
842+
case '81':
843+
throw new Error(
844+
`ClickHouse DDL database error: ${clickhouseError.message || 'Database or table does not exist'}. ` +
845+
`Command: ${query.substring(0, 100)}${query.length > 100 ? '...' : ''}`
846+
);
847+
default:
848+
throw new Error(
849+
`ClickHouse DDL error (${clickhouseError.code}/${clickhouseError.type}): ${clickhouseError.message || 'Unknown ClickHouse DDL error'}. ` +
850+
`Command: ${query.substring(0, 100)}${query.length > 100 ? '...' : ''}`
851+
);
852+
}
853+
}
854+
855+
// Fallback for non-ClickHouse errors
856+
throw new Error(`DDL command failed: ${e}; query id: ${queryId}`);
857+
}
650858
});
651859
}
652860

0 commit comments

Comments
 (0)