Skip to content

[SQLite-kit] Fix column rename breaking INSERT INTO...SELECT during table recreate #5597

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
funsaized wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[SQLite-kit] Fix column rename breaking INSERT INTO...SELECT during table recreate #5597

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

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions drizzle-kit/src/jsonStatements.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export interface JsonRecreateTableStatement {
compositePKs: string[][];
uniqueConstraints?: string[];
checkConstraints: string[];
columnRenames?: { oldName: string; newName: string }[];
}

export interface JsonRecreateSingleStoreTableStatement {
Expand Down
32 changes: 28 additions & 4 deletions drizzle-kit/src/sqlgenerator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3769,11 +3769,23 @@ class SQLiteRecreateTableConvertor extends Convertor {
}

convert(statement: JsonRecreateTableStatement): string | string[] {
const { tableName, columns, compositePKs, referenceData, checkConstraints } = statement;
const { tableName, columns, compositePKs, referenceData, checkConstraints, columnRenames } = statement;

const columnNames = columns.map((it) => `"${it.name}"`).join(', ');
const newTableName = `__new_${tableName}`;

const renameMap = new Map<string, string>();
if (columnRenames) {
for (const { oldName, newName } of columnRenames) {
renameMap.set(newName, oldName);
}
}

const selectColumnNames = columns.map((it) => {
const oldName = renameMap.get(it.name);
return oldName ? `"${oldName}"` : `"${it.name}"`;
}).join(', ');

const sqlStatements: string[] = [];

sqlStatements.push(`PRAGMA foreign_keys=OFF;`);
Expand All @@ -3798,7 +3810,7 @@ class SQLiteRecreateTableConvertor extends Convertor {

// migrate data
sqlStatements.push(
`INSERT INTO \`${newTableName}\`(${columnNames}) SELECT ${columnNames} FROM \`${tableName}\`;`,
`INSERT INTO \`${newTableName}\`(${columnNames}) SELECT ${selectColumnNames} FROM \`${tableName}\`;`,
);

// drop table
Expand Down Expand Up @@ -3836,11 +3848,23 @@ class LibSQLRecreateTableConvertor extends Convertor {
}

convert(statement: JsonRecreateTableStatement): string[] {
const { tableName, columns, compositePKs, referenceData, checkConstraints } = statement;
const { tableName, columns, compositePKs, referenceData, checkConstraints, columnRenames } = statement;

const columnNames = columns.map((it) => `"${it.name}"`).join(', ');
const newTableName = `__new_${tableName}`;

const renameMap = new Map<string, string>();
if (columnRenames) {
for (const { oldName, newName } of columnRenames) {
renameMap.set(newName, oldName);
}
}

const selectColumnNames = columns.map((it) => {
const oldName = renameMap.get(it.name);
return oldName ? `"${oldName}"` : `"${it.name}"`;
}).join(', ');

const sqlStatements: string[] = [];

const mappedCheckConstraints: string[] = checkConstraints.map((it) =>
Expand All @@ -3864,7 +3888,7 @@ class LibSQLRecreateTableConvertor extends Convertor {

// migrate data
sqlStatements.push(
`INSERT INTO \`${newTableName}\`(${columnNames}) SELECT ${columnNames} FROM \`${tableName}\`;`,
`INSERT INTO \`${newTableName}\`(${columnNames}) SELECT ${selectColumnNames} FROM \`${tableName}\`;`,
);

// drop table
Expand Down
45 changes: 45 additions & 0 deletions drizzle-kit/src/statementCombiner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
JsonCreateIndexStatement,
JsonRecreateTableStatement,
JsonRenameColumnStatement,
JsonStatement,
prepareCreateIndexesJson,
} from './jsonStatements';
Expand Down Expand Up @@ -80,7 +81,17 @@ export const libSQLCombineStatements = (
) => {
// const tablesContext: Record<string, string[]> = {};
const newStatements: Record<string, JsonStatement[]> = {};
const columnRenamesByTable: Record<string, { oldName: string; newName: string }[]> = {};

for (const statement of statements) {
if (statement.type === 'alter_table_rename_column') {
const s = statement as unknown as JsonRenameColumnStatement;
if (!columnRenamesByTable[s.tableName]) {
columnRenamesByTable[s.tableName] = [];
}
columnRenamesByTable[s.tableName].push({ oldName: s.oldColumnName, newName: s.newColumnName });
}

if (
statement.type === 'alter_table_alter_column_drop_autoincrement'
|| statement.type === 'alter_table_alter_column_set_autoincrement'
Expand Down Expand Up @@ -293,6 +304,18 @@ export const libSQLCombineStatements = (
}
}

for (const [tableName, renames] of Object.entries(columnRenamesByTable)) {
const stmts = newStatements[tableName];
if (stmts) {
const recreate = stmts.find((s) => s.type === 'recreate_table') as JsonRecreateTableStatement | undefined;
if (recreate) {
recreate.columnRenames = renames;
// drop retained rename_column stmts — recreate handles them via columnRenames
newStatements[tableName] = stmts.filter((s) => s.type !== 'alter_table_rename_column');
}
}
}

const combinedStatements = Object.values(newStatements).flat();
const renamedTables = combinedStatements.filter((it) => it.type === 'rename_table');
const renamedColumns = combinedStatements.filter((it) => it.type === 'alter_table_rename_column');
Expand All @@ -309,7 +332,17 @@ export const sqliteCombineStatements = (
) => {
// const tablesContext: Record<string, string[]> = {};
const newStatements: Record<string, JsonStatement[]> = {};
const columnRenamesByTable: Record<string, { oldName: string; newName: string }[]> = {};

for (const statement of statements) {
if (statement.type === 'alter_table_rename_column') {
const s = statement as unknown as JsonRenameColumnStatement;
if (!columnRenamesByTable[s.tableName]) {
columnRenamesByTable[s.tableName] = [];
}
columnRenamesByTable[s.tableName].push({ oldName: s.oldColumnName, newName: s.newColumnName });
}

if (
statement.type === 'alter_table_alter_column_set_type'
|| statement.type === 'alter_table_alter_column_set_default'
Expand Down Expand Up @@ -436,6 +469,18 @@ export const sqliteCombineStatements = (
}
}

for (const [tableName, renames] of Object.entries(columnRenamesByTable)) {
const stmts = newStatements[tableName];
if (stmts) {
const recreate = stmts.find((s) => s.type === 'recreate_table') as JsonRecreateTableStatement | undefined;
if (recreate) {
recreate.columnRenames = renames;
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Skip columnRenames when rename_column statement is retained

This assignment copies every alter_table_rename_column into recreate_table, even in flows where the rename statement is still executed (for example when rename_table is present and the combiner keeps prior statements). In that case migration order becomes RENAME COLUMN ... followed by recreate SQL that now selects the old name from the already-renamed table, so INSERT INTO ... SELECT fails with a missing-column error. columnRenames should only include renames that were removed from the final statement list, not renames that will run explicitly.

Useful? React with 👍 / 👎.

// drop retained rename_column stmts — recreate handles them via columnRenames
newStatements[tableName] = stmts.filter((s) => s.type !== 'alter_table_rename_column');
}
}
}

const combinedStatements = Object.values(newStatements).flat();

const renamedTables = combinedStatements.filter((it) => it.type === 'rename_table');
Expand Down
80 changes: 80 additions & 0 deletions drizzle-kit/tests/statements-combiner/sqlite-statements-combiner.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ test(`renamed column and altered this column type`, async (t) => {
referenceData: [],
uniqueConstraints: [],
checkConstraints: [],
columnRenames: [{ oldName: 'lastName', newName: 'lastName123' }],
},
];
expect(sqliteCombineStatements(statements, json2)).toStrictEqual(
Expand Down Expand Up @@ -1209,3 +1210,82 @@ test(`add column and fk`, async (t) => {
newJsonStatements,
);
});

test(`renamed table + renamed column + altered column type should absorb rename into recreate`, async (t) => {
const statements: JsonStatement[] = [
{
type: 'rename_table',
tableNameFrom: 'task',
tableNameTo: 'todo',
schema: '',
} as unknown as JsonStatement,
{
type: 'alter_table_rename_column',
tableName: 'todo',
oldColumnName: 'is_invisible',
newColumnName: 'is_visible',
schema: '',
},
{
type: 'alter_table_alter_column_set_type',
tableName: 'todo',
columnName: 'is_visible',
newDataType: 'integer',
oldDataType: 'text',
schema: '',
columnDefault: undefined,
columnOnUpdate: undefined,
columnNotNull: false,
columnAutoIncrement: false,
columnPk: false,
columnIsUnique: false,
} as unknown as JsonStatement,
];

const json2: SQLiteSchemaSquashed = {
version: '6',
dialect: 'sqlite',
tables: {
todo: {
name: 'todo',
columns: {
id: {
name: 'id',
type: 'integer',
primaryKey: true,
notNull: true,
autoincrement: false,
},
is_visible: {
name: 'is_visible',
type: 'integer',
primaryKey: false,
notNull: false,
autoincrement: false,
},
},
indexes: {},
foreignKeys: {},
compositePrimaryKeys: {},
uniqueConstraints: {},
checkConstraints: {},
},
},
enums: {},
views: {},
};

const result = sqliteCombineStatements(statements, json2);

const renameColStmts = result.filter((s) => s.type === 'alter_table_rename_column');
expect(renameColStmts).toHaveLength(0);

const renameTableStmts = result.filter((s) => s.type === 'rename_table');
expect(renameTableStmts).toHaveLength(1);

const recreateStmts = result.filter((s) => s.type === 'recreate_table');
expect(recreateStmts).toHaveLength(1);
expect((recreateStmts[0] as any).columnRenames).toStrictEqual([
{ oldName: 'is_invisible', newName: 'is_visible' },
]);
});