Simplify reinsert logic when altering a table in SQLite (#4272)

This commit is contained in:
Nicola Krumschmidt 2021-02-03 19:49:42 +01:00 committed by GitHub
parent 09e9fa4193
commit edf994d7f7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 36 additions and 80 deletions

View File

@ -12,12 +12,10 @@ const omit = require('lodash/omit');
const { nanonum } = require('../../../util/nanoid');
const { COMMA_NO_PAREN_REGEX } = require('../../../constants');
const {
createTempTable,
createNewTable,
copyAllData,
copyData,
dropTempTable,
dropOriginal,
reinsertData,
copyData,
renameTable,
getTableSql,
} = require('./internal/sqlite-ddl-operations');
@ -65,31 +63,16 @@ class SQLite3_DDL {
});
}
async renameTable() {
return this.trx.raw(renameTable(this.tableName(), this.alteredName));
renameTable() {
return this.trx.raw(renameTable(this.alteredName, this.tableName()));
}
dropOriginal() {
return this.trx.raw(dropOriginal(this.tableName()));
}
dropTempTable() {
return this.trx.raw(dropTempTable(this.alteredName));
}
async copyData() {
async copyData(iterator) {
const commands = await copyData(
this.trx,
this.tableName(),
this.alteredName
);
for (const command of commands) {
await this.trx.raw(command);
}
}
async reinsertData(iterator) {
const commands = await reinsertData(
this.trx,
iterator,
this.tableName(),
@ -100,9 +83,9 @@ class SQLite3_DDL {
}
}
createTempTable(createTable) {
createNewTable(sql) {
return this.trx.raw(
createTempTable(createTable, this.tableName(), this.alteredName)
createNewTable(sql, this.tableName(), this.alteredName)
);
}
@ -267,9 +250,7 @@ class SQLite3_DDL {
fromPairs(columns.map((column) => [column, column]))
)
);
return this.reinsertMapped(createTable, newSql, (row) =>
omit(row, ...mappedColumns)
);
return this.alter(newSql, (row) => omit(row, ...mappedColumns));
});
},
{ connection: this.connection }
@ -320,7 +301,7 @@ class SQLite3_DDL {
const newSql = oneLineSql.replace(defs, updatedDefs);
return this.reinsertMapped(createTable, newSql, (row) => {
return this.alter(newSql, (row) => {
return row;
});
},
@ -367,7 +348,7 @@ class SQLite3_DDL {
const newSql = oneLineSql.replace(defs, updatedDefs);
return this.reinsertMapped(createTable, newSql, (row) => {
return this.alter(newSql, (row) => {
return row;
});
},
@ -411,7 +392,7 @@ class SQLite3_DDL {
newColumnDefinitions
);
return this.reinsertMapped(tableInfo, newSQL, (row) => {
return this.alter(newSQL, (row) => {
return row;
});
},
@ -464,7 +445,7 @@ class SQLite3_DDL {
newColumnDefinitions.join(', ')
);
return await this.generateReinsertCommands(tableInfo, newSQL, (row) => {
return await this.generateAlterCommands(newSQL, (row) => {
return row;
});
},
@ -479,29 +460,22 @@ class SQLite3_DDL {
* It'll be helpful to refactor this file heavily to combine/optimize some of these calls
*/
reinsertMapped(createTable, newSql, mapRow) {
alter(newSql, mapRow) {
return Promise.resolve()
.then(() => this.createTempTable(createTable))
.then(() => this.copyData())
.then(() => this.createNewTable(newSql))
.then(() => this.copyData(mapRow))
.then(() => this.dropOriginal())
.then(() => this.trx.raw(newSql))
.then(() => this.reinsertData(mapRow))
.then(() => this.dropTempTable());
.then(() => this.renameTable());
}
async generateReinsertCommands(createTable, newSql, mapRow) {
async generateAlterCommands(newSql, mapRow) {
const result = [];
result.push(
createTempTable(createTable, this.tableName(), this.alteredName)
);
result.push(createNewTable(newSql, this.tableName(), this.alteredName));
result.push(copyAllData(this.tableName(), this.alteredName));
result.push(dropOriginal(this.tableName()));
result.push(renameTable(this.alteredName, this.tableName()));
result.push(newSql);
result.push(copyAllData(this.alteredName, this.tableName()));
result.push(dropTempTable(this.alteredName));
return result;
}
}

View File

@ -13,20 +13,14 @@ function insertChunked(trx, chunkSize, target, iterator, existingData) {
return result;
}
function createTempTable(createTable, tablename, alteredName) {
return createTable.sql.replace(tablename, alteredName);
function createNewTable(sql, tablename, alteredName) {
return sql.replace(tablename, alteredName);
}
// ToDo To be removed
async function copyData(trx, tableName, alteredName) {
async function copyData(trx, iterator, tableName, alteredName) {
const existingData = await trx.raw(`SELECT * FROM "${tableName}"`);
return insertChunked(trx, 20, alteredName, identity, existingData);
}
// ToDo To be removed
async function reinsertData(trx, iterator, tableName, alteredName) {
const existingData = await trx.raw(`SELECT * FROM "${alteredName}"`);
return insertChunked(trx, 20, tableName, iterator, existingData);
return insertChunked(trx, 20, alteredName, iterator, existingData);
}
function copyAllData(sourceTable, targetTable) {
@ -37,10 +31,6 @@ function dropOriginal(tableName) {
return `DROP TABLE "${tableName}"`;
}
function dropTempTable(alteredName) {
return `DROP TABLE "${alteredName}"`;
}
function renameTable(tableName, alteredName) {
return `ALTER TABLE "${tableName}" RENAME TO "${alteredName}"`;
}
@ -51,11 +41,9 @@ function getTableSql(tableName) {
module.exports = {
copyAllData,
copyData,
createTempTable,
createNewTable,
dropOriginal,
dropTempTable,
reinsertData,
copyData,
renameTable,
getTableSql,
};

View File

@ -1366,7 +1366,7 @@ module.exports = (knex) => {
expect(await hasCol('i0')).to.equal(false);
// Constraint i0 should be unaffected:
expect(await getCreateTableExpr()).to.equal(
"CREATE TABLE TEST('i1' integer, [i2] integer, `i3` integer, i4 " +
'CREATE TABLE "TEST"(\'i1\' integer, [i2] integer, `i3` integer, i4 ' +
'integer, I5 integer, unique(i4, i5), constraint i0 primary ' +
'key([i3], "i4"), unique([i2]), foreign key (i1) references bar ' +
'("i3") )'
@ -1375,24 +1375,24 @@ module.exports = (knex) => {
expect(await hasCol('i1')).to.equal(false);
// Foreign key on i1 should also be dropped:
expect(await getCreateTableExpr()).to.equal(
'CREATE TABLE TEST([i2] integer, `i3` integer, i4 integer, I5 integer, ' +
'CREATE TABLE "TEST"([i2] integer, `i3` integer, i4 integer, I5 integer, ' +
'unique(i4, i5), constraint i0 primary key([i3], "i4"), unique([i2]))'
);
await dropCol('i2');
expect(await hasCol('i2')).to.equal(false);
expect(await getCreateTableExpr()).to.equal(
'CREATE TABLE TEST(`i3` integer, i4 integer, I5 integer, ' +
'CREATE TABLE "TEST"(`i3` integer, i4 integer, I5 integer, ' +
'unique(i4, i5), constraint i0 primary key([i3], "i4"))'
);
await dropCol('i3');
expect(await hasCol('i3')).to.equal(false);
expect(await getCreateTableExpr()).to.equal(
'CREATE TABLE TEST(i4 integer, I5 integer, unique(i4, i5))'
'CREATE TABLE "TEST"(i4 integer, I5 integer, unique(i4, i5))'
);
await dropCol('i4');
expect(await hasCol('i4')).to.equal(false);
expect(await getCreateTableExpr()).to.equal(
'CREATE TABLE TEST(I5 integer)'
'CREATE TABLE "TEST"(I5 integer)'
);
let lastColDeletionError;
await knex.schema

View File

@ -66,12 +66,10 @@ describe('Schema', () => {
if (isSQLite(knex)) {
expect(queries).to.eql([
'CREATE TABLE `_knex_temp_alter111` (`id` integer not null primary key autoincrement, `fkey_two` integer not null, `fkey_three` integer not null)',
'CREATE TABLE `_knex_temp_alter111` (`id` integer not null primary key autoincrement, `fkey_two` integer not null, `fkey_three` integer not null, CONSTRAINT fk_fkey_threeee FOREIGN KEY (`fkey_three`) REFERENCES `foreign_keys_table_three` (`id`))',
'INSERT INTO _knex_temp_alter111 SELECT * FROM foreign_keys_table_one;',
'DROP TABLE "foreign_keys_table_one"',
'CREATE TABLE `foreign_keys_table_one` (`id` integer not null primary key autoincrement, `fkey_two` integer not null, `fkey_three` integer not null, CONSTRAINT fk_fkey_threeee FOREIGN KEY (`fkey_three`) REFERENCES `foreign_keys_table_three` (`id`))',
'INSERT INTO foreign_keys_table_one SELECT * FROM _knex_temp_alter111;',
'DROP TABLE "_knex_temp_alter111"',
'ALTER TABLE "_knex_temp_alter111" RENAME TO "foreign_keys_table_one"',
]);
}
@ -105,12 +103,10 @@ describe('Schema', () => {
const queries = await builder.generateDdlCommands();
expect(queries).to.eql([
'CREATE TABLE `_knex_temp_alter111` (`id` integer not null primary key autoincrement, `fkey_two` integer not null, `fkey_three` integer not null)',
'CREATE TABLE `_knex_temp_alter111` (`id` integer not null primary key autoincrement, `fkey_two` integer not null, `fkey_three` integer not null, CONSTRAINT fk_fkey_threeee FOREIGN KEY (`fkey_three`) REFERENCES `foreign_keys_table_three` (`id`) ON DELETE CASCADE)',
'INSERT INTO _knex_temp_alter111 SELECT * FROM foreign_keys_table_one;',
'DROP TABLE "foreign_keys_table_one"',
'CREATE TABLE `foreign_keys_table_one` (`id` integer not null primary key autoincrement, `fkey_two` integer not null, `fkey_three` integer not null, CONSTRAINT fk_fkey_threeee FOREIGN KEY (`fkey_three`) REFERENCES `foreign_keys_table_three` (`id`) ON DELETE CASCADE)',
'INSERT INTO foreign_keys_table_one SELECT * FROM _knex_temp_alter111;',
'DROP TABLE "_knex_temp_alter111"',
'ALTER TABLE "_knex_temp_alter111" RENAME TO "foreign_keys_table_one"',
]);
});
@ -133,12 +129,10 @@ describe('Schema', () => {
const queries = await builder.generateDdlCommands();
expect(queries).to.eql([
'CREATE TABLE `_knex_temp_alter111` (`id` integer not null primary key autoincrement, `fkey_two` integer not null, `fkey_three` integer not null)',
'CREATE TABLE `_knex_temp_alter111` (`id` integer not null primary key autoincrement, `fkey_two` integer not null, `fkey_three` integer not null, CONSTRAINT fk_fkey_threeee FOREIGN KEY (`fkey_three`) REFERENCES `foreign_keys_table_three` (`id`) ON UPDATE CASCADE)',
'INSERT INTO _knex_temp_alter111 SELECT * FROM foreign_keys_table_one;',
'DROP TABLE "foreign_keys_table_one"',
'CREATE TABLE `foreign_keys_table_one` (`id` integer not null primary key autoincrement, `fkey_two` integer not null, `fkey_three` integer not null, CONSTRAINT fk_fkey_threeee FOREIGN KEY (`fkey_three`) REFERENCES `foreign_keys_table_three` (`id`) ON UPDATE CASCADE)',
'INSERT INTO foreign_keys_table_one SELECT * FROM _knex_temp_alter111;',
'DROP TABLE "_knex_temp_alter111"',
'ALTER TABLE "_knex_temp_alter111" RENAME TO "foreign_keys_table_one"',
]);
});