feat: allow per-migration control over transaction behavior (#9459)

Closes #7087

docs/migrations.md

@@ -1,11 +1,15 @@
 # Migrations
 
--   [How migrations work](#how-migrations-work)
--   [Creating a new migration](#creating-a-new-migration)
--   [Running and reverting migrations](#running-and-reverting-migrations)
--   [Generating migrations](#generating-migrations)
--   [Connection option](#connection-option)
--   [Using migration API to write migrations](#using-migration-api-to-write-migrations)
+- [Migrations](#migrations)
+  - [How migrations work](#how-migrations-work)
+  - [Creating a new migration](#creating-a-new-migration)
+  - [Running and reverting migrations](#running-and-reverting-migrations)
+    - [Faking Migrations and Rollbacks](#faking-migrations-and-rollbacks)
+    - [Transaction modes](#transaction-modes)
+  - [Generating migrations](#generating-migrations)
+  - [DataSoure option](#datasoure-option)
+  - [Timestamp option](#timestamp-option)
+  - [Using migration API to write migrations](#using-migration-api-to-write-migrations)
 
 ## How migrations work
 
@@ -184,6 +188,34 @@ This is also possible with rollbacks.
 typeorm migration:revert --fake
 ```
 
+### Transaction modes
+
+By default, TypeORM will run all your migrations within a single wrapping transaction.
+This corresponds to the `--transaction all` flag.
+If you require more fine grained transaction control, you can use the `--transaction each` flag to wrap every migration individually, or the `--transaction none` flag to opt out of wrapping the migrations in transactions altogether.
+
+In addition to these flags, you can also override the transaction behavior on a per-migration basis by setting the `transaction` property on the `MigrationInterface` to `true` or `false`. This only works in the `each` or `none` transaction mode.
+
+```typescript
+import { MigrationInterface, QueryRunner } from "typeorm"
+
+export class AddIndexTIMESTAMP implements MigrationInterface {
+    transaction = false
+
+    async up(queryRunner: QueryRunner): Promise<void> {
+        await queryRunner.query(
+            `CREATE INDEX CONCURRENTLY post_names_idx ON post(name)`
+        )
+    }
+
+    async down(queryRunner: QueryRunner): Promise<void> {
+        await queryRunner.query(
+            `DROP INDEX CONCURRENTLY post_names_idx`,
+        )
+    }
+}
+```
+
 ## Generating migrations
 
 TypeORM is able to automatically generate migration files with schema changes you made.

src/error/ForbiddenTransactionModeOverrideError.ts

@@ -0,0 +1,19 @@
+import { Migration } from "../migration/Migration"
+import { TypeORMError } from "./TypeORMError"
+
+/**
+ * Thrown when the per-migration transaction mode is overriden but the global transaction mode is set to "all".
+ */
+export class ForbiddenTransactionModeOverrideError extends TypeORMError {
+    constructor(migrationsOverridingTransactionMode: Migration[]) {
+        const migrationNames = migrationsOverridingTransactionMode.map(
+            (migration) => `"${migration.name}"`,
+        )
+
+        super(
+            `Migrations ${migrationNames.join(
+                ", ",
+            )} override the transaction mode, but the global transaction mode is "all"`,
+        )
+    }
+}

src/error/index.ts

@@ -59,3 +59,4 @@ export * from "./OffsetWithoutLimitNotSupportedError"
 export * from "./CannotExecuteNotConnectedError"
 export * from "./NoConnectionOptionError"
 export * from "./TypeORMError"
+export * from "./ForbiddenTransactionModeOverrideError"

src/migration/Migration.ts

@@ -29,6 +29,11 @@ export class Migration {
      */
     instance?: MigrationInterface
 
+    /**
+     * Whether to run this migration within a transaction
+     */
+    transaction?: boolean
+
     // -------------------------------------------------------------------------
     // Constructor
     // -------------------------------------------------------------------------
@@ -38,10 +43,12 @@ export class Migration {
         timestamp: number,
         name: string,
         instance?: MigrationInterface,
+        transaction?: boolean,
     ) {
         this.id = id
         this.timestamp = timestamp
         this.name = name
         this.instance = instance
+        this.transaction = transaction
     }
 }

src/migration/MigrationExecutor.ts

@@ -5,7 +5,7 @@ import { ObjectLiteral } from "../common/ObjectLiteral"
 import { QueryRunner } from "../query-runner/QueryRunner"
 import { MssqlParameter } from "../driver/sqlserver/MssqlParameter"
 import { MongoQueryRunner } from "../driver/mongodb/MongoQueryRunner"
-import { TypeORMError } from "../error"
+import { ForbiddenTransactionModeOverrideError, TypeORMError } from "../error"
 import { InstanceChecker } from "../util/InstanceChecker"
 
 /**
@@ -259,6 +259,55 @@ export class MigrationExecutor {
             `${pendingMigrations.length} migrations are new migrations must be executed.`,
         )
 
+        if (this.transaction === "all") {
+            // If we desire to run all migrations in a single transaction
+            // but there is a migration that explicitly overrides the transaction mode
+            // then we have to fail since we cannot properly resolve that intent
+            // In theory we could support overrides that are set to `true`,
+            // however to keep the interface more rigid, we fail those too
+            const migrationsOverridingTransactionMode =
+                pendingMigrations.filter(
+                    (migration) =>
+                        !(migration.instance?.transaction === undefined),
+                )
+
+            if (migrationsOverridingTransactionMode.length > 0) {
+                const error = new ForbiddenTransactionModeOverrideError(
+                    migrationsOverridingTransactionMode,
+                )
+                this.connection.logger.logMigration(
+                    `Migrations failed, error: ${error.message}`,
+                )
+                throw error
+            }
+        }
+
+        // Set the per-migration defaults for the transaction mode
+        // so that we have one centralized place that controls this behavior
+
+        // When transaction mode is `each` the default is to run in a transaction
+        // When transaction mode is `none` the default is to not run in a transaction
+        // When transaction mode is `all` the default is to not run in a transaction
+        // since all the migrations are already running in one single transaction
+
+        const txModeDefault = {
+            each: true,
+            none: false,
+            all: false,
+        }[this.transaction]
+
+        for (const migration of pendingMigrations) {
+            if (migration.instance) {
+                const instanceTx = migration.instance.transaction
+
+                if (instanceTx === undefined) {
+                    migration.transaction = txModeDefault
+                } else {
+                    migration.transaction = instanceTx
+                }
+            }
+        }
+
         // start transaction if its not started yet
         let transactionStartedByUs = false
         if (this.transaction === "all" && !queryRunner.isTransactionActive) {
@@ -277,10 +326,7 @@ export class MigrationExecutor {
                     continue
                 }
 
-                if (
-                    this.transaction === "each" &&
-                    !queryRunner.isTransactionActive
-                ) {
+                if (migration.transaction && !queryRunner.isTransactionActive) {
                     await queryRunner.startTransaction()
                     transactionStartedByUs = true
                 }
@@ -301,10 +347,7 @@ export class MigrationExecutor {
                             migration,
                         )
                         // commit transaction if we started it
-                        if (
-                            this.transaction === "each" &&
-                            transactionStartedByUs
-                        )
+                        if (migration.transaction && transactionStartedByUs)
                             await queryRunner.commitTransaction()
                     })
                     .then(() => {

src/migration/MigrationInterface.ts

@@ -9,6 +9,14 @@ export interface MigrationInterface {
      */
     name?: string
 
+    /**
+     * Optional flag to determine whether to run the migration in a transaction or not.
+     * Can only be used when `migrationsTransactionMode` is either "each" or "none"
+     * Defaults to `true` when `migrationsTransactionMode` is "each"
+     * Defaults to `false` when `migrationsTransactionMode` is "none"
+     */
+    transaction?: boolean
+
     /**
      * Run the migrations.
      */

test/github-issues/7087/entity/user.ts

@@ -0,0 +1,8 @@
+import { PrimaryGeneratedColumn } from "../../../../src/decorator/columns/PrimaryGeneratedColumn"
+import { Entity } from "../../../../src/decorator/entity/Entity"
+
+@Entity({ name: "users", synchronize: false })
+export class User {
+    @PrimaryGeneratedColumn("uuid")
+    id: number
+}

test/github-issues/7087/issue-7087.ts

@@ -0,0 +1,70 @@
+import "reflect-metadata"
+import {
+    createTestingConnections,
+    closeTestingConnections,
+    reloadTestingDatabases,
+} from "../../utils/test-utils"
+import { DataSource } from "../../../src/data-source/DataSource"
+import { Migration } from "../../../src/migration/Migration"
+import { ForbiddenTransactionModeOverrideError } from "../../../src/error/ForbiddenTransactionModeOverrideError"
+
+describe("github issues > #7087 Allow to specify transaction property for individual migrations", () => {
+    let connections: DataSource[]
+    before(
+        async () =>
+            (connections = await createTestingConnections({
+                __dirname,
+                schemaCreate: false,
+                dropSchema: true,
+                enabledDrivers: ["postgres"],
+            })),
+    )
+    beforeEach(() => reloadTestingDatabases(connections))
+    after(() => closeTestingConnections(connections))
+
+    it("should fail to run all necessary migrations when transaction is all and there are transaction overrides", () =>
+        Promise.all(
+            connections.map(async (connection) => {
+                return connection
+                    .runMigrations({ transaction: "all" })
+                    .should.be.rejectedWith(
+                        ForbiddenTransactionModeOverrideError,
+                        'Migrations "InsertUser0000000000002", "CreateIndex0000000000003" override the transaction mode, but the global transaction mode is "all"',
+                    )
+            }),
+        ))
+
+    it("should set correct transaction mode when transaction is each", () =>
+        Promise.all(
+            connections.map(async (connection) => {
+                const migrations: Migration[] = await connection.runMigrations({
+                    transaction: "each",
+                })
+
+                migrations.length.should.be.equal(3)
+                migrations[0].name.should.be.equal("CreateUsers0000000000001")
+                migrations[0].transaction!.should.be.true
+                migrations[1].name.should.be.equal("InsertUser0000000000002")
+                migrations[1].transaction!.should.be.true
+                migrations[2].name.should.be.equal("CreateIndex0000000000003")
+                migrations[2].transaction!.should.be.false
+            }),
+        ))
+
+    it("should set correct transaction mode when transaction is none", () =>
+        Promise.all(
+            connections.map(async (connection) => {
+                const migrations: Migration[] = await connection.runMigrations({
+                    transaction: "none",
+                })
+
+                migrations.length.should.be.equal(3)
+                migrations[0].name.should.be.equal("CreateUsers0000000000001")
+                migrations[0].transaction!.should.be.false
+                migrations[1].name.should.be.equal("InsertUser0000000000002")
+                migrations[1].transaction!.should.be.true
+                migrations[2].name.should.be.equal("CreateIndex0000000000003")
+                migrations[2].transaction!.should.be.false
+            }),
+        ))
+})

test/github-issues/7087/migration/0000000000001-CreateUsers.ts

@@ -0,0 +1,25 @@
+import { MigrationInterface } from "../../../../src/migration/MigrationInterface"
+import { QueryRunner } from "../../../../src/query-runner/QueryRunner"
+import { Table } from "../../../../src/schema-builder/table/Table"
+
+export class CreateUsers0000000000001 implements MigrationInterface {
+    public up(queryRunner: QueryRunner): Promise<any> {
+        return queryRunner.createTable(
+            new Table({
+                name: "users",
+                columns: [
+                    {
+                        name: "id",
+                        type: "uuid",
+                        isPrimary: true,
+                        default: "uuid_generate_v4()",
+                    },
+                ],
+            }),
+        )
+    }
+
+    public down(queryRunner: QueryRunner): Promise<any> {
+        return queryRunner.dropTable("users")
+    }
+}

test/github-issues/7087/migration/0000000000002-InsertUser.ts

@@ -0,0 +1,16 @@
+import { MigrationInterface } from "../../../../src/migration/MigrationInterface"
+import { QueryRunner } from "../../../../src/query-runner/QueryRunner"
+import { User } from "../entity/user"
+
+export class InsertUser0000000000002 implements MigrationInterface {
+    public transaction = true
+
+    public up(queryRunner: QueryRunner): Promise<any> {
+        const userRepo = queryRunner.connection.getRepository<User>(User)
+        return userRepo.save(new User())
+    }
+
+    public down(queryRunner: QueryRunner): Promise<any> {
+        return Promise.resolve()
+    }
+}

test/github-issues/7087/migration/0000000000003-CreateIndex.ts

@@ -0,0 +1,16 @@
+import { MigrationInterface } from "../../../../src/migration/MigrationInterface"
+import { QueryRunner } from "../../../../src/query-runner/QueryRunner"
+
+export class CreateIndex0000000000003 implements MigrationInterface {
+    public transaction = false
+
+    public up(queryRunner: QueryRunner): Promise<any> {
+        return queryRunner.query(
+            "CREATE INDEX CONCURRENTLY user_ids_idx ON users(id)",
+        )
+    }
+
+    public down(queryRunner: QueryRunner): Promise<any> {
+        return Promise.resolve()
+    }
+}