diff --git a/integration-tests/__tests__/alter-table.spec.js b/integration-tests/__tests__/alter-table.spec.js index 62e3df85f..1aec9af5b 100644 --- a/integration-tests/__tests__/alter-table.spec.js +++ b/integration-tests/__tests__/alter-table.spec.js @@ -26,13 +26,15 @@ function pickImportantTableInfo(engine, table) { .map(props => _.omitBy(props, (v, k) => k == 'defaultValue' && v == 'NULL' && engine.setNullDefaultInsteadOfDrop) ), - // foreignKeys: table.foreignKeys - // .sort((a, b) => a.refTableName.localeCompare(b.refTableName)) - // .map(fk => ({ - // constraintType: fk.constraintType, - // refTableName: fk.refTableName, - // columns: fk.columns.map(col => ({ columnName: col.columnName, refColumnName: col.refColumnName })), - // })), + + // TODO: + foreignKeys: table.foreignKeys + .sort((a, b) => a.refTableName.localeCompare(b.refTableName)) + .map(fk => ({ + constraintType: fk.constraintType, + refTableName: fk.refTableName, + columns: fk.columns.map(col => ({ columnName: col.columnName, refColumnName: col.refColumnName })), + })), }; } @@ -103,6 +105,7 @@ async function testTableDiff(engine, conn, driver, mangle, changedTable = 't1') await driver.script(conn, sql); + // TODO: // if (!engine.skipIncrementalAnalysis) { // const structure2RealIncremental = await driver.analyseIncremental(conn, structure1Source); // checkTableStructure(engine, tget(structure2RealIncremental), tget(structure2)); @@ -116,6 +119,7 @@ async function testTableDiff(engine, conn, driver, mangle, changedTable = 't1') const TESTED_COLUMNS = ['col_pk', 'col_std', 'col_def', 'col_fk', 'col_ref', 'col_idx', 'col_uq']; // const TESTED_COLUMNS = ['col_pk']; +// const TESTED_COLUMNS = ['col_fk']; // const TESTED_COLUMNS = ['col_idx']; // const TESTED_COLUMNS = ['col_def']; // const TESTED_COLUMNS = ['col_std']; @@ -179,11 +183,25 @@ describe('Alter table', () => { )( 'Drop column - %s - %s', testWrapper(async (conn, driver, column, engine) => { - await testTableDiff(engine, conn, driver, tbl => (tbl.columns = tbl.columns.filter(x => x.columnName != column))); + await testTableDiff(engine, conn, driver, + tbl => { + tbl.columns = tbl.columns.filter(x => x.columnName != column); + tbl.foreignKeys = tbl.foreignKeys + .map(fk => ({ + ...fk, + columns: fk.columns.filter(col => col.columnName != column) + })) + .filter(fk => fk.columns.length > 0); + } + ); }) ); - test.each(createEnginesColumnsSource(engines.filter(x => !x.skipNullable && !x.skipChangeNullability)))( + test.each( + createEnginesColumnsSource(engines.filter(x => !x.skipNullability && !x.skipChangeNullability)).filter( + ([_label, col]) => !col.endsWith('_pk') + ) + )( 'Change nullability - %s - %s', testWrapper(async (conn, driver, column, engine) => { await testTableDiff( @@ -202,7 +220,11 @@ describe('Alter table', () => { engine, conn, driver, - tbl => (tbl.columns = tbl.columns.map(x => (x.columnName == column ? { ...x, columnName: 'col_renamed' } : x))) + tbl => { + tbl.columns = tbl.columns.map(x => (x.columnName == column ? { ...x, columnName: 'col_renamed' } : x)); + tbl.foreignKeys = tbl.foreignKeys.map(fk => ({...fk, columns: fk.columns.map(col => col.columnName == column ? { ...col, columnName: 'col_renamed' } : col) + })); + } ); }) ); diff --git a/integration-tests/docker-compose.yaml b/integration-tests/docker-compose.yaml index c64836a4d..e48b5ddb1 100644 --- a/integration-tests/docker-compose.yaml +++ b/integration-tests/docker-compose.yaml @@ -44,7 +44,7 @@ services: # - 15942:9042 # # clickhouse: - # image: bitnami/clickhouse:24.8.4 + # image: bitnamilegacy/clickhouse:24.8.4 # restart: always # ports: # - 15005:8123 diff --git a/integration-tests/tools.js b/integration-tests/tools.js index a1a14dfd9..55b316e8a 100644 --- a/integration-tests/tools.js +++ b/integration-tests/tools.js @@ -22,7 +22,9 @@ async function connect(engine, database) { if (engine.generateDbFile) { const conn = await driver.connect({ ...connection, - databaseFile: (engine.databaseFileLocationOnServer ?? 'dbtemp/') + database, + databaseFile: + (engine.databaseFileLocationOnServer ?? (process.env.CITEST ? 'dbtemp/' : 'integration-tests/dbtemp/')) + + database, }); return conn; } else { diff --git a/packages/tools/src/SqlDumper.ts b/packages/tools/src/SqlDumper.ts index dc767c325..b71d97830 100644 --- a/packages/tools/src/SqlDumper.ts +++ b/packages/tools/src/SqlDumper.ts @@ -26,6 +26,7 @@ import _isDate from 'lodash/isDate'; import _isArray from 'lodash/isArray'; import _isPlainObject from 'lodash/isPlainObject'; import _keys from 'lodash/keys'; +import _cloneDeep from 'lodash/cloneDeep'; import uuidv1 from 'uuid/v1'; export class SqlDumper implements AlterProcessor { @@ -667,6 +668,68 @@ export class SqlDumper implements AlterProcessor { } } + sanitizeTableConstraints(table: TableInfo): TableInfo { + // Create a deep copy of the table + const sanitized = _cloneDeep(table); + + // Get the set of existing column names + const existingColumns = new Set(sanitized.columns.map(col => col.columnName)); + + // Filter primary key columns to only include existing columns + if (sanitized.primaryKey) { + const validPkColumns = sanitized.primaryKey.columns.filter(col => existingColumns.has(col.columnName)); + if (validPkColumns.length === 0) { + // If no valid columns remain, remove the primary key entirely + sanitized.primaryKey = null; + } else if (validPkColumns.length < sanitized.primaryKey.columns.length) { + // Update primary key with only valid columns + sanitized.primaryKey = { + ...sanitized.primaryKey, + columns: validPkColumns + }; + } + } + + // Filter sorting key columns to only include existing columns + if (sanitized.sortingKey) { + const validSkColumns = sanitized.sortingKey.columns.filter(col => existingColumns.has(col.columnName)); + if (validSkColumns.length === 0) { + sanitized.sortingKey = null; + } else if (validSkColumns.length < sanitized.sortingKey.columns.length) { + sanitized.sortingKey = { + ...sanitized.sortingKey, + columns: validSkColumns + }; + } + } + + // Filter foreign keys to only include those with all columns present + if (sanitized.foreignKeys) { + sanitized.foreignKeys = sanitized.foreignKeys.filter(fk => + fk.columns.every(col => existingColumns.has(col.columnName)) + ); + } + + // Filter indexes to only include those with all columns present + if (sanitized.indexes) { + sanitized.indexes = sanitized.indexes.filter(idx => + idx.columns.every(col => existingColumns.has(col.columnName)) + ); + } + + // Filter unique constraints to only include those with all columns present + if (sanitized.uniques) { + sanitized.uniques = sanitized.uniques.filter(uq => + uq.columns.every(col => existingColumns.has(col.columnName)) + ); + } + + // Filter dependencies (references from other tables) - these should remain as-is + // since they don't affect the CREATE TABLE statement for this table + + return sanitized; + } + recreateTable(oldTable: TableInfo, newTable: TableInfo) { if (!oldTable.pairingId || !newTable.pairingId || oldTable.pairingId != newTable.pairingId) { throw new Error('Recreate is not possible: oldTable.paringId != newTable.paringId'); @@ -681,48 +744,51 @@ export class SqlDumper implements AlterProcessor { })) .filter(x => x.newcol); + // Create a sanitized version of newTable with constraints that only reference existing columns + const sanitizedNewTable = this.sanitizeTableConstraints(newTable); + if (this.driver.supportsTransactions) { this.dropConstraints(oldTable, true); this.renameTable(oldTable, tmpTable); - this.createTable(newTable); + this.createTable(sanitizedNewTable); - const autoinc = newTable.columns.find(x => x.autoIncrement); + const autoinc = sanitizedNewTable.columns.find(x => x.autoIncrement); if (autoinc) { - this.allowIdentityInsert(newTable, true); + this.allowIdentityInsert(sanitizedNewTable, true); } this.putCmd( '^insert ^into %f (%,i) select %,i ^from %f', - newTable, + sanitizedNewTable, columnPairs.map(x => x.newcol.columnName), columnPairs.map(x => x.oldcol.columnName), { ...oldTable, pureName: tmpTable } ); if (autoinc) { - this.allowIdentityInsert(newTable, false); + this.allowIdentityInsert(sanitizedNewTable, false); } if (this.dialect.dropForeignKey) { - newTable.dependencies.forEach(cnt => this.createConstraint(cnt)); + sanitizedNewTable.dependencies.forEach(cnt => this.createConstraint(cnt)); } this.dropTable({ ...oldTable, pureName: tmpTable }); } else { // we have to preserve old table as long as possible - this.createTable({ ...newTable, pureName: tmpTable }); + this.createTable({ ...sanitizedNewTable, pureName: tmpTable }); this.putCmd( '^insert ^into %f (%,i) select %,s ^from %f', - { ...newTable, pureName: tmpTable }, + { ...sanitizedNewTable, pureName: tmpTable }, columnPairs.map(x => x.newcol.columnName), columnPairs.map(x => x.oldcol.columnName), oldTable ); this.dropTable(oldTable); - this.renameTable({ ...newTable, pureName: tmpTable }, newTable.pureName); + this.renameTable({ ...sanitizedNewTable, pureName: tmpTable }, newTable.pureName); } } diff --git a/packages/tools/src/alterPlan.ts b/packages/tools/src/alterPlan.ts index 5462efab8..a1a8dfb78 100644 --- a/packages/tools/src/alterPlan.ts +++ b/packages/tools/src/alterPlan.ts @@ -91,8 +91,8 @@ interface AlterOperation_RenameConstraint { } interface AlterOperation_RecreateTable { operationType: 'recreateTable'; - table: TableInfo; - operations: AlterOperation[]; + oldTable: TableInfo; + newTable: TableInfo; } interface AlterOperation_FillPreloadedRows { operationType: 'fillPreloadedRows'; @@ -249,11 +249,11 @@ export class AlterPlan { }); } - recreateTable(table: TableInfo, operations: AlterOperation[]) { + recreateTable(oldTable: TableInfo, newTable: TableInfo) { this.operations.push({ operationType: 'recreateTable', - table, - operations, + oldTable, + newTable, }); this.recreates.tables += 1; } @@ -337,7 +337,13 @@ export class AlterPlan { return opRes; }), op, - ]; + ].filter(op => { + // filter duplicated drops + const existingDrop = this.operations.find( + o => o.operationType == 'dropConstraint' && o.oldObject === op['oldObject'] + ); + return existingDrop == null; + }); return res; } @@ -498,53 +504,121 @@ export class AlterPlan { return []; } - const table = this.wholeNewDb.tables.find( + const oldTable = this.wholeOldDb.tables.find( + x => x.pureName == op[objectField].pureName && x.schemaName == op[objectField].schemaName + ); + const newTable = this.wholeNewDb.tables.find( x => x.pureName == op[objectField].pureName && x.schemaName == op[objectField].schemaName ); this.recreates.tables += 1; return [ { operationType: 'recreateTable', - table, - operations: [op], + oldTable, + newTable, + // operations: [op], }, ]; } return null; } - _groupTableRecreations(): AlterOperation[] { - const res = []; - const recreates = {}; + _removeRecreatedTableAlters(): AlterOperation[] { + const res: AlterOperation[] = []; + const recreates = new Set(); for (const op of this.operations) { - if (op.operationType == 'recreateTable' && op.table) { - const existingRecreate = recreates[`${op.table.schemaName}||${op.table.pureName}`]; - if (existingRecreate) { - existingRecreate.operations.push(...op.operations); - } else { - const recreate = { - ...op, - operations: [...op.operations], - }; - res.push(recreate); - recreates[`${op.table.schemaName}||${op.table.pureName}`] = recreate; - } - } else { - // @ts-ignore - const oldObject: TableInfo = op.oldObject || op.object; - if (oldObject) { - const recreated = recreates[`${oldObject.schemaName}||${oldObject.pureName}`]; - if (recreated) { - recreated.operations.push(op); - continue; - } - } - res.push(op); + if (op.operationType == 'recreateTable' && op.oldTable && op.newTable) { + const key = `${op.oldTable.schemaName}||${op.oldTable.pureName}`; + recreates.add(key); } } + + for (const op of this.operations) { + switch (op.operationType) { + case 'createColumn': + case 'createConstraint': + { + const key = `${op.newObject.schemaName}||${op.newObject.pureName}`; + if (recreates.has(key)) { + // skip create inside recreated table + continue; + } + } + break; + case 'dropColumn': + case 'dropConstraint': + case 'changeColumn': + { + const key = `${op.oldObject.schemaName}||${op.oldObject.pureName}`; + if (recreates.has(key)) { + // skip drop/change inside recreated table + continue; + } + } + break; + case 'renameColumn': + { + const key = `${op.object.schemaName}||${op.object.pureName}`; + if (recreates.has(key)) { + // skip rename inside recreated table + continue; + } + } + break; + } + res.push(op); + } return res; } + _groupTableRecreations(): AlterOperation[] { + const res = []; + const recreates = new Set(); + for (const op of this.operations) { + if (op.operationType == 'recreateTable' && op.oldTable && op.newTable) { + const key = `${op.oldTable.schemaName}||${op.oldTable.pureName}`; + if (recreates.has(key)) { + // prevent duplicate recreates + continue; + } + recreates.add(key); + } + + res.push(op); + } + return res; + + // const res = []; + // const recreates = {}; + // for (const op of this.operations) { + // if (op.operationType == 'recreateTable' && op.table) { + // const existingRecreate = recreates[`${op.table.schemaName}||${op.table.pureName}`]; + // if (existingRecreate) { + // existingRecreate.operations.push(...op.operations); + // } else { + // const recreate = { + // ...op, + // operations: [...op.operations], + // }; + // res.push(recreate); + // recreates[`${op.table.schemaName}||${op.table.pureName}`] = recreate; + // } + // } else { + // // @ts-ignore + // const oldObject: TableInfo = op.oldObject || op.object; + // if (oldObject) { + // const recreated = recreates[`${oldObject.schemaName}||${oldObject.pureName}`]; + // if (recreated) { + // recreated.operations.push(op); + // continue; + // } + // } + // res.push(op); + // } + // } + // return res; + } + _moveForeignKeysToLast(): AlterOperation[] { if (!this.dialect.createForeignKey) { return this.operations; @@ -611,6 +685,8 @@ export class AlterPlan { // console.log('*****************OPERATIONS3', this.operations); + this.operations = this._removeRecreatedTableAlters(); + this.operations = this._moveForeignKeysToLast(); // console.log('*****************OPERATIONS4', this.operations); @@ -673,16 +749,16 @@ export function runAlterOperation(op: AlterOperation, processor: AlterProcessor) break; case 'recreateTable': { - const oldTable = generateTablePairingId(op.table); - const newTable = _.cloneDeep(oldTable); - const newDb = DatabaseAnalyser.createEmptyStructure(); - newDb.tables.push(newTable); - // console.log('////////////////////////////newTable1', newTable); - op.operations.forEach(child => runAlterOperation(child, new DatabaseInfoAlterProcessor(newDb))); - // console.log('////////////////////////////op.operations', op.operations); - // console.log('////////////////////////////op.table', op.table); - // console.log('////////////////////////////newTable2', newTable); - processor.recreateTable(oldTable, newTable); + // const oldTable = generateTablePairingId(op.table); + // const newTable = _.cloneDeep(oldTable); + // const newDb = DatabaseAnalyser.createEmptyStructure(); + // newDb.tables.push(newTable); + // // console.log('////////////////////////////newTable1', newTable); + // op.operations.forEach(child => runAlterOperation(child, new DatabaseInfoAlterProcessor(newDb))); + // // console.log('////////////////////////////op.operations', op.operations); + // // console.log('////////////////////////////op.table', op.table); + // // console.log('////////////////////////////newTable2', newTable); + processor.recreateTable(op.oldTable, op.newTable); } break; } diff --git a/plugins/dbgate-plugin-mysql/src/frontend/drivers.js b/plugins/dbgate-plugin-mysql/src/frontend/drivers.js index 9ae5c4946..0ed90f494 100644 --- a/plugins/dbgate-plugin-mysql/src/frontend/drivers.js +++ b/plugins/dbgate-plugin-mysql/src/frontend/drivers.js @@ -46,6 +46,8 @@ const dialect = { dropReferencesWhenDropTable: false, requireStandaloneSelectForScopeIdentity: true, + dropColumnDependencies: ['dependencies'], + columnProperties: { columnComment: true, isUnsigned: true, diff --git a/plugins/dbgate-plugin-oracle/src/frontend/Dumper.js b/plugins/dbgate-plugin-oracle/src/frontend/Dumper.js index 84d2ba84b..1c7aef034 100644 --- a/plugins/dbgate-plugin-oracle/src/frontend/Dumper.js +++ b/plugins/dbgate-plugin-oracle/src/frontend/Dumper.js @@ -64,6 +64,21 @@ class Dumper extends SqlDumper { this.putCmd('^alter ^table %f ^rename ^column %i ^to %i', column, column.columnName, newcol); } + createForeignKeyFore(fk) { + if (fk.constraintName != null && !this.dialect.anonymousForeignKey) { + this.put('^constraint %i ', fk.constraintName); + } + this.put( + '^foreign ^key (%,i) ^references %f (%,i)', + fk.columns.map(x => x.columnName), + { schemaName: fk.refSchemaName, pureName: fk.refTableName }, + fk.columns.map(x => x.refColumnName) + ); + if (fk.deleteAction && fk.deleteAction.toUpperCase() !== 'NO ACTION') { + this.put(' ^on ^delete %k', fk.deleteAction); + } + } + // dropTable(obj, options = {}) { // this.put('^drop ^table'); // if (options.testIfExists) this.put(' ^if ^exists'); diff --git a/plugins/dbgate-plugin-sqlite/src/backend/Analyser.js b/plugins/dbgate-plugin-sqlite/src/backend/Analyser.js index 4dde1fb4a..fe9e951ff 100644 --- a/plugins/dbgate-plugin-sqlite/src/backend/Analyser.js +++ b/plugins/dbgate-plugin-sqlite/src/backend/Analyser.js @@ -129,6 +129,7 @@ class Analyser extends DatabaseAnalyser { updateAction: fkcol.on_update, deleteAction: fkcol.on_delete, constraintName: `FK_${tableName}_${fkcol.id}`, + constraintType: 'foreignKey', }; return fk; });