From f6d0b0bd864d894535ae3c8d61e3c890643cdc96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 29 Apr 2025 13:29:55 +0200 Subject: [PATCH 1/4] use native queries for table renaming on sqlite --- .../AbstractSqliteQueryRunner.ts | 93 +++---------------- src/query-runner/BaseQueryRunner.ts | 4 + test/functional/query-runner/rename-table.ts | 31 ------- 3 files changed, 18 insertions(+), 110 deletions(-) diff --git a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts index 55a9e1a7f7..c02d31c1b6 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, - ) - - // Skip renaming if Index has user defined constraint name - if (index.name !== oldIndexName) return + const table = InstanceChecker.isTable(tableOrName) + ? tableOrName + : await this.getCachedTable(tableOrName) + const oldName = this.escapePath(table.name) + const newName = this.escapePath(newTableName) - index.name = this.connection.namingStrategy.indexName( - newTable, - index.columnNames, - index.where, - ) - }) + // Generate rename table queries + const up = new Query(`ALTER TABLE ${oldName} RENAME TO ${newName}`) + const down = new Query(`ALTER TABLE ${newName} RENAME TO ${oldName}`) - // 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 } /** diff --git a/src/query-runner/BaseQueryRunner.ts b/src/query-runner/BaseQueryRunner.ts index 9858888938..759ecaf83c 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-table.ts b/test/functional/query-runner/rename-table.ts index f493db5533..ee98057214 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() From 80523ed7661b451d2a6ff0e42954a9745d9333ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 29 Apr 2025 13:35:14 +0200 Subject: [PATCH 2/4] use native queries to add columns on sqlite --- .../AbstractSqliteQueryRunner.ts | 43 +++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts index c02d31c1b6..babe1113f6 100644 --- a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts +++ b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts @@ -474,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]) } /** @@ -487,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) + } } /** From 67419ac936af81060e9ffacd354d76d15815f596 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 29 Apr 2025 13:42:59 +0200 Subject: [PATCH 3/4] use native queries to rename columns on sqlite --- .../AbstractSqliteQueryRunner.ts | 26 +++++++++++++++++-- test/functional/query-runner/rename-column.ts | 20 +------------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts index babe1113f6..8c13bded9b 100644 --- a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts +++ b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts @@ -546,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 { @@ -554,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 + } } /** diff --git a/test/functional/query-runner/rename-column.ts b/test/functional/query-runner/rename-column.ts index b510dc99d2..320eccb873 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() From 3043e57666b7f15aec31165715b7bee12f5df9d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 29 Apr 2025 13:50:31 +0200 Subject: [PATCH 4/4] use native queries to drop columns on sqlite --- .../AbstractSqliteQueryRunner.ts | 110 ++++++++++++++---- 1 file changed, 89 insertions(+), 21 deletions(-) diff --git a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts index 8c13bded9b..7607f25ce6 100644 --- a/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts +++ b/src/driver/sqlite-abstract/AbstractSqliteQueryRunner.ts @@ -751,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) + } } /**