From a3bc76a6c9e8af393dbfaa52b348b0b1f48bd4fc Mon Sep 17 00:00:00 2001 From: Meno Abels Date: Fri, 16 Jan 2026 11:18:04 +0100 Subject: [PATCH] fix(drizzle-kit): prevent duplicate CREATE INDEX on table recreation When a table with indexes needed recreation (e.g., composite PK changes), CREATE INDEX statements were being emitted twice: - Once from _moveDataStatements() during table recreation - Again from separate create_index JSON statements This resulted in SQL like: CREATE UNIQUE INDEX `UserSlug_userSlug` ... PRAGMA foreign_keys=ON; CREATE UNIQUE INDEX `UserSlug_userSlug` ... <-- duplicate! Fix: Track which tables have been recreated and skip create_index statements for those tables since indexes are already created in _moveDataStatements(). Added regression test with UserSlugBindings table schema that has composite primary key, uniqueIndex, and regular index. --- .../src/cli/commands/libSqlPushUtils.ts | 8 ++ .../src/cli/commands/sqlitePushUtils.ts | 8 ++ drizzle-kit/tests/push/libsql.test.ts | 76 +++++++++++++++++++ 3 files changed, 92 insertions(+) diff --git a/drizzle-kit/src/cli/commands/libSqlPushUtils.ts b/drizzle-kit/src/cli/commands/libSqlPushUtils.ts index 31e90c8722..cbd0d9fd3f 100644 --- a/drizzle-kit/src/cli/commands/libSqlPushUtils.ts +++ b/drizzle-kit/src/cli/commands/libSqlPushUtils.ts @@ -125,6 +125,9 @@ export const libSqlLogSuggestionsAndReturn = async ( const columnsToRemove: string[] = []; const tablesToTruncate: string[] = []; + // Track tables that have been recreated to avoid duplicate index creation + const recreatedTables = new Set(); + for (const statement of statements) { if (statement.type === 'drop_table') { const res = await connection.query<{ count: string }>( @@ -231,6 +234,8 @@ export const libSqlLogSuggestionsAndReturn = async ( ); } else if (statement.type === 'recreate_table') { const tableName = statement.tableName; + // Mark table as recreated to skip duplicate index creation later + recreatedTables.add(tableName); let dataLoss = false; @@ -336,6 +341,9 @@ export const libSqlLogSuggestionsAndReturn = async ( statementsToExecute.push( ...(Array.isArray(fromJsonStatement) ? fromJsonStatement : [fromJsonStatement]), ); + } else if (statement.type === 'create_index' && recreatedTables.has(statement.tableName)) { + // Skip create_index for recreated tables - indexes are already created in _moveDataStatements + continue; } else { const fromJsonStatement = fromJson([statement], 'turso', 'push', json2); statementsToExecute.push( diff --git a/drizzle-kit/src/cli/commands/sqlitePushUtils.ts b/drizzle-kit/src/cli/commands/sqlitePushUtils.ts index a18b369451..b117096ed1 100644 --- a/drizzle-kit/src/cli/commands/sqlitePushUtils.ts +++ b/drizzle-kit/src/cli/commands/sqlitePushUtils.ts @@ -140,6 +140,9 @@ export const logSuggestionsAndReturn = async ( const schemasToRemove: string[] = []; const tablesToTruncate: string[] = []; + // Track tables that have been recreated to avoid duplicate index creation + const recreatedTables = new Set(); + for (const statement of statements) { if (statement.type === 'drop_table') { const res = await connection.query<{ count: string }>( @@ -217,6 +220,8 @@ export const logSuggestionsAndReturn = async ( ); } else if (statement.type === 'recreate_table') { const tableName = statement.tableName; + // Mark table as recreated to skip duplicate index creation later + recreatedTables.add(tableName); const oldTableName = getOldTableName(tableName, meta); let dataLoss = false; @@ -302,6 +307,9 @@ export const logSuggestionsAndReturn = async ( if (pragmaState) { statementsToExecute.push(`PRAGMA foreign_keys=ON;`); } + } else if (statement.type === 'create_index' && recreatedTables.has(statement.tableName)) { + // Skip create_index for recreated tables - indexes are already created in _moveDataStatements + continue; } else { const fromJsonStatement = fromJson([statement], 'sqlite', 'push'); statementsToExecute.push( diff --git a/drizzle-kit/tests/push/libsql.test.ts b/drizzle-kit/tests/push/libsql.test.ts index 2ae2e38110..e7e68b10fe 100644 --- a/drizzle-kit/tests/push/libsql.test.ts +++ b/drizzle-kit/tests/push/libsql.test.ts @@ -10,6 +10,7 @@ import { int, integer, numeric, + primaryKey, real, sqliteTable, sqliteView, @@ -1398,3 +1399,78 @@ test('alter view ".as"', async () => { expect(statements.length).toBe(0); expect(sqlStatements.length).toBe(0); }); + +// Regression test for duplicate CREATE INDEX on table recreation +// Bug: When a recreate_table occurs, CREATE INDEX statements were emitted twice: +// CREATE UNIQUE INDEX `UserSlug_userSlug` ... +// PRAGMA foreign_keys=ON; +// CREATE UNIQUE INDEX `UserSlug_userSlug` ... <-- duplicate! +test('recreate table with composite pk, unique index and regular index should not duplicate CREATE INDEX', async (t) => { + // Schema with composite PK, uniqueIndex, and regular index (matches UserSlugBindings) + const schema = { + UserSlugBindings: sqliteTable( + 'UserSlugBindings', + { + userId: text('userId').notNull(), + userSlug: text('userSlug').notNull(), + created: text('created').notNull(), + }, + (table) => [ + primaryKey({ columns: [table.userSlug, table.userId] }), + uniqueIndex('UserSlug_userSlug').on(table.userSlug), + index('UserSlug_created').on(table.created), + ], + ), + }; + + const turso = createClient({ url: ':memory:' }); + + // ==================== FIRST PUSH: empty DB -> schema ==================== + // This simulates the initial `drizzle-kit push` to an empty database + const { + sqlStatements: firstPushStatements, + } = await diffTestSchemasPushLibSQL(turso, {}, schema, [], false); + + // First push should have clean CREATE statements, no ALTER/recreate + expect(firstPushStatements.some((s) => s.includes('CREATE TABLE `UserSlugBindings`'))).toBe(true); + expect(firstPushStatements.some((s) => s.includes('CREATE UNIQUE INDEX `UserSlug_userSlug`'))).toBe(true); + expect(firstPushStatements.some((s) => s.includes('CREATE INDEX `UserSlug_created`'))).toBe(true); + + // No ALTER TABLE or __new_ (recreate) statements on first push + expect(firstPushStatements.some((s) => s.includes('ALTER TABLE'))).toBe(false); + expect(firstPushStatements.some((s) => s.includes('__new_'))).toBe(false); + + // Each index should appear exactly once + const firstPushUniqueIndexCount = firstPushStatements.filter( + (s) => s.includes('CREATE UNIQUE INDEX') && s.includes('UserSlug_userSlug'), + ).length; + const firstPushRegularIndexCount = firstPushStatements.filter( + (s) => s.includes('CREATE INDEX') && s.includes('UserSlug_created'), + ).length; + expect(firstPushUniqueIndexCount).toBe(1); + expect(firstPushRegularIndexCount).toBe(1); + + // ==================== SECOND PUSH: schema -> schema ==================== + // This simulates running `drizzle-kit push` again on the same DB + const { + sqlStatements: secondPushStatements, + columnsToRemove, + tablesToRemove, + } = await diffTestSchemasPushLibSQL(turso, schema, schema, [], false); + + // Count CREATE INDEX occurrences in second push output + const secondPushUniqueIndexCount = secondPushStatements.filter( + (s) => s.includes('CREATE UNIQUE INDEX') && s.includes('UserSlug_userSlug'), + ).length; + const secondPushRegularIndexCount = secondPushStatements.filter( + (s) => s.includes('CREATE INDEX') && s.includes('UserSlug_created'), + ).length; + + // CRITICAL: Each index should appear at most ONCE, not twice + // Bug was: _moveDataStatements created indexes, then create_index statements added duplicates + expect(secondPushUniqueIndexCount).toBeLessThanOrEqual(1); + expect(secondPushRegularIndexCount).toBeLessThanOrEqual(1); + + expect(columnsToRemove!.length).toBe(0); + expect(tablesToRemove!.length).toBe(0); +});