Skip to content

Commit

Permalink
Properly drop old indices on migration (#62)
Browse files Browse the repository at this point in the history 
* Properly drop old indices on migration and add an indexFixer if we got into a bad state

* Bump version
  • Loading branch information
@mariarek @berickson1
mariarek authored and berickson1 committed Sep 27, 2018
1 parent 201a274 commit 1920f3b
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 10 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "nosqlprovider",
"version": "0.6.18",
"version": "0.6.19",
"description": "A cross-browser/platform indexeddb-like client library",
"author": "David de Regt <[email protected]>",
"scripts": {
Expand Down
58 changes: 50 additions & 8 deletions src/SqlProviderBase.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
}

protected _upgradeDb(trans: SqlTransaction, oldVersion: number, wipeAnyway: boolean): SyncTasks.Promise<void> {
// Get a list of all tables and indexes on the tables
// Get a list of all tables, columns and indexes on the tables
return this._getMetadata(trans).then(fullMeta => {
// Get Index metadatas
let indexMetadata: IndexMetadata[] =
Expand All @@ -147,6 +147,7 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
return metaObj;
})
.filter(meta => !!meta && !!meta.storeName) as IndexMetadata[];

return trans.runQuery('SELECT type, name, tbl_name, sql from sqlite_master', [])
.then(rows => {
let tableNames: string[] = [];
Expand All @@ -171,7 +172,6 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
if (row['type'] === 'table') {
tableNames.push(row['name']);
tableSqlStatements[row['name']] = row['sql'];

const nameSplit = row['name'].split('_');
if (nameSplit.length === 1) {
if (!indexNames[row['name']]) {
Expand Down Expand Up @@ -260,7 +260,24 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
tableNames = _.filter(tableNames, name => _.includes(tableNamesNeeded, name));
}

const tableColumns: { [table: string]: string[] } = {};

const getColumnNames = (tableName: string): string[] => {
// Try to get all the column names from SQL create statement
const r = /CREATE\s+TABLE\s+\w+\s+\(([^\)]+)\)/;
const columnPart = tableSqlStatements[tableName].match(r);
if (columnPart) {
return columnPart[1].split(',').map(p => p.trim().split(/\s+/)[0]);
}
return [];
};

_.each(tableNames, table => {
tableColumns[table] = getColumnNames(table);
});

return SyncTasks.all(dropQueries).then(() => {

let tableQueries: SyncTasks.Promise<any>[] = [];

// Go over each store and see what needs changing
Expand Down Expand Up @@ -362,6 +379,10 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
.then(() => indexMaker(storeSchema.indexes));
};

const columnExists = (tableName: string, columnName: string) => {
return _.includes(tableColumns[tableName], columnName);
};

const needsFullMigration = () => {
// Check all the indices in the schema
return _.some(storeSchema.indexes, index => {
Expand All @@ -385,7 +406,7 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
return true;
}
} else {
if (!_.includes(indexNames[storeSchema.name], indexIdentifier)) {
if (!columnExists(storeSchema.name, 'nsp_i_' + index.name)) {
return true;
}
}
Expand Down Expand Up @@ -414,6 +435,13 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
return trans.runQuery('DROP TABLE temp_' + storeSchema.name);
};

// find is there are some columns that should be, but are not indices
// this is to fix a mismatch between the schema in metadata and the actual table state
const someIndicesMissing = _.some(columnBasedIndices, index =>
columnExists(storeSchema.name, 'nsp_i_' + index.name)
&& !_.includes(indexNames[storeSchema.name], getIndexIdentifier(storeSchema, index))
);

// If the table exists, check if we can to determine if a migration is needed
// If a full migration is needed, we have to copy all the data over and re-populate indices
// If a in-place migration is enough, we can just copy the data
Expand All @@ -423,6 +451,14 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
const doSqlInPlaceMigration = tableExists && !doFullMigration && removedColumnIndexMetas.length > 0;
const adddNewColumns = tableExists && !doFullMigration && !doSqlInPlaceMigration
&& newNoBackfillIndices.length > 0;
const recreateIndices = tableExists && !doFullMigration && !doSqlInPlaceMigration && someIndicesMissing;

const indexFixer = () => {
if (recreateIndices) {
return indexMaker(storeSchema.indexes);
}
return SyncTasks.Resolved([]);
};

const indexTableAndMetaDropper = () => {
const indexTablesToDrop = doFullMigration
Expand Down Expand Up @@ -482,11 +518,14 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
};

tableQueries.push(
indexTableAndMetaDropper(),
createTempTable()
.then(tableMaker)
.then(sqlInPlaceMigrator)
.then(dropTempTable)
SyncTasks.all([
indexTableAndMetaDropper(),
dropColumnIndices(),
])
.then(createTempTable)
.then(tableMaker)
.then(sqlInPlaceMigrator)
.then(dropTempTable)
);

}
Expand All @@ -498,7 +537,10 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
indexTableAndMetaDropper(),
columnAdder()
.then(newIndexMaker)
.then(indexFixer)
);
} else if (recreateIndices) {
tableQueries.push(indexFixer());
}

});
Expand Down
97 changes: 96 additions & 1 deletion src/tests/NoSqlProviderTests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2196,6 +2196,10 @@ describe('NoSqlProvider', function () {
{
name: 'ind1',
keyPath: 'content',
},
{
name: 'ind2',
keyPath: 'content',
}
]
}
Expand Down Expand Up @@ -2285,6 +2289,80 @@ describe('NoSqlProvider', function () {
});
});

it('Recreates missing indices', () => {
return openProvider(provName, {
version: 1,
stores: [
{
name: 'test',
primaryKeyPath: 'id',
indexes: [{
name: 'ind1',
keyPath: 'tt',
doNotBackfill: true
}]
}
]
}, true)
.then(prov => {
return prov.put('test', { id: 'abc', tt: 'a' , zz: 'aa'})
.then(() => {
return prov.openTransaction(['test'], true).then(trans => {
// simulate broken index
return (trans as SqlTransaction).runQuery('DROP INDEX test_ind1');
});
})
.then(() => {
return prov.close();
});
})
.then(() => {
return openProvider(provName, {
version: 2,
stores: [
{
name: 'test',
primaryKeyPath: 'id',
indexes: [{
name: 'ind1',
keyPath: 'tt',
doNotBackfill: true
}]
}
]
}, false)
.then(prov => {
const p1 = prov.getOnly('test', 'ind1', 'a').then((items: any[]) => {
assert.equal(items.length, 1);
assert.equal(items[0].id, 'abc');
assert.equal(items[0].tt, 'a');
assert.equal(items[0].zz, 'aa');
});

const p2 = prov.openTransaction(undefined, false).then(trans => {
return (trans as SqlTransaction).runQuery('SELECT type, name, tbl_name, sql from sqlite_master')
.then(fullMeta => {
const oldIndexReallyExists = _.some(fullMeta, (metaObj: any) => {
if (metaObj.type === 'index' && metaObj.tbl_name === 'test'
&& metaObj.name === 'test_ind1') {
return true;
}
return false;
});
if (!oldIndexReallyExists) {
return SyncTasks.Rejected('Unchanged index should still exist!');
}
return SyncTasks.Resolved(undefined);
});
});

return SyncTasks.all([p1, p2]).then(() => {
return prov.close();
});
});
});
});

it('Add remove index in the same upgrade, while preserving other indices', () => {
return openProvider(provName, {
version: 1,
Expand Down Expand Up @@ -2348,7 +2426,24 @@ describe('NoSqlProvider', function () {
return undefined;
});

return SyncTasks.all([p1, p2, p3]).then(() => {
const p4 = prov.openTransaction(undefined, false).then(trans => {
return (trans as SqlTransaction).runQuery('SELECT type, name, tbl_name, sql from sqlite_master')
.then(fullMeta => {
const oldIndexReallyExists = _.some(fullMeta, (metaObj: any) => {
if (metaObj.type === 'index' && metaObj.tbl_name === 'test'
&& metaObj.name === 'test_ind2') {
return true;
}
return false;
});
if (!oldIndexReallyExists) {
return SyncTasks.Rejected('Unchanged index should still exist!');
}
return SyncTasks.Resolved(undefined);
});
});

return SyncTasks.all([p1, p2, p3, p4]).then(() => {
return prov.close();
});
});
Expand Down

0 comments on commit 1920f3b

Please sign in to comment.