Skip to content

[Sqlite-kit]: Avoid duplicate CREATE INDEX during push table recreation #4991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Sqlite-kit]: Avoid duplicate CREATE INDEX during push table recreation #4991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 11 additions & 23 deletions drizzle-kit/src/cli/commands/libSqlPushUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { JsonStatement } from 'src/jsonStatements';
import { findAddedAndRemoved, SQLiteDB } from 'src/utils';
import { SQLiteSchemaInternal, SQLiteSchemaSquashed, SQLiteSquasher } from '../../serializer/sqliteSchema';
import {
CreateSqliteIndexConvertor,
fromJson,
LibSQLModifyColumn,
SQLiteCreateTableConvertor,
Expand Down Expand Up @@ -86,28 +85,17 @@ export const _moveDataStatements = (
}),
);

// rename table
statements.push(
new SqliteRenameTableConvertor().convert({
fromSchema: '',
tableNameFrom: newTableName,
tableNameTo: tableName,
toSchema: '',
type: 'rename_table',
}),
);

for (const idx of Object.values(json.tables[tableName].indexes)) {
statements.push(
new CreateSqliteIndexConvertor().convert({
type: 'create_index',
tableName: tableName,
schema: '',
data: idx,
}),
);
}
return statements;
// rename table
statements.push(
new SqliteRenameTableConvertor().convert({
fromSchema: '',
tableNameFrom: newTableName,
tableNameTo: tableName,
toSchema: '',
type: 'rename_table',
}),
);
return statements;
};

export const libSqlLogSuggestionsAndReturn = async (
Expand Down
44 changes: 16 additions & 28 deletions drizzle-kit/src/cli/commands/sqlitePushUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import chalk from 'chalk';

import { SQLiteSchemaInternal, SQLiteSchemaSquashed, SQLiteSquasher } from '../../serializer/sqliteSchema';
import {
CreateSqliteIndexConvertor,
fromJson,
SQLiteCreateTableConvertor,
SQLiteDropTableConvertor,
Expand All @@ -13,11 +12,11 @@ import type { JsonStatement } from '../../jsonStatements';
import { findAddedAndRemoved, type SQLiteDB } from '../../utils';

export const _moveDataStatements = (
tableName: string,
json: SQLiteSchemaSquashed,
dataLoss: boolean = false,
tableName: string,
json: SQLiteSchemaSquashed,
dataLoss: boolean = false,
) => {
const statements: string[] = [];
const statements: string[] = [];

const newTableName = `__new_${tableName}`;

Expand Down Expand Up @@ -73,29 +72,18 @@ export const _moveDataStatements = (
}),
);

// rename table
statements.push(
new SqliteRenameTableConvertor().convert({
fromSchema: '',
tableNameFrom: newTableName,
tableNameTo: tableName,
toSchema: '',
type: 'rename_table',
}),
);

for (const idx of Object.values(json.tables[tableName].indexes)) {
statements.push(
new CreateSqliteIndexConvertor().convert({
type: 'create_index',
tableName: tableName,
schema: '',
data: idx,
}),
);
}

return statements;
// rename table
statements.push(
new SqliteRenameTableConvertor().convert({
fromSchema: '',
tableNameFrom: newTableName,
tableNameTo: tableName,
toSchema: '',
type: 'rename_table',
}),
);

return statements;
};

export const getOldTableName = (
Expand Down
49 changes: 49 additions & 0 deletions drizzle-kit/tests/push/sqlite.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1297,6 +1297,55 @@ test('add check constraint to table', async (t) => {
expect(tablesToTruncate!.length).toBe(0);
});

test('recreate table with existing unique index does not duplicate index creation', async () => {
const client = new Database(':memory:');

// Base schema with a unique index on email
const schema1 = {
users: sqliteTable(
'users',
{
id: int('id').primaryKey({ autoIncrement: false }),
name: text('name'),
email: text('email').notNull().unique(),
},
(t) => ({
// also define an explicit unique index to cover both paths
emailIdx: uniqueIndex('users_email_unique').on(t.email),
}),
),
};

// Change that forces table recreation (add check constraint)
const schema2 = {
users: sqliteTable(
'users',
{
id: int('id').primaryKey({ autoIncrement: false }),
name: text('name'),
email: text('email').notNull().unique(),
},
(table) => ({
emailIdx: uniqueIndex('users_email_unique').on(table.email),
someCheck: check('some_check', sql`${table.id} >= 0`),
}),
),
};

const { sqlStatements } = await diffTestSchemasPushSqlite(
client,
schema1,
schema2,
[],
);

// Ensure only one CREATE UNIQUE INDEX for users_email_unique exists
const createIdx = sqlStatements.filter((s) =>
s.startsWith('CREATE UNIQUE INDEX `users_email_unique`')
);
expect(createIdx.length).toBe(1);
});

test('drop check constraint', async (t) => {
const client = new Database(':memory:');

Expand Down