diff --git a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts index 55a9e1a7f76..7607f25ce66 100644 --- a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts +++ b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts @@ -443,90 +443,25 @@ export abstract class AbstractSqliteQueryRunner * Renames the given table. */ async renameTable( - oldTableOrName: Table | string, + tableOrName: Table | string, newTableName: string, ): Promise { - const oldTable = InstanceChecker.isTable(oldTableOrName) - ? oldTableOrName - : await this.getCachedTable(oldTableOrName) - const newTable = oldTable.clone() - - newTable.name = newTableName - - // rename table - const up = new Query( - `ALTER TABLE ${this.escapePath( - oldTable.name, - )} RENAME TO ${this.escapePath(newTableName)}`, - ) - const down = new Query( - `ALTER TABLE ${this.escapePath( - newTableName, - )} RENAME TO ${this.escapePath(oldTable.name)}`, - ) - await this.executeQueries(up, down) - - // rename unique constraints - newTable.uniques.forEach((unique) => { - const oldUniqueName = - this.connection.namingStrategy.uniqueConstraintName( - oldTable, - unique.columnNames, - ) - - // Skip renaming if Unique has user defined constraint name - if (unique.name !== oldUniqueName) return - - unique.name = this.connection.namingStrategy.uniqueConstraintName( - newTable, - unique.columnNames, - ) - }) - - // rename foreign key constraints - newTable.foreignKeys.forEach((foreignKey) => { - const oldForeignKeyName = - this.connection.namingStrategy.foreignKeyName( - oldTable, - foreignKey.columnNames, - this.getTablePath(foreignKey), - foreignKey.referencedColumnNames, - ) - - // Skip renaming if foreign key has user defined constraint name - if (foreignKey.name !== oldForeignKeyName) return - - foreignKey.name = this.connection.namingStrategy.foreignKeyName( - newTable, - foreignKey.columnNames, - this.getTablePath(foreignKey), - foreignKey.referencedColumnNames, - ) - }) - - // rename indices - newTable.indices.forEach((index) => { - const oldIndexName = this.connection.namingStrategy.indexName( - oldTable, - index.columnNames, - index.where, - ) + const table = InstanceChecker.isTable(tableOrName) + ? tableOrName + : await this.getCachedTable(tableOrName) + const oldName = this.escapePath(table.name) + const newName = this.escapePath(newTableName) - // Skip renaming if Index has user defined constraint name - if (index.name !== oldIndexName) return + // Generate rename table queries + const up = new Query(`ALTER TABLE ${oldName} RENAME TO ${newName}`) + const down = new Query(`ALTER TABLE ${newName} RENAME TO ${oldName}`) - index.name = this.connection.namingStrategy.indexName( - newTable, - index.columnNames, - index.where, - ) - }) - - // rename old table; - oldTable.name = newTable.name + // Execute the rename + await this.executeQueries(up, down) - // recreate table with new constraint names - await this.recreateTable(newTable, oldTable) + // Clear the table cache and update the in-memory table name + this.clearCachedTable(table.name) + table.name = newTableName } /** @@ -539,7 +474,7 @@ export abstract class AbstractSqliteQueryRunner const table = InstanceChecker.isTable(tableOrName) ? tableOrName : await this.getCachedTable(tableOrName) - return this.addColumns(table!, [column]) + return this.addColumns(table, [column]) } /** @@ -552,9 +487,44 @@ export abstract class AbstractSqliteQueryRunner const table = InstanceChecker.isTable(tableOrName) ? tableOrName : await this.getCachedTable(tableOrName) - const changedTable = table.clone() - columns.forEach((column) => changedTable.addColumn(column)) - await this.recreateTable(changedTable, table) + + // Determine if we can use the native ADD COLUMN syntax or need to recreate the table + const shouldRecreateTable = columns.some( + (column) => + column.isPrimary || + (column.generatedType && column.asExpression), + ) + if (shouldRecreateTable) { + const changedTable = table.clone() + columns.forEach((column) => changedTable.addColumn(column)) + await this.recreateTable(changedTable, table) + return + } + + const tableName = this.escapePath(table.name) + const upQueries: Query[] = [] + const downQueries: Query[] = [] + for (const column of columns) { + upQueries.push( + new Query( + `ALTER TABLE ${tableName} ADD ${this.buildCreateColumnSql( + column, + )}`, + ), + ) + + downQueries.push( + new Query( + `ALTER TABLE ${tableName} DROP COLUMN "${column.name}"`, + ), + ) + + table.addColumn(column) + } + + if (upQueries.length > 0) { + await this.executeQueries(upQueries, downQueries) + } } /** @@ -576,7 +546,7 @@ export abstract class AbstractSqliteQueryRunner `Column "${oldTableColumnOrName}" was not found in the "${table.name}" table.`, ) - let newColumn: TableColumn | undefined = undefined + let newColumn: TableColumn if (InstanceChecker.isTableColumn(newTableColumnOrName)) { newColumn = newTableColumnOrName } else { @@ -584,7 +554,29 @@ export abstract class AbstractSqliteQueryRunner newColumn.name = newTableColumnOrName } - return this.changeColumn(table, oldColumn, newColumn) + if (oldColumn.name === newColumn.name) return + + const tableName = this.escapePath(table.name) + const upQueries: Query[] = [ + new Query( + `ALTER TABLE ${tableName} RENAME COLUMN "${oldColumn.name}" TO "${newColumn.name}"`, + ), + ] + const downQueries: Query[] = [ + new Query( + `ALTER TABLE ${tableName} RENAME COLUMN "${newColumn.name}" TO "${oldColumn.name}"`, + ), + ] + + await this.executeQueries(upQueries, downQueries) + + // Update column name in the in-memory table object + const tableColumn = table.columns.find( + (column) => column.name === oldColumn.name, + ) + if (tableColumn) { + tableColumn.name = newColumn.name + } } /** @@ -759,32 +751,100 @@ export abstract class AbstractSqliteQueryRunner ? tableOrName : await this.getCachedTable(tableOrName) - // clone original table and remove column and its constraints from cloned table - const changedTable = table.clone() - columns.forEach((column: TableColumn | string) => { - const columnInstance = InstanceChecker.isTableColumn(column) - ? column - : table.findColumnByName(column) - if (!columnInstance) + const tableColumns = columns.map((columnOrName) => { + const columnName = InstanceChecker.isTableColumn(columnOrName) + ? columnOrName.name + : columnOrName + const column = table.findColumnByName(columnName) + if (!column) { throw new Error( - `Column "${column}" was not found in table "${table.name}"`, + `Column "${columnName}" was not found in table "${table.name}"`, ) + } + return column + }) - changedTable.removeColumn(columnInstance) - changedTable - .findColumnUniques(columnInstance) - .forEach((unique) => - changedTable.removeUniqueConstraint(unique), - ) - changedTable - .findColumnIndices(columnInstance) - .forEach((index) => changedTable.removeIndex(index)) - changedTable - .findColumnForeignKeys(columnInstance) - .forEach((fk) => changedTable.removeForeignKey(fk)) + // Determine if we can use the native DROP COLUMN syntax or need to recreate the table + const shouldRecreateTable = tableColumns.some((column) => { + // If column is part of a primary key, we need to recreate the table + if (column.isPrimary) { + return true + } + + // If column is referenced by foreign keys, we need to recreate the table + const referencingForeignKeys = table.foreignKeys.filter( + (foreignKey) => foreignKey.columnNames.includes(column.name), + ) + if (referencingForeignKeys.length > 0) { + return true + } + + // If column is part of a unique constraint, we need to recreate the table + const uniqueConstraints = table.uniques.filter((unique) => + unique.columnNames.includes(column.name), + ) + if (uniqueConstraints.length > 0) { + return true + } + + // Check if it's a generated column + if (column.generatedType && column.asExpression) { + return true + } + + return false }) - await this.recreateTable(changedTable, table) + if (shouldRecreateTable) { + // Fall back to recreating the table if we need to handle constraints + const changedTable = table.clone() + tableColumns.forEach((column) => { + changedTable + .findColumnUniques(column) + .forEach((unique) => + changedTable.removeUniqueConstraint(unique), + ) + changedTable + .findColumnIndices(column) + .forEach((index) => changedTable.removeIndex(index)) + changedTable + .findColumnForeignKeys(column) + .forEach((fk) => changedTable.removeForeignKey(fk)) + changedTable.removeColumn(column) + }) + await this.recreateTable(changedTable, table) + } else { + // Use native DROP COLUMN if it's a simple column drop + const upQueries: Query[] = [] + const downQueries: Query[] = [] + const tableName = this.escapePath(table.name) + + for (const column of tableColumns) { + // Drop column + upQueries.push( + new Query( + `ALTER TABLE ${tableName} DROP COLUMN "${column.name}"`, + ), + ) + downQueries.push( + new Query( + `ALTER TABLE ${tableName} ADD ${this.buildCreateColumnSql( + column, + )}`, + ), + ) + + // Remove column from the in-memory table + const columnIndex = table.columns.findIndex( + (tableColumn) => tableColumn.name === column.name, + ) + if (columnIndex !== -1) { + table.columns.splice(columnIndex, 1) + } + } + + await this.executeQueries(upQueries, downQueries) + } } /** diff --git a/src/query-runner/BaseQueryRunner.ts b/src/query-runner/BaseQueryRunner.ts index 98588889382..759ecaf83c5 100644 --- a/src/query-runner/BaseQueryRunner.ts +++ b/src/query-runner/BaseQueryRunner.ts @@ -255,6 +255,10 @@ export abstract class BaseQueryRunner { } } + protected async clearCachedTable(tableName: string) { + delete this.cachedTablePaths[tableName] + } + /** * Gets table from previously loaded tables, otherwise loads it from database. */ diff --git a/test/functional/query-runner/rename-column.ts b/test/functional/query-runner/rename-column.ts index b510dc99d2a..320eccb8739 100644 --- a/test/functional/query-runner/rename-column.ts +++ b/test/functional/query-runner/rename-column.ts @@ -92,17 +92,12 @@ describe("query runner > rename column", () => { await queryRunner.renameColumn(table!, "text", "text2") table = await queryRunner.getTable("post") - const newUniqueConstraintName = - connection.namingStrategy.uniqueConstraintName(table!, [ - "text2", - "tag", - ]) tableUnique = table!.uniques.find((unique) => { return !!unique.columnNames.find( (columnName) => columnName === "tag", ) }) - tableUnique!.name!.should.be.equal(newUniqueConstraintName) + tableUnique!.name!.should.be.equal(oldUniqueConstraintName) } await queryRunner.executeMemoryDownSql() @@ -206,11 +201,6 @@ describe("query runner > rename column", () => { "name2", ) table = await queryRunner.getTable(questionTableName) - const newIndexName = connection.namingStrategy.indexName( - table!, - ["name2"], - ) - table!.indices[0].name!.should.be.equal(newIndexName) await queryRunner.renameColumn( categoryTableName, @@ -218,14 +208,6 @@ describe("query runner > rename column", () => { "questionId2", ) table = await queryRunner.getTable(categoryTableName) - const newForeignKeyName = - connection.namingStrategy.foreignKeyName( - table!, - ["questionId2"], - "question", - ["id"], - ) - table!.foreignKeys[0].name!.should.be.equal(newForeignKeyName) await queryRunner.executeMemoryDownSql() diff --git a/test/functional/query-runner/rename-table.ts b/test/functional/query-runner/rename-table.ts index f493db55336..ee980572147 100644 --- a/test/functional/query-runner/rename-table.ts +++ b/test/functional/query-runner/rename-table.ts @@ -120,24 +120,6 @@ describe("query runner > rename table", () => { // should successfully drop pk if pk constraint was correctly renamed. await queryRunner.dropPrimaryKey(table!) - // MySql does not support unique constraints - if ( - !DriverUtils.isMySQLFamily(connection.driver) && - !(connection.driver.options.type === "sap") - ) { - const newUniqueConstraintName = - connection.namingStrategy.uniqueConstraintName(table!, [ - "text", - "tag", - ]) - let tableUnique = table!.uniques.find((unique) => { - return !!unique.columnNames.find( - (columnName) => columnName === "tag", - ) - }) - tableUnique!.name!.should.be.equal(newUniqueConstraintName) - } - await queryRunner.executeMemoryDownSql() table = await queryRunner.getTable("post") @@ -257,25 +239,12 @@ describe("query runner > rename table", () => { "renamedQuestion", ) table = await queryRunner.getTable(renamedQuestionTableName) - const newIndexName = connection.namingStrategy.indexName( - table!, - ["name"], - ) - table!.indices[0].name!.should.be.equal(newIndexName) await queryRunner.renameTable( categoryTableName, "renamedCategory", ) table = await queryRunner.getTable(renamedCategoryTableName) - const newForeignKeyName = - connection.namingStrategy.foreignKeyName( - table!, - ["questionId"], - "question", - ["id"], - ) - table!.foreignKeys[0].name!.should.be.equal(newForeignKeyName) await queryRunner.executeMemoryDownSql()