[BUG]: Table already exists error during migrations when migrations are run the first time

[winghouchan]

Report hasn't been filed before.

• I have verified that the bug I'm about to report hasn't been filed before.

What version of drizzle-orm are you using?

0.36.4

What version of drizzle-kit are you using?

0.27.2

Other packages

No response

Describe the Bug

Environment

• Runtime: Hermes (React Native)
• Driver: OP SQLite

Steps to reproduce

Setup

1. Create a React Native app with OP SQLite that applies a database migration (contents of migration is not significant).

Reproduction

1. Start the app, ensuring there is no existing database (the app has been freshly installed or the app data has been cleared).

Expected behaviour

Migrations are applied successfully.

Actual behaviour

Migrations error with message saying the first table that appears in the migration already exists.

[winghouchan]

Investigation notes

Factor: migrations start in useEffect hook

Migrations are applied in React Native using the useMigrations hook from the relevant driver (documentation for OP SQLite). For OP SQLite, the hook looks like this:

drizzle-orm/drizzle-orm/src/op-sqlite/migrator.ts

Lines 41 to 99 in 10d2230

	export async function migrate<TSchema extends Record<string, unknown>>(
	db: OPSQLiteDatabase<TSchema>,
	config: MigrationConfig,
	) {
	const migrations = await readMigrationFiles(config);
	return db.dialect.migrate(migrations, db.session);
	}
	interface State {
	success: boolean;
	error?: Error;
	}
	type Action =
	| { type: 'migrating' }
	| { type: 'migrated'; payload: true }
	| { type: 'error'; payload: Error };
	export const useMigrations = (db: OPSQLiteDatabase<any>, migrations: {
	journal: {
	entries: { idx: number; when: number; tag: string; breakpoints: boolean }[];
	};
	migrations: Record<string, string>;
	}): State => {
	const initialState: State = {
	success: false,
	error: undefined,
	};
	const fetchReducer = (state: State, action: Action): State => {
	switch (action.type) {
	case 'migrating': {
	return { ...initialState };
	}
	case 'migrated': {
	return { ...initialState, success: action.payload };
	}
	case 'error': {
	return { ...initialState, error: action.payload };
	}
	default: {
	return state;
	}
	}
	};
	const [state, dispatch] = useReducer(fetchReducer, initialState);
	useEffect(() => {
	dispatch({ type: 'migrating' });
	migrate(db, migrations).then(() => {
	dispatch({ type: 'migrated', payload: true });
	}).catch((error) => {
	dispatch({ type: 'error', payload: error as Error });
	});
	}, []);
	return state;
	};

What is significant here is the migrate function, which calls SQLite core driver's migrate function (at L46), is run inside the useEffect hook (at L91). When React is run in strict mode, effects will be run twice (documentation). This means the migration will be run twice, a factor in causing the error saying a table already exists.

Factor: asynchronous APIs

Another point of significance is OP SQLite having an asynchronous API. Expo SQLite's useMigration hook is similar in that it runs the migrate function inside a useEffect hook. However, as the Expo SQLite driver uses Expo SQLite's synchronous API (source), the second migration is guaranteed to run after the first one has completed. This means the checks that are run to determine if the migration should be applied have the state of the database after the first migration is applied and determines the second migration should not be applied:

drizzle-orm/drizzle-orm/src/sqlite-core/dialect.ts

