Skip to content

Commit 74efd8e

Browse files
mariarekberickson1
mariarek
authored and
berickson1
committed
Remove index without doing a full migration (#58)
* Remove index without doing a full migration
1 parent c348ccc commit 74efd8e

4 files changed

Lines changed: 251 additions & 75 deletions

File tree

‎package.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
},
2121
"devDependencies": {
2222
"@types/mocha": "2.2.38",
23+
"@types/sinon": "^5.0.2",
2324
"awesome-typescript-loader": "^5.2.0",
2425
"mocha": "^5.2.0",
26+
"sinon": "^6.3.1",
2527
"sqlite3": "^4.0.2",
2628
"tslint": "^5.11.0",
2729
"tslint-microsoft-contrib": "^5.0.3",

‎src/SqlProviderBase.ts‎

Lines changed: 129 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,21 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
198198
}
199199
});
200200

201+
const deleteFromMeta = (metasToDelete: IndexMetadata[]) => {
202+
if (metasToDelete.length === 0) {
203+
return SyncTasks.Resolved([]);
204+
}
205+
206+
// Generate as many '?' as there are params
207+
let placeholder = '?';
208+
for (let i = 1; i < metasToDelete.length; i++) {
209+
placeholder += ',?';
210+
}
211+
212+
return trans.runQuery('DELETE FROM metadata WHERE name IN (' + placeholder + ')',
213+
_.map(metasToDelete, meta => meta.key));
214+
};
215+
201216
// Check each table!
202217
let dropQueries: SyncTasks.Promise<any>[] = [];
203218
if (wipeAnyway || (this._schema!!!.lastUsableVersion && oldVersion < this._schema!!!.lastUsableVersion!!!)) {
@@ -209,14 +224,8 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
209224
dropQueries = _.map(tableNames, name => trans.runQuery('DROP TABLE ' + name));
210225

211226
if (indexMetadata.length > 0) {
212-
// Generate as many '?' as there are params
213-
let placeholder = '?';
214-
for (let i = 1; i < indexMetadata.length; i++) {
215-
placeholder += ',?';
216-
}
217227
// Drop all existing metadata
218-
dropQueries.push(trans.runQuery('DELETE FROM metadata WHERE name IN (' + placeholder + ')',
219-
_.map(indexMetadata, meta => meta.key)));
228+
dropQueries.push(deleteFromMeta(indexMetadata));
220229
indexMetadata = [];
221230
}
222231
tableNames = [];
@@ -239,13 +248,7 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
239248

240249
// Clean up metas
241250
if (metasToDelete.length > 0) {
242-
// Generate as many '?' as there are params
243-
let placeholder = '?';
244-
for (let i = 1; i < metasToDelete.length; i++) {
245-
placeholder += ',?';
246-
}
247-
transList.push(trans.runQuery('DELETE FROM metadata where name in (' + placeholder + ')',
248-
_.map(metasToDelete, meta => meta.key)));
251+
transList.push(deleteFromMeta(metasToDelete));
249252
indexMetadata = _.filter(indexMetadata, meta => !_.includes(metaKeysToDelete, meta.key));
250253
}
251254
return transList;
@@ -278,20 +281,23 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
278281
if (NoSqlProviderUtils.isCompoundKeyPath(index.keyPath)) {
279282
return SyncTasks.Rejected('Can\'t use multiEntry and compound keys');
280283
} else {
281-
return trans.runQuery('CREATE TABLE ' + indexIdentifier + ' (nsp_key TEXT, nsp_refpk TEXT' +
284+
return trans.runQuery('CREATE TABLE IF NOT EXISTS ' + indexIdentifier +
285+
' (nsp_key TEXT, nsp_refpk TEXT' +
282286
(index.includeDataInIndex ? ', nsp_data TEXT' : '') + ')').then(() => {
283-
return trans.runQuery('CREATE ' + (index.unique ? 'UNIQUE ' : '') + 'INDEX ' +
287+
return trans.runQuery('CREATE ' + (index.unique ? 'UNIQUE ' : '') +
288+
'INDEX IF NOT EXISTS ' +
284289
indexIdentifier + '_pi ON ' + indexIdentifier + ' (nsp_key, nsp_refpk' +
285290
(index.includeDataInIndex ? ', nsp_data' : '') + ')');
286291
});
287292
}
288293
} else if (index.fullText && this._supportsFTS3) {
289294
// If FTS3 isn't supported, we'll make a normal column and use LIKE to seek over it, so the
290295
// fallback below works fine.
291-
return trans.runQuery('CREATE VIRTUAL TABLE ' + indexIdentifier +
296+
return trans.runQuery('CREATE VIRTUAL TABLE IF NOT EXISTS ' + indexIdentifier +
292297
' USING FTS3(nsp_key TEXT, nsp_refpk TEXT)');
293298
} else {
294-
return trans.runQuery('CREATE ' + (index.unique ? 'UNIQUE ' : '') + 'INDEX ' + indexIdentifier +
299+
return trans.runQuery('CREATE ' + (index.unique ? 'UNIQUE ' : '') +
300+
'INDEX IF NOT EXISTS ' + indexIdentifier +
295301
' ON ' + storeSchema.name + ' (nsp_i_' + index.name +
296302
(index.includeDataInIndex ? ', nsp_data' : '') + ')');
297303
}
@@ -306,23 +312,36 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
306312
fieldList.push('nsp_pk TEXT PRIMARY KEY');
307313
fieldList.push('nsp_data TEXT');
308314

309-
const columnBasedIndexes = _.filter(storeSchema.indexes, index =>
315+
const columnBasedIndices = _.filter(storeSchema.indexes, index =>
310316
!indexUsesSeparateTable(index, this._supportsFTS3));
311-
312-
const indexColumns = _.map(columnBasedIndexes, index => 'nsp_i_' + index.name + ' TEXT');
317+
318+
const indexColumns = _.map(columnBasedIndices, index => 'nsp_i_' + index.name + ' TEXT');
313319
fieldList = fieldList.concat(indexColumns);
314320
const tableMakerSql = 'CREATE TABLE ' + storeSchema.name + ' (' + fieldList.join(', ') + ')';
315321

316322
const currentIndexMetas = _.filter(indexMetadata, meta => meta.storeName === storeSchema.name);
323+
324+
const indexIdentifierDictionary = _.keyBy(storeSchema.indexes, index => getIndexIdentifier(storeSchema, index));
325+
const indexMetaDictionary = _.keyBy(currentIndexMetas, meta => meta.key);
326+
327+
// find indices in the schema that did not exist before
328+
const newIndices = _.filter(storeSchema.indexes, index =>
329+
!indexMetaDictionary[getIndexIdentifier(storeSchema, index)]);
330+
331+
// find indices in the meta that do not exist in the new schema
332+
const allRemovedIndexMetas = _.filter(currentIndexMetas, meta =>
333+
!indexIdentifierDictionary[meta.key]);
334+
335+
const [removedTableIndexMetas, removedColumnIndexMetas] = _.partition(allRemovedIndexMetas,
336+
meta => indexUsesSeparateTable(meta.index, this._supportsFTS3));
317337

318338
// find new indices which don't require backfill
319-
const newNoBackfillIndices = _.filter(storeSchema.indexes, index => {
320-
const indexIdentifier = getIndexIdentifier(storeSchema, index);
321-
return !!index.doNotBackfill && !_.some(currentIndexMetas, meta => meta.key === indexIdentifier);
339+
const newNoBackfillIndices = _.filter(newIndices, index => {
340+
return !!index.doNotBackfill;
322341
});
323342

324343
// columns requiring no backfill could be simply added to the table
325-
const newIndexColumnsNoBackfill = _.intersection(newNoBackfillIndices, columnBasedIndexes);
344+
const newIndexColumnsNoBackfill = _.intersection(newNoBackfillIndices, columnBasedIndices);
326345

327346
const columnAdder = () => {
328347
const addQueries = _.map(newIndexColumnsNoBackfill, index =>
@@ -338,27 +357,16 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
338357
.then(() => indexMaker(storeSchema.indexes));
339358
};
340359

341-
const needsDataMigration = () => {
342-
// If some indices have been removed - migration is needed
343-
const areIndicesRemoved = _.some(currentIndexMetas, meta =>
344-
!_.some(storeSchema.indexes, newIndex => getIndexIdentifier(storeSchema, newIndex) === meta.key));
345-
if (areIndicesRemoved) {
346-
return true;
347-
}
348-
360+
const needsFullMigration = () => {
349361
// Check all the indices in the schema
350362
return _.some(storeSchema.indexes, index => {
351363
const indexIdentifier = getIndexIdentifier(storeSchema, index);
352-
const indexMeta = _.find(indexMetadata, meta => meta.key === indexIdentifier);
364+
const indexMeta = indexMetaDictionary[indexIdentifier];
353365

354366
// if there's a new index that doesn't require backfill, continue
355-
if (!indexMeta && index.doNotBackfill) {
356-
return false;
357-
}
358-
359-
// If we have a new index that requires backfill - we need to migrate
367+
// If there's a new index that requires backfill - we need to migrate
360368
if (!indexMeta) {
361-
return true;
369+
return !index.doNotBackfill;
362370
}
363371

364372
// If the index schemas don't match - we need to migrate
@@ -381,44 +389,99 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
381389
});
382390
};
383391

384-
// If the table exists, check if we can view the sql statement used to create this table. Use it to determine
385-
// if a migration is needed, otherwise just make a copy and fully migrate the data over.
392+
const dropColumnIndices = () => {
393+
return _.map(indexNames[storeSchema.name], indexName =>
394+
trans.runQuery('DROP INDEX ' + indexName));
395+
};
396+
397+
const dropIndexTables = (tableNames: string[]) => {
398+
return _.map(tableNames, tableName =>
399+
trans.runQuery('DROP TABLE IF EXISTS ' + storeSchema.name + '_' + tableName)
400+
);
401+
};
402+
403+
const createTempTable = () => {
404+
// Then rename the table to a temp_[name] table so we can migrate the data out of it
405+
return trans.runQuery('ALTER TABLE ' + storeSchema.name + ' RENAME TO temp_' + storeSchema.name);
406+
};
407+
408+
const dropTempTable = () => {
409+
return trans.runQuery('DROP TABLE temp_' + storeSchema.name);
410+
};
411+
412+
// If the table exists, check if we can to determine if a migration is needed
413+
// If a full migration is needed, we have to copy all the data over and re-populate indices
414+
// If a in-place migration is enough, we can just copy the data
415+
// If no migration is needed, we can just add new column for new indices
386416
const tableExists = _.includes(tableNames, storeSchema.name);
387-
const tableRequiresMigration = tableExists && needsDataMigration();
388-
389-
if (tableExists && tableRequiresMigration) {
390-
// Nuke old indexes on the original table (since they don't change names and we don't need them anymore).
391-
// Also new old multientry/FTS tables (if they still exist after the purge above.)
392-
let indexDroppers: SyncTasks.Promise<any[]>[] = _.map(indexNames[storeSchema.name], indexName =>
393-
trans.runQuery('DROP INDEX ' + indexName)).concat(_.map(indexTables[storeSchema.name], tableName =>
394-
trans.runQuery('DROP TABLE IF EXISTS ' + storeSchema.name + '_' + tableName)));
395-
396-
let nukeIndexesAndRename = SyncTasks.all(indexDroppers).then(() => {
397-
// Then rename the table to a temp_[name] table so we can migrate the data out of it
398-
return trans.runQuery('ALTER TABLE ' + storeSchema.name + ' RENAME TO temp_' + storeSchema.name);
399-
});
417+
const doFullMigration = tableExists && needsFullMigration();
418+
const doSqlInPlaceMigration = tableExists && !doFullMigration && removedColumnIndexMetas.length > 0;
419+
const adddNewColumns = tableExists && !doFullMigration && !doSqlInPlaceMigration
420+
&& newNoBackfillIndices.length > 0;
421+
422+
const indexTableAndMetaDropper = () => {
423+
const indexTablesToDrop = doFullMigration
424+
? indexTables[storeSchema.name] : removedTableIndexMetas.map(meta => meta.key);
425+
return SyncTasks.all([deleteFromMeta(allRemovedIndexMetas), ...dropIndexTables(indexTablesToDrop)]);
426+
};
400427

428+
if (!tableExists) {
429+
// Table doesn't exist -- just go ahead and create it without the migration path
430+
tableQueries.push(tableMaker());
431+
}
432+
433+
if (doFullMigration) {
401434
// Migrate the data over using our existing put functions
402435
// (since it will do the right things with the indexes)
403436
// and delete the temp table.
404-
let migrator = () => {
437+
const jsMigrator = () => {
405438
let store = trans.getStore(storeSchema.name);
406439
return trans.internal_getResultsFromQuery('SELECT nsp_data FROM temp_' + storeSchema.name)
407440
.then(objs => {
408-
return store.put(objs).then(() => {
409-
return trans.runQuery('DROP TABLE temp_' + storeSchema.name);
410-
});
441+
return store.put(objs);
411442
});
412443
};
413444

414-
tableQueries.push(nukeIndexesAndRename.then(tableMaker).then(migrator));
415-
} else if (tableExists && newNoBackfillIndices.length > 0) {
416-
// we can add new indices without migrating
417-
tableQueries.push(columnAdder().then(() => indexMaker(newNoBackfillIndices)));
418-
} else if (!tableExists) {
419-
// Table doesn't exist -- just go ahead and create it without the migration path
420-
tableQueries.push(tableMaker());
445+
tableQueries.push(
446+
SyncTasks.all([
447+
indexTableAndMetaDropper(),
448+
dropColumnIndices(),
449+
])
450+
.then(createTempTable)
451+
.then(tableMaker)
452+
.then(jsMigrator)
453+
.then(dropTempTable)
454+
);
455+
}
456+
457+
if (doSqlInPlaceMigration) {
458+
const sqlInPlaceMigrator = () => {
459+
return trans.runQuery('INSERT INTO ' + storeSchema.name +
460+
' SELECT ' + ['nsp_pk', 'nsp_data', ...indexColumns].join(', ') +
461+
' FROM temp_' + storeSchema.name);
462+
463+
};
464+
465+
tableQueries.push(
466+
indexTableAndMetaDropper(),
467+
createTempTable()
468+
.then(tableMaker)
469+
.then(sqlInPlaceMigrator)
470+
.then(dropTempTable)
471+
);
472+
421473
}
474+
475+
if (adddNewColumns) {
476+
const newIndexMaker = () => indexMaker(newNoBackfillIndices);
477+
478+
tableQueries.push(
479+
indexTableAndMetaDropper(),
480+
columnAdder()
481+
.then(newIndexMaker)
482+
);
483+
}
484+
422485
});
423486

424487
return SyncTasks.all(tableQueries);

0 commit comments

Comments
 (0)