Skip to content

Commit 22c20fc

Browse files
funsaizedsisyphus-dev-ai
funsaized
and
sisyphus-dev-ai
committed
fix: use old column names in SELECT during SQLite table recreate on column rename
When rename_column coincides with table recreation, the generated INSERT INTO...SELECT now correctly uses old column names in SELECT while using new names in INSERT. Also strips retained rename_column statements when a recreate_table absorbs them via columnRenames, preventing double-rename when rename_table is present. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
1 parent 273c780 commit 22c20fc

4 files changed

Lines changed: 154 additions & 4 deletions

File tree

‎drizzle-kit/src/jsonStatements.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export interface JsonRecreateTableStatement {
7272
compositePKs: string[][];
7373
uniqueConstraints?: string[];
7474
checkConstraints: string[];
75+
columnRenames?: { oldName: string; newName: string }[];
7576
}
7677

7778
export interface JsonRecreateSingleStoreTableStatement {

‎drizzle-kit/src/sqlgenerator.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3769,11 +3769,23 @@ class SQLiteRecreateTableConvertor extends Convertor {
37693769
}
37703770

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

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

3777+
const renameMap = new Map<string, string>();
3778+
if (columnRenames) {
3779+
for (const { oldName, newName } of columnRenames) {
3780+
renameMap.set(newName, oldName);
3781+
}
3782+
}
3783+
3784+
const selectColumnNames = columns.map((it) => {
3785+
const oldName = renameMap.get(it.name);
3786+
return oldName ? `"${oldName}"` : `"${it.name}"`;
3787+
}).join(', ');
3788+
37773789
const sqlStatements: string[] = [];
37783790

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

37993811
// migrate data
38003812
sqlStatements.push(
3801-
`INSERT INTO \`${newTableName}\`(${columnNames}) SELECT ${columnNames} FROM \`${tableName}\`;`,
3813+
`INSERT INTO \`${newTableName}\`(${columnNames}) SELECT ${selectColumnNames} FROM \`${tableName}\`;`,
38023814
);
38033815

38043816
// drop table
@@ -3836,11 +3848,23 @@ class LibSQLRecreateTableConvertor extends Convertor {
38363848
}
38373849

38383850
convert(statement: JsonRecreateTableStatement): string[] {
3839-
const { tableName, columns, compositePKs, referenceData, checkConstraints } = statement;
3851+
const { tableName, columns, compositePKs, referenceData, checkConstraints, columnRenames } = statement;
38403852

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

3856+
const renameMap = new Map<string, string>();
3857+
if (columnRenames) {
3858+
for (const { oldName, newName } of columnRenames) {
3859+
renameMap.set(newName, oldName);
3860+
}
3861+
}
3862+
3863+
const selectColumnNames = columns.map((it) => {
3864+
const oldName = renameMap.get(it.name);
3865+
return oldName ? `"${oldName}"` : `"${it.name}"`;
3866+
}).join(', ');
3867+
38443868
const sqlStatements: string[] = [];
38453869

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

38653889
// migrate data
38663890
sqlStatements.push(
3867-
`INSERT INTO \`${newTableName}\`(${columnNames}) SELECT ${columnNames} FROM \`${tableName}\`;`,
3891+
`INSERT INTO \`${newTableName}\`(${columnNames}) SELECT ${selectColumnNames} FROM \`${tableName}\`;`,
38683892
);
38693893

38703894
// drop table

‎drizzle-kit/src/statementCombiner.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
JsonCreateIndexStatement,
33
JsonRecreateTableStatement,
4+
JsonRenameColumnStatement,
45
JsonStatement,
56
prepareCreateIndexesJson,
67
} from './jsonStatements';
@@ -80,7 +81,17 @@ export const libSQLCombineStatements = (
8081
) => {
8182
// const tablesContext: Record<string, string[]> = {};
8283
const newStatements: Record<string, JsonStatement[]> = {};
84+
const columnRenamesByTable: Record<string, { oldName: string; newName: string }[]> = {};
85+
8386
for (const statement of statements) {
87+
if (statement.type === 'alter_table_rename_column') {
88+
const s = statement as unknown as JsonRenameColumnStatement;
89+
if (!columnRenamesByTable[s.tableName]) {
90+
columnRenamesByTable[s.tableName] = [];
91+
}
92+
columnRenamesByTable[s.tableName].push({ oldName: s.oldColumnName, newName: s.newColumnName });
93+
}
94+
8495
if (
8596
statement.type === 'alter_table_alter_column_drop_autoincrement'
8697
|| statement.type === 'alter_table_alter_column_set_autoincrement'
@@ -293,6 +304,18 @@ export const libSQLCombineStatements = (
293304
}
294305
}
295306

307+
for (const [tableName, renames] of Object.entries(columnRenamesByTable)) {
308+
const stmts = newStatements[tableName];
309+
if (stmts) {
310+
const recreate = stmts.find((s) => s.type === 'recreate_table') as JsonRecreateTableStatement | undefined;
311+
if (recreate) {
312+
recreate.columnRenames = renames;
313+
// drop retained rename_column stmts — recreate handles them via columnRenames
314+
newStatements[tableName] = stmts.filter((s) => s.type !== 'alter_table_rename_column');
315+
}
316+
}
317+
}
318+
296319
const combinedStatements = Object.values(newStatements).flat();
297320
const renamedTables = combinedStatements.filter((it) => it.type === 'rename_table');
298321
const renamedColumns = combinedStatements.filter((it) => it.type === 'alter_table_rename_column');
@@ -309,7 +332,17 @@ export const sqliteCombineStatements = (
309332
) => {
310333
// const tablesContext: Record<string, string[]> = {};
311334
const newStatements: Record<string, JsonStatement[]> = {};
335+
const columnRenamesByTable: Record<string, { oldName: string; newName: string }[]> = {};
336+
312337
for (const statement of statements) {
338+
if (statement.type === 'alter_table_rename_column') {
339+
const s = statement as unknown as JsonRenameColumnStatement;
340+
if (!columnRenamesByTable[s.tableName]) {
341+
columnRenamesByTable[s.tableName] = [];
342+
}
343+
columnRenamesByTable[s.tableName].push({ oldName: s.oldColumnName, newName: s.newColumnName });
344+
}
345+
313346
if (
314347
statement.type === 'alter_table_alter_column_set_type'
315348
|| statement.type === 'alter_table_alter_column_set_default'
@@ -436,6 +469,18 @@ export const sqliteCombineStatements = (
436469
}
437470
}
438471

472+
for (const [tableName, renames] of Object.entries(columnRenamesByTable)) {
473+
const stmts = newStatements[tableName];
474+
if (stmts) {
475+
const recreate = stmts.find((s) => s.type === 'recreate_table') as JsonRecreateTableStatement | undefined;
476+
if (recreate) {
477+
recreate.columnRenames = renames;
478+
// drop retained rename_column stmts — recreate handles them via columnRenames
479+
newStatements[tableName] = stmts.filter((s) => s.type !== 'alter_table_rename_column');
480+
}
481+
}
482+
}
483+
439484
const combinedStatements = Object.values(newStatements).flat();
440485

441486
const renamedTables = combinedStatements.filter((it) => it.type === 'rename_table');

‎drizzle-kit/tests/statements-combiner/sqlite-statements-combiner.test.ts‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ test(`renamed column and altered this column type`, async (t) => {
137137
referenceData: [],
138138
uniqueConstraints: [],
139139
checkConstraints: [],
140+
columnRenames: [{ oldName: 'lastName', newName: 'lastName123' }],
140141
},
141142
];
142143
expect(sqliteCombineStatements(statements, json2)).toStrictEqual(
@@ -1209,3 +1210,82 @@ test(`add column and fk`, async (t) => {
12091210
newJsonStatements,
12101211
);
12111212
});
1213+
1214+
test(`renamed table + renamed column + altered column type should absorb rename into recreate`, async (t) => {
1215+
const statements: JsonStatement[] = [
1216+
{
1217+
type: 'rename_table',
1218+
tableNameFrom: 'task',
1219+
tableNameTo: 'todo',
1220+
schema: '',
1221+
} as unknown as JsonStatement,
1222+
{
1223+
type: 'alter_table_rename_column',
1224+
tableName: 'todo',
1225+
oldColumnName: 'is_invisible',
1226+
newColumnName: 'is_visible',
1227+
schema: '',
1228+
},
1229+
{
1230+
type: 'alter_table_alter_column_set_type',
1231+
tableName: 'todo',
1232+
columnName: 'is_visible',
1233+
newDataType: 'integer',
1234+
oldDataType: 'text',
1235+
schema: '',
1236+
columnDefault: undefined,
1237+
columnOnUpdate: undefined,
1238+
columnNotNull: false,
1239+
columnAutoIncrement: false,
1240+
columnPk: false,
1241+
columnIsUnique: false,
1242+
} as unknown as JsonStatement,
1243+
];
1244+
1245+
const json2: SQLiteSchemaSquashed = {
1246+
version: '6',
1247+
dialect: 'sqlite',
1248+
tables: {
1249+
todo: {
1250+
name: 'todo',
1251+
columns: {
1252+
id: {
1253+
name: 'id',
1254+
type: 'integer',
1255+
primaryKey: true,
1256+
notNull: true,
1257+
autoincrement: false,
1258+
},
1259+
is_visible: {
1260+
name: 'is_visible',
1261+
type: 'integer',
1262+
primaryKey: false,
1263+
notNull: false,
1264+
autoincrement: false,
1265+
},
1266+
},
1267+
indexes: {},
1268+
foreignKeys: {},
1269+
compositePrimaryKeys: {},
1270+
uniqueConstraints: {},
1271+
checkConstraints: {},
1272+
},
1273+
},
1274+
enums: {},
1275+
views: {},
1276+
};
1277+
1278+
const result = sqliteCombineStatements(statements, json2);
1279+
1280+
const renameColStmts = result.filter((s) => s.type === 'alter_table_rename_column');
1281+
expect(renameColStmts).toHaveLength(0);
1282+
1283+
const renameTableStmts = result.filter((s) => s.type === 'rename_table');
1284+
expect(renameTableStmts).toHaveLength(1);
1285+
1286+
const recreateStmts = result.filter((s) => s.type === 'recreate_table');
1287+
expect(recreateStmts).toHaveLength(1);
1288+
expect((recreateStmts[0] as any).columnRenames).toStrictEqual([
1289+
{ oldName: 'is_invisible', newName: 'is_visible' },
1290+
]);
1291+
});

0 commit comments

Comments
 (0)