diff --git a/src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts b/src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts index 810e6735a..63787b313 100644 --- a/src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts +++ b/src/driver/aurora-mysql/AuroraMysqlQueryRunner.ts @@ -1414,7 +1414,7 @@ export class AuroraMysqlQueryRunner tableOrName: Table | string, columns: TableColumn[] | string[], ): Promise { - for (const column of columns) { + for (const column of [...columns]) { await this.dropColumn(tableOrName, column) } } diff --git a/src/driver/cockroachdb/CockroachQueryRunner.ts b/src/driver/cockroachdb/CockroachQueryRunner.ts index c23f01837..a1c26d8d8 100644 --- a/src/driver/cockroachdb/CockroachQueryRunner.ts +++ b/src/driver/cockroachdb/CockroachQueryRunner.ts @@ -2311,7 +2311,7 @@ export class CockroachQueryRunner tableOrName: Table | string, columns: TableColumn[] | string[], ): Promise { - for (const column of columns) { + for (const column of [...columns]) { await this.dropColumn(tableOrName, column) } } @@ -2515,7 +2515,7 @@ export class CockroachQueryRunner tableOrName: Table | string, uniqueConstraints: TableUnique[], ): Promise { - for (const uniqueConstraint of uniqueConstraints) { + for (const uniqueConstraint of [...uniqueConstraints]) { await this.dropUniqueConstraint(tableOrName, uniqueConstraint) } } @@ -2712,7 +2712,7 @@ export class CockroachQueryRunner tableOrName: Table | string, foreignKeys: TableForeignKey[], ): Promise { - for (const foreignKey of foreignKeys) { + for (const foreignKey of [...foreignKeys]) { await this.dropForeignKey(tableOrName, foreignKey) } } @@ -2797,7 +2797,7 @@ export class CockroachQueryRunner tableOrName: Table | string, indices: TableIndex[], ): Promise { - for (const index of indices) { + for (const index of [...indices]) { await this.dropIndex(tableOrName, index) } } diff --git a/src/driver/mysql/MysqlQueryRunner.ts b/src/driver/mysql/MysqlQueryRunner.ts index 1b977f59b..643dd107a 100644 --- a/src/driver/mysql/MysqlQueryRunner.ts +++ b/src/driver/mysql/MysqlQueryRunner.ts @@ -1799,7 +1799,7 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner { tableOrName: Table | string, columns: TableColumn[] | string[], ): Promise { - for (const column of columns) { + for (const column of [...columns]) { await this.dropColumn(tableOrName, column) } } diff --git a/src/driver/oracle/OracleQueryRunner.ts b/src/driver/oracle/OracleQueryRunner.ts index ec58e155f..62af61291 100644 --- a/src/driver/oracle/OracleQueryRunner.ts +++ b/src/driver/oracle/OracleQueryRunner.ts @@ -1730,7 +1730,7 @@ export class OracleQueryRunner extends BaseQueryRunner implements QueryRunner { tableOrName: Table | string, columns: TableColumn[] | string[], ): Promise { - for (const column of columns) { + for (const column of [...columns]) { await this.dropColumn(tableOrName, column) } } diff --git a/src/driver/postgres/PostgresQueryRunner.ts b/src/driver/postgres/PostgresQueryRunner.ts index 3e0173da0..cdb6d2b0a 100644 --- a/src/driver/postgres/PostgresQueryRunner.ts +++ b/src/driver/postgres/PostgresQueryRunner.ts @@ -2527,7 +2527,7 @@ export class PostgresQueryRunner tableOrName: Table | string, columns: TableColumn[] | string[], ): Promise { - for (const column of columns) { + for (const column of [...columns]) { await this.dropColumn(tableOrName, column) } } @@ -2728,7 +2728,7 @@ export class PostgresQueryRunner tableOrName: Table | string, uniqueConstraints: TableUnique[], ): Promise { - for (const uniqueConstraint of uniqueConstraints) { + for (const uniqueConstraint of [...uniqueConstraints]) { await this.dropUniqueConstraint(tableOrName, uniqueConstraint) } } @@ -2966,7 +2966,7 @@ export class PostgresQueryRunner tableOrName: Table | string, foreignKeys: TableForeignKey[], ): Promise { - for (const foreignKey of foreignKeys) { + for (const foreignKey of [...foreignKeys]) { await this.dropForeignKey(tableOrName, foreignKey) } } @@ -3094,7 +3094,7 @@ export class PostgresQueryRunner tableOrName: Table | string, indices: TableIndex[], ): Promise { - for (const index of indices) { + for (const index of [...indices]) { await this.dropIndex(tableOrName, index) } } diff --git a/src/driver/sap/SapQueryRunner.ts b/src/driver/sap/SapQueryRunner.ts index 420b88f77..9920ffe76 100644 --- a/src/driver/sap/SapQueryRunner.ts +++ b/src/driver/sap/SapQueryRunner.ts @@ -1828,7 +1828,7 @@ export class SapQueryRunner extends BaseQueryRunner implements QueryRunner { tableOrName: Table | string, columns: TableColumn[] | string[], ): Promise { - for (const column of columns) { + for (const column of [...columns]) { await this.dropColumn(tableOrName, column) } } diff --git a/src/driver/spanner/SpannerQueryRunner.ts b/src/driver/spanner/SpannerQueryRunner.ts index e2b560e88..4c1a73b70 100644 --- a/src/driver/spanner/SpannerQueryRunner.ts +++ b/src/driver/spanner/SpannerQueryRunner.ts @@ -1052,7 +1052,7 @@ export class SpannerQueryRunner extends BaseQueryRunner implements QueryRunner { tableOrName: Table | string, columns: TableColumn[] | string[], ): Promise { - for (const column of columns) { + for (const column of [...columns]) { await this.dropColumn(tableOrName, column) } } @@ -1342,7 +1342,7 @@ export class SpannerQueryRunner extends BaseQueryRunner implements QueryRunner { tableOrName: Table | string, foreignKeys: TableForeignKey[], ): Promise { - for (const foreignKey of foreignKeys) { + for (const foreignKey of [...foreignKeys]) { await this.dropForeignKey(tableOrName, foreignKey) } } @@ -1416,7 +1416,7 @@ export class SpannerQueryRunner extends BaseQueryRunner implements QueryRunner { tableOrName: Table | string, indices: TableIndex[], ): Promise { - for (const index of indices) { + for (const index of [...indices]) { await this.dropIndex(tableOrName, index) } } diff --git a/src/driver/sqlserver/SqlServerQueryRunner.ts b/src/driver/sqlserver/SqlServerQueryRunner.ts index b21efe984..7a7dc4785 100644 --- a/src/driver/sqlserver/SqlServerQueryRunner.ts +++ b/src/driver/sqlserver/SqlServerQueryRunner.ts @@ -2115,7 +2115,7 @@ export class SqlServerQueryRunner tableOrName: Table | string, columns: TableColumn[] | string[], ): Promise { - for (const column of columns) { + for (const column of [...columns]) { await this.dropColumn(tableOrName, column) } } diff --git a/test/functional/query-runner/drop-column.ts b/test/functional/query-runner/drop-column.ts index 93b17dd15..a0868f978 100644 --- a/test/functional/query-runner/drop-column.ts +++ b/test/functional/query-runner/drop-column.ts @@ -4,7 +4,8 @@ import { closeTestingConnections, createTestingConnections, } from "../../utils/test-utils" -import { DataSource } from "../../../src" +import { DataSource, Table } from "../../../src" +import { DriverUtils } from "../../../src/driver/DriverUtils" describe("query runner > drop column", () => { let connections: DataSource[] @@ -146,4 +147,96 @@ describe("query runner > drop column", () => { }), )) }) + + describe("array modification during iteration", () => { + it("should drop all columns without skipping any when iterating over array", () => + Promise.all( + connections.map(async (connection) => { + // Skip drivers that don't support dropping multiple columns + if ( + connection.driver.options.type === "mongodb" || + connection.driver.options.type === "better-sqlite3" || + connection.driver.options.type === "capacitor" || + connection.driver.options.type === "cordova" || + connection.driver.options.type === "react-native" || + connection.driver.options.type === "nativescript" || + connection.driver.options.type === "expo" || + connection.driver.options.type === "sqljs" + ) { + return + } + + const queryRunner = connection.createQueryRunner() + + // Create test table with multiple columns + const table = new Table({ + name: "test_drop_columns_array", + columns: [ + { + name: "id", + type: DriverUtils.isSQLiteFamily( + connection.driver, + ) + ? "integer" + : "int", + isPrimary: true, + isGenerated: + connection.driver.options.type !== + "spanner", + generationStrategy: + connection.driver.options.type !== "spanner" + ? "increment" + : undefined, + }, + { + name: "col1", + type: "varchar", + length: "255", + }, + { + name: "col2", + type: "varchar", + length: "255", + }, + { + name: "col3", + type: "varchar", + length: "255", + }, + { + name: "col4", + type: "varchar", + length: "255", + }, + ], + }) + + await queryRunner.createTable(table) + + // Get the table to ensure it was created correctly + const createdTable = await queryRunner.getTable( + "test_drop_columns_array", + ) + expect(createdTable!.columns).to.have.length(5) // id + 4 test columns + + // Drop multiple columns at once - this tests the array modification bug fix + const columnsToRemove = ["col1", "col2", "col3", "col4"] + await queryRunner.dropColumns( + createdTable!, + columnsToRemove, + ) + + // Verify all columns were dropped (only 'id' should remain) + const updatedTable = await queryRunner.getTable( + "test_drop_columns_array", + ) + expect(updatedTable!.columns).to.have.length(1) + expect(updatedTable!.columns[0].name).to.equal("id") + + // Clean up + await queryRunner.dropTable("test_drop_columns_array") + await queryRunner.release() + }), + )) + }) }) diff --git a/test/functional/query-runner/drop-foreign-key.ts b/test/functional/query-runner/drop-foreign-key.ts index 2f7363a82..240121c9a 100644 --- a/test/functional/query-runner/drop-foreign-key.ts +++ b/test/functional/query-runner/drop-foreign-key.ts @@ -1,10 +1,12 @@ import "reflect-metadata" -import { DataSource } from "../../../src/data-source/DataSource" +import { expect } from "chai" +import { DataSource, Table, TableColumn, TableForeignKey } from "../../../src" import { closeTestingConnections, createTestingConnections, reloadTestingDatabases, } from "../../utils/test-utils" +import { DriverUtils } from "../../../src/driver/DriverUtils" describe("query runner > drop foreign key", () => { let connections: DataSource[] @@ -39,4 +41,164 @@ describe("query runner > drop foreign key", () => { await queryRunner.release() }), )) + + it("should drop all foreign keys without skipping any when iterating over array", () => + Promise.all( + connections.map(async (connection) => { + // Skip databases that don't support foreign keys + if (connection.driver.options.type === "spanner") { + return + } + + const queryRunner = connection.createQueryRunner() + await queryRunner.connect() + + try { + // Create a referenced table first + await queryRunner.createTable( + new Table({ + name: "referenced_table", + columns: [ + new TableColumn({ + name: "id", + type: DriverUtils.isSQLiteFamily( + connection.driver, + ) + ? "integer" + : "int", + isPrimary: true, + isGenerated: true, + generationStrategy: "increment", + }), + new TableColumn({ + name: "name", + type: "varchar", + length: "100", + }), + ], + }), + true, + ) + + // Create a test table for foreign keys + await queryRunner.createTable( + new Table({ + name: "test_fk_table", + columns: [ + new TableColumn({ + name: "id", + type: DriverUtils.isSQLiteFamily( + connection.driver, + ) + ? "integer" + : "int", + isPrimary: true, + isGenerated: true, + generationStrategy: "increment", + }), + new TableColumn({ + name: "ref_id_1", + type: "int", + isNullable: true, + }), + new TableColumn({ + name: "ref_id_2", + type: "int", + isNullable: true, + }), + new TableColumn({ + name: "ref_id_3", + type: "int", + isNullable: true, + }), + new TableColumn({ + name: "ref_id_4", + type: "int", + isNullable: true, + }), + ], + }), + true, + ) + + // Add multiple foreign keys + await queryRunner.createForeignKeys("test_fk_table", [ + new TableForeignKey({ + name: "test_fk_1", + columnNames: ["ref_id_1"], + referencedTableName: "referenced_table", + referencedColumnNames: ["id"], + }), + new TableForeignKey({ + name: "test_fk_2", + columnNames: ["ref_id_2"], + referencedTableName: "referenced_table", + referencedColumnNames: ["id"], + }), + new TableForeignKey({ + name: "test_fk_3", + columnNames: ["ref_id_3"], + referencedTableName: "referenced_table", + referencedColumnNames: ["id"], + }), + new TableForeignKey({ + name: "test_fk_4", + columnNames: ["ref_id_4"], + referencedTableName: "referenced_table", + referencedColumnNames: ["id"], + }), + ]) + + // Get the table with foreign keys + const table = await queryRunner.getTable("test_fk_table") + if (!table) { + throw new Error("Test table not found") + } + + // Find only our test foreign keys + const testForeignKeys = table.foreignKeys.filter((fk) => + fk.name?.startsWith("test_fk_"), + ) + + expect(testForeignKeys).to.have.length( + 4, + `Should have 4 test foreign keys before dropping, found: ${testForeignKeys + .map((fk) => fk.name) + .join(", ")}`, + ) + + // Drop all test foreign keys - this should not skip any due to array modification + await queryRunner.dropForeignKeys( + "test_fk_table", + testForeignKeys, + ) + + // Verify all test foreign keys were dropped + const finalTable = await queryRunner.getTable( + "test_fk_table", + ) + if (!finalTable) { + throw new Error("Final test table not found") + } + + const remainingTestForeignKeys = + finalTable.foreignKeys.filter((fk) => + fk.name?.startsWith("test_fk_"), + ) + + expect(remainingTestForeignKeys).to.have.length( + 0, + `All test foreign keys should be dropped, but found remaining: ${remainingTestForeignKeys + .map((fk) => fk.name) + .join(", ")}`, + ) + + // Clean up test tables + await queryRunner.dropTable("test_fk_table") + await queryRunner.dropTable("referenced_table") + } finally { + await queryRunner.release() + } + }), + )) }) diff --git a/test/functional/query-runner/drop-index.ts b/test/functional/query-runner/drop-index.ts index e81aa5925..c8027de60 100644 --- a/test/functional/query-runner/drop-index.ts +++ b/test/functional/query-runner/drop-index.ts @@ -1,10 +1,12 @@ import "reflect-metadata" -import { DataSource } from "../../../src/data-source/DataSource" +import { expect } from "chai" +import { DataSource, Table, TableColumn, TableIndex } from "../../../src" import { closeTestingConnections, createTestingConnections, reloadTestingDatabases, } from "../../utils/test-utils" +import { DriverUtils } from "../../../src/driver/DriverUtils" describe("query runner > drop index", () => { let connections: DataSource[] @@ -54,4 +56,127 @@ describe("query runner > drop index", () => { await queryRunner.release() }), )) + + it("should drop all indices without skipping any when iterating over array", () => + Promise.all( + connections.map(async (connection) => { + const queryRunner = connection.createQueryRunner() + await queryRunner.connect() + + try { + // Create a test table for this specific test + await queryRunner.createTable( + new Table({ + name: "test_index_table", + columns: [ + new TableColumn({ + name: "id", + type: DriverUtils.isSQLiteFamily( + connection.driver, + ) + ? "integer" + : "int", + isPrimary: true, + isGenerated: true, + generationStrategy: "increment", + }), + new TableColumn({ + name: "idx_col_1", + type: "varchar", + length: "100", + isNullable: true, + }), + new TableColumn({ + name: "idx_col_2", + type: "varchar", + length: "100", + isNullable: true, + }), + new TableColumn({ + name: "idx_col_3", + type: "varchar", + length: "100", + isNullable: true, + }), + new TableColumn({ + name: "idx_col_4", + type: "varchar", + length: "100", + isNullable: true, + }), + ], + }), + true, + ) + + // Add multiple test indices on different columns to avoid Oracle conflicts + await queryRunner.createIndices("test_index_table", [ + new TableIndex({ + name: "test_idx_1", + columnNames: ["idx_col_1"], + }), + new TableIndex({ + name: "test_idx_2", + columnNames: ["idx_col_2"], + }), + new TableIndex({ + name: "test_idx_3", + columnNames: ["idx_col_3"], + }), + new TableIndex({ + name: "test_idx_4", + columnNames: ["idx_col_4"], + }), + ]) + + // Get the table with indices + const table = await queryRunner.getTable("test_index_table") + if (!table) { + throw new Error("Test table not found") + } + + // Find only our test indices + const testIndices = table.indices.filter((idx) => + idx.name?.startsWith("test_idx_"), + ) + + expect(testIndices).to.have.length( + 4, + `Should have 4 test indices before dropping, found: ${testIndices + .map((i) => i.name) + .join(", ")}`, + ) + + // Drop all test indices - this should not skip any due to array modification + await queryRunner.dropIndices( + "test_index_table", + testIndices, + ) + + // Verify all test indices were dropped + const finalTable = await queryRunner.getTable( + "test_index_table", + ) + if (!finalTable) { + throw new Error("Final test table not found") + } + + const remainingTestIndices = finalTable.indices.filter( + (idx) => idx.name?.startsWith("test_idx_"), + ) + + expect(remainingTestIndices).to.have.length( + 0, + `All test indices should be dropped, but found remaining: ${remainingTestIndices + .map((i) => i.name) + .join(", ")}`, + ) + + // Clean up the test table + await queryRunner.dropTable("test_index_table") + } finally { + await queryRunner.release() + } + }), + )) }) diff --git a/test/functional/query-runner/drop-unique-constraint.ts b/test/functional/query-runner/drop-unique-constraint.ts index bc36246f7..4f4788e33 100644 --- a/test/functional/query-runner/drop-unique-constraint.ts +++ b/test/functional/query-runner/drop-unique-constraint.ts @@ -1,10 +1,12 @@ import "reflect-metadata" -import { DataSource } from "../../../src/data-source/DataSource" +import { expect } from "chai" +import { DataSource, Table, TableColumn, TableUnique } from "../../../src" import { closeTestingConnections, createTestingConnections, reloadTestingDatabases, } from "../../utils/test-utils" +import { DriverUtils } from "../../../src/driver/DriverUtils" describe("query runner > drop unique constraint", () => { let connections: DataSource[] @@ -51,4 +53,133 @@ describe("query runner > drop unique constraint", () => { await queryRunner.release() }), )) + + it("should drop all unique constraints without skipping any when iterating over array", () => + Promise.all( + connections.map(async (connection) => { + const queryRunner = connection.createQueryRunner() + await queryRunner.connect() + + try { + // Create a test table for unique constraints + await queryRunner.createTable( + new Table({ + name: "test_unique_table", + columns: [ + new TableColumn({ + name: "id", + type: DriverUtils.isSQLiteFamily( + connection.driver, + ) + ? "integer" + : "int", + isPrimary: true, + isGenerated: true, + generationStrategy: "increment", + }), + new TableColumn({ + name: "unique_col_1", + type: "varchar", + length: "100", + isNullable: true, + }), + new TableColumn({ + name: "unique_col_2", + type: "varchar", + length: "100", + isNullable: true, + }), + new TableColumn({ + name: "unique_col_3", + type: "varchar", + length: "100", + isNullable: true, + }), + new TableColumn({ + name: "unique_col_4", + type: "varchar", + length: "100", + isNullable: true, + }), + ], + }), + true, + ) + + // Add multiple unique constraints + await queryRunner.createUniqueConstraints( + "test_unique_table", + [ + new TableUnique({ + name: "test_uq_1", + columnNames: ["unique_col_1"], + }), + new TableUnique({ + name: "test_uq_2", + columnNames: ["unique_col_2"], + }), + new TableUnique({ + name: "test_uq_3", + columnNames: ["unique_col_3"], + }), + new TableUnique({ + name: "test_uq_4", + columnNames: ["unique_col_4"], + }), + ], + ) + + // Get the table with unique constraints + const table = await queryRunner.getTable( + "test_unique_table", + ) + if (!table) { + throw new Error("Test table not found") + } + + // Find only our test unique constraints + const testUniqueConstraints = table.uniques.filter((uq) => + uq.name?.startsWith("test_uq_"), + ) + + expect(testUniqueConstraints).to.have.length( + 4, + `Should have 4 test unique constraints before dropping, found: ${testUniqueConstraints + .map((uq) => uq.name) + .join(", ")}`, + ) + + // Drop all test unique constraints - this should not skip any due to array modification + await queryRunner.dropUniqueConstraints( + "test_unique_table", + testUniqueConstraints, + ) + + // Verify all test unique constraints were dropped + const finalTable = await queryRunner.getTable( + "test_unique_table", + ) + if (!finalTable) { + throw new Error("Final test table not found") + } + + const remainingTestUniqueConstraints = + finalTable.uniques.filter((uq) => + uq.name?.startsWith("test_uq_"), + ) + + expect(remainingTestUniqueConstraints).to.have.length( + 0, + `All test unique constraints should be dropped, but found remaining: ${remainingTestUniqueConstraints + .map((uq) => uq.name) + .join(", ")}`, + ) + + // Clean up the test table + await queryRunner.dropTable("test_unique_table") + } finally { + await queryRunner.release() + } + }), + )) })