feat(Migrator): allow passing transactions to Migrator. (#1480)

Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
Co-authored-by: João Lucas de Oliveira Lopes <55464917+jlucaso1@users.noreply.github.com>

Author: jlucaso1

src/migration/migrator.ts

@@ -546,14 +546,30 @@ export class Migrator {
       }
     }
 
-    const disableTransaction =
+    const disableTransactions =
       options?.disableTransactions ?? this.#props.disableTransactions
 
-    if (!adapter.supportsTransactionalDdl || disableTransaction) {
-      return this.#props.db.connection().execute(run)
+    if (this.#props.db.isTransaction) {
+      if (!adapter.supportsTransactionalDdl) {
+        throw new Error(
+          'Transactional DDL is not supported in this dialect. Passing a transaction to this migrator would result in failure or unexpected behavior.',
+        )
+      }
+
+      if (disableTransactions) {
+        throw new Error(
+          '`disableTransactions` is true but the migrator was given a transaction. Passing a transaction to this migrator would result in failure or unexpected behavior.',
+        )
+      }
+
+      return run(this.#props.db)
+    }
+
+    if (adapter.supportsTransactionalDdl && !disableTransactions) {
+      return this.#props.db.transaction().execute(run)
     }
 
-    return this.#props.db.transaction().execute(run)
+    return this.#props.db.connection().execute(run)
   }
 
   async #getState(db: Kysely<any>): Promise<MigrationState> {

test/node/src/migration.test.ts

@@ -4,21 +4,22 @@ import { dirname, join } from 'pathe'
 import { createSandbox, type SinonSpy } from 'sinon'
 import {
   FileMigrationProvider,
-  Migration,
-  MigrationResultSet,
+  type Migration,
+  type MigrationResultSet,
   DEFAULT_MIGRATION_LOCK_TABLE,
   DEFAULT_MIGRATION_TABLE,
+  type MigrationProvider,
   Migrator,
   NO_MIGRATIONS,
-  MigratorProps,
+  type MigratorProps,
   type Kysely,
 } from '../../../dist/cjs/index.js'
 import {
   clearDatabase,
   destroyTest,
   expect,
   initTest,
-  TestContext,
+  type TestContext,
   DIALECTS,
   type Database,
 } from './test-setup.js'
@@ -868,6 +869,72 @@ for (const dialect of DIALECTS) {
           expect(transactionSpy.called).to.be.false
         }
       })
+
+      if (sqlSpec === 'postgres' || sqlSpec === 'mssql') {
+        it('should run migrations using a provided transaction', async () => {
+          const migrationName = 'trx_connection_method_fail_case'
+
+          const trx = await ctx.db.startTransaction().execute()
+
+          try {
+            const provider: MigrationProvider = {
+              getMigrations: async () => ({
+                [migrationName]: {
+                  async up(db: Kysely<any>): Promise<void> {
+                    expect(db).to.equal(trx)
+                  },
+                },
+              }),
+            }
+
+            const migrator = new Migrator({ db: trx, provider })
+
+            const { results, error } = await migrator.migrateUp()
+
+            expect(trx.isCommitted).to.be.false
+            expect(trx.isRolledBack).to.be.false
+
+            expect(error).to.be.undefined
+            expect(results).to.eql([
+              { migrationName, direction: 'Up', status: 'Success' },
+            ])
+          } finally {
+            await trx.rollback().execute()
+          }
+        })
+      }
+
+      if (sqlSpec === 'mysql' || sqlSpec === 'sqlite') {
+        it('should refuse to run migrations using a provided transaction due to lack of support for transactional DDL', async () => {
+          const trx = await ctx.db.startTransaction().execute()
+          try {
+            const provider: MigrationProvider = {
+              getMigrations: async () => ({
+                some_migration: {
+                  async up(db: Kysely<any>): Promise<void> {
+                    expect(db).to.equal(trx)
+                  },
+                },
+              }),
+            }
+
+            const migrator = new Migrator({ db: trx, provider })
+
+            const { results, error } = await migrator.migrateUp()
+
+            expect(error).to.be.an.instanceOf(Error)
+            expect(getMessage(error)).to.eql(
+              'Transactional DDL is not supported in this dialect. Passing a transaction to this migrator would result in failure or unexpected behavior.',
+            )
+            expect(results).to.be.undefined
+
+            expect(trx.isCommitted).to.be.false
+            expect(trx.isRolledBack).to.be.false
+          } finally {
+            await trx.rollback().execute()
+          }
+        })
+      }
     })
 
     describe('migrateDown', () => {