Lines 823 to 835 in 10d2230

	const dbMigrations = session.values<[number, string, string]>(
	sql`SELECT id, hash, created_at FROM ${sql.identifier(migrationsTable)} ORDER BY created_at DESC LIMIT 1`,
	);
	const lastDbMigration = dbMigrations[0] ?? undefined;
	session.run(sql`BEGIN`);
	try {
	for (const migration of migrations) {
	if (!lastDbMigration || Number(lastDbMigration[2])! < migration.folderMillis) {
	for (const stmt of migration.sql) {
	session.run(sql.raw(stmt));
	}

For asynchronous APIs, the check is similar:

drizzle-orm/drizzle-orm/src/sqlite-core/dialect.ts

Lines 875 to 886 in 10d2230

	const dbMigrations = await session.values<[number, string, string]>(
	sql`SELECT id, hash, created_at FROM ${sql.identifier(migrationsTable)} ORDER BY created_at DESC LIMIT 1`,
	);
	const lastDbMigration = dbMigrations[0] ?? undefined;
	await session.transaction(async (tx) => {
	for (const migration of migrations) {
	if (!lastDbMigration || Number(lastDbMigration[2])! < migration.folderMillis) {
	for (const stmt of migration.sql) {
	await tx.run(sql.raw(stmt));
	}

However, as the second migration begins before the first migration has been completed, the database read for the last migration (L875–L877) doesn't return anything so the check (L833) does not prevent the migration from being applied a second time, another factor in causing the error saying a table already exists.

Factor: broken transactions

As seen above, the statements that apply the migration run inside a transaction. In SQLite, only 1 simultaneous write transaction is allowed (SQLite documentation). As a result, it would be expected that the application is informed the database is busy. However, asynchronous transactions are ended prematurely. The logger shows something like this:

begin
<migration 1, statement 1>
commit
begin
<migration 2, statement 1>
commit
<migration 1, statement n>
// migration 2 no longer continues because of the duplicate table error

This issue is being tracked in #2275.

[winghouchan]

Fixing broken transactions by awaiting the statements that build the transaction and the transaction itself doesn't actually cause the database to inform the application the database is busy. As the statements that make up the transaction are sent individually, something like this happens:

begin
begin

This causes an SQLite error saying transactions cannot be nested.

Example patch

diff --git a/node_modules/drizzle-orm/op-sqlite/session.js b/node_modules/drizzle-orm/op-sqlite/session.js
index ff84604..14cf731 100644
--- a/node_modules/drizzle-orm/op-sqlite/session.js
+++ b/node_modules/drizzle-orm/op-sqlite/session.js
@@ -27,31 +27,31 @@ class OPSQLiteSession extends SQLiteSession {
       customResultMapper
     );
   }
-  transaction(transaction, config = {}) {
+  async transaction(transaction, config = {}) {
     const tx = new OPSQLiteTransaction("async", this.dialect, this, this.schema);
-    this.run(sql.raw(`begin${config?.behavior ? " " + config.behavior : ""}`));
+    await this.run(sql.raw(`begin${config?.behavior ? " " + config.behavior : ""}`));
     try {
-      const result = transaction(tx);
-      this.run(sql`commit`);
+      const result = await transaction(tx);
+      await await this.run(sql`commit`);
       return result;
     } catch (err) {
-      this.run(sql`rollback`);
+      await this.run(sql`rollback`);
       throw err;
     }
   }
 }
 class OPSQLiteTransaction extends SQLiteTransaction {
   static [entityKind] = "OPSQLiteTransaction";
-  transaction(transaction) {
+  async transaction(transaction) {
     const savepointName = `sp${this.nestedIndex}`;
     const tx = new OPSQLiteTransaction("async", this.dialect, this.session, this.schema, this.nestedIndex + 1);
-    this.session.run(sql.raw(`savepoint ${savepointName}`));
+    await this.session.run(sql.raw(`savepoint ${savepointName}`));
     try {
       const result = transaction(tx);
-      this.session.run(sql.raw(`release savepoint ${savepointName}`));
+      await this.session.run(sql.raw(`release savepoint ${savepointName}`));
       return result;
     } catch (err) {
-      this.session.run(sql.raw(`rollback to savepoint ${savepointName}`));
+      await this.session.run(sql.raw(`rollback to savepoint ${savepointName}`));
       throw err;
     }
   }

[winghouchan]

Here's a example patch of making the useMigrations hook resilient to being called multiple times:

diff --git a/node_modules/drizzle-orm/op-sqlite/migrator.js b/node_modules/drizzle-orm/op-sqlite/migrator.js
index 02354ae..28f71a9 100644
--- a/node_modules/drizzle-orm/op-sqlite/migrator.js
+++ b/node_modules/drizzle-orm/op-sqlite/migrator.js
@@ -1,4 +1,4 @@
-import { useEffect, useReducer } from "react";
+import { useEffect, useReducer, useState } from "react";
 async function readMigrationFiles({ journal, migrations }) {
   const migrationQueries = [];
   for await (const journalEntry of journal.entries) {
@@ -48,14 +48,34 @@ const useMigrations = (db, migrations) => {
     }
   };
   const [state, dispatch] = useReducer(fetchReducer, initialState);
-  useEffect(() => {
-    dispatch({ type: "migrating" });
-    migrate(db, migrations).then(() => {
+  const [migrationQueue, setMigrationQueue] = useState([])
+
+  const runMigrations = async () => {
+    try {
+      dispatch({ type: 'migrating' })
+      await migrationQueue[0](db, migrations)
       dispatch({ type: "migrated", payload: true });
-    }).catch((error) => {
+    } catch (error) {
       dispatch({ type: "error", payload: error });
-    });
+    } finally {
+      setMigrationQueue((currentMigrations) => currentMigrations.slice(1))
+    }
+  }
+
+  useEffect(() => {
+    setMigrationQueue((currentMigrations) => [...currentMigrations, migrate])
+
+    return () => {
+      setMigrationQueue((currentMigrations) => currentMigrations.slice(0, -1))
+    }
   }, []);
+
+  useEffect(() => {
+    if (migrationQueue.length) {
+      runMigrations()
+    }
+  }, [migrationQueue.length])
+
   return state;
 };
 export {

How it works:

1. Migrations are pushed to a queue via an effect. This effect's cleanup function removes the pushed migration from the queue.
2. An effect reacts to the queue length. When it changes, it runs the first migration then removes it from the queue. As the queue length changes due to a migration being removed upon completion, the effect will keep running until no migrations are left.

Warning

This doesn't solve useMigrations being called in multiple places (not sure why you would). It only solves useMigrations being called in a single place multiple times.