Skip to content

Commit c348ccc

Browse files
mariarekberickson1
mariarek
authored and
berickson1
committed
Add doNotBackfill to index schema (#57)
* Add doNotBackfill to index schema * PR comments + one more UT * Add throw when target is null
1 parent dc95ad5 commit c348ccc

6 files changed

Lines changed: 347 additions & 26 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,4 @@ npm run webtest
3131
## Testing
3232
1. Compile tests
3333
1. Open test.html in browser
34+
1. You can add `?grep=foo` to the URL to run only tests containing 'foo' in the name

‎src/IndexedDbProvider.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ export class IndexedDbProvider extends NoSqlProvider.DbProvider {
121121
const target = <IDBOpenDBRequest>(event.currentTarget || event.target);
122122
const trans = target.transaction;
123123

124+
if (!trans) {
125+
throw new Error('onupgradeneeded: target is null!');
126+
}
127+
124128
if (schema.lastUsableVersion && event.oldVersion < schema.lastUsableVersion) {
125129
// Clear all stores if it's past the usable version
126130
console.log('Old version detected (' + event.oldVersion + '), clearing all data');
@@ -139,8 +143,8 @@ export class IndexedDbProvider extends NoSqlProvider.DbProvider {
139143
// Create all stores
140144
_.each(schema.stores, storeSchema => {
141145
let store: IDBObjectStore;
142-
let migrateData = false;
143-
if (!_.includes(db.objectStoreNames, storeSchema.name)) {
146+
const storeExistedBefore = _.includes(db.objectStoreNames, storeSchema.name);
147+
if (!storeExistedBefore) { // store doesn't exist yet
144148
let primaryKeyPath = storeSchema.primaryKeyPath;
145149
if (this._fakeComplicatedKeys && NoSqlProviderUtils.isCompoundKeyPath(primaryKeyPath)) {
146150
// Going to have to hack the compound primary key index into a column, so here it is.
@@ -149,9 +153,8 @@ export class IndexedDbProvider extends NoSqlProvider.DbProvider {
149153

150154
// Any is to fix a lib.d.ts issue in TS 2.0.3 - it doesn't realize that keypaths can be compound for some reason...
151155
store = db.createObjectStore(storeSchema.name, { keyPath: primaryKeyPath } as any);
152-
} else {
156+
} else { // store exists, might need to update indexes and migrate the data
153157
store = trans.objectStore(storeSchema.name);
154-
migrateData = true;
155158

156159
// Check for any indexes no longer in the schema or have been changed
157160
_.each(store.indexNames, indexName => {
@@ -185,8 +188,10 @@ export class IndexedDbProvider extends NoSqlProvider.DbProvider {
185188
});
186189
}
187190

188-
// Check any indexes in the schema that need to be created
191+
// IndexedDB deals well with adding new indexes on the fly, so we don't need to force migrate,
192+
// unless adding multiEntry or fullText index
189193
let needsMigrate = false;
194+
// Check any indexes in the schema that need to be created
190195
_.each(storeSchema.indexes, indexSchema => {
191196
if (!_.includes(store.indexNames, indexSchema.name)) {
192197
const keyPath = indexSchema.keyPath;
@@ -201,7 +206,7 @@ export class IndexedDbProvider extends NoSqlProvider.DbProvider {
201206
indexStore.createIndex('key', 'key');
202207
indexStore.createIndex('refkey', 'refkey');
203208

204-
if (migrateData) {
209+
if (storeExistedBefore && !indexSchema.doNotBackfill) {
205210
needsMigrate = true;
206211
}
207212
}
@@ -221,7 +226,7 @@ export class IndexedDbProvider extends NoSqlProvider.DbProvider {
221226
multiEntry: true
222227
});
223228

224-
if (migrateData) {
229+
if (storeExistedBefore && !indexSchema.doNotBackfill) {
225230
needsMigrate = true;
226231
}
227232
} else {

‎src/NoSqlProvider.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ export interface IndexSchema {
2727
multiEntry?: boolean;
2828
fullText?: boolean;
2929
includeDataInIndex?: boolean;
30+
doNotBackfill?: boolean;
3031
}
3132

3233
// Schema type describing a data store. Must give a keypath for the primary key for the store. Further indexes are optional.

‎src/SqlProviderBase.ts‎

Lines changed: 76 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,17 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
208208

209209
dropQueries = _.map(tableNames, name => trans.runQuery('DROP TABLE ' + name));
210210

211-
// Drop all existing metadata
212-
const metaToDropArg = _.map(indexMetadata, meta => meta.key).join(', ');
213-
dropQueries.push(trans.runQuery('DELETE FROM metadata where name in (?) ', [metaToDropArg]));
214-
indexMetadata = [];
211+
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+
}
217+
// Drop all existing metadata
218+
dropQueries.push(trans.runQuery('DELETE FROM metadata WHERE name IN (' + placeholder + ')',
219+
_.map(indexMetadata, meta => meta.key)));
220+
indexMetadata = [];
221+
}
215222
tableNames = [];
216223
} else {
217224
// Just delete tables we don't care about anymore. Preserve multi-entry tables, they may not be changed
@@ -232,7 +239,13 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
232239

233240
// Clean up metas
234241
if (metasToDelete.length > 0) {
235-
transList.push(trans.runQuery('DELETE FROM metadata where name in (?)', [metasToDelete.join(', ')]));
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)));
236249
indexMetadata = _.filter(indexMetadata, meta => !_.includes(metaKeysToDelete, meta.key));
237250
}
238251
return transList;
@@ -246,9 +259,11 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
246259

247260
// Go over each store and see what needs changing
248261
_.each(this._schema!!!.stores, storeSchema => {
249-
const indexMaker = () => {
262+
263+
// creates indexes for provided schemas
264+
const indexMaker = (indexes: NoSqlProvider.IndexSchema[] = []) => {
250265
let metaQueries: SyncTasks.Promise<any>[] = [];
251-
const indexQueries = _.map(storeSchema.indexes, index => {
266+
const indexQueries = _.map(indexes, index => {
252267
const indexIdentifier = getIndexIdentifier(storeSchema, index);
253268

254269
// Store meta for the index
@@ -293,31 +308,64 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
293308

294309
const columnBasedIndexes = _.filter(storeSchema.indexes, index =>
295310
!indexUsesSeparateTable(index, this._supportsFTS3));
311+
296312
const indexColumns = _.map(columnBasedIndexes, index => 'nsp_i_' + index.name + ' TEXT');
297313
fieldList = fieldList.concat(indexColumns);
298314
const tableMakerSql = 'CREATE TABLE ' + storeSchema.name + ' (' + fieldList.join(', ') + ')';
315+
316+
const currentIndexMetas = _.filter(indexMetadata, meta => meta.storeName === storeSchema.name);
317+
318+
// 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);
322+
});
323+
324+
// columns requiring no backfill could be simply added to the table
325+
const newIndexColumnsNoBackfill = _.intersection(newNoBackfillIndices, columnBasedIndexes);
326+
327+
const columnAdder = () => {
328+
const addQueries = _.map(newIndexColumnsNoBackfill, index =>
329+
trans.runQuery('ALTER TABLE ' + storeSchema.name + ' ADD COLUMN ' + 'nsp_i_' + index.name + ' TEXT')
330+
);
331+
332+
return SyncTasks.all(addQueries);
333+
};
299334

300335
const tableMaker = () => {
301336
// Create the table
302337
return trans.runQuery(tableMakerSql)
303-
.then(indexMaker);
338+
.then(() => indexMaker(storeSchema.indexes));
304339
};
305340

306-
const needsMigration = () => {
307-
// Check if sql used to create the base table has changed
308-
if (tableSqlStatements[storeSchema.name] !== tableMakerSql) {
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) {
309346
return true;
310347
}
311348

312-
// Check if indicies are missing
349+
// Check all the indices in the schema
313350
return _.some(storeSchema.indexes, index => {
314-
// Check if key paths agree
315351
const indexIdentifier = getIndexIdentifier(storeSchema, index);
316352
const indexMeta = _.find(indexMetadata, meta => meta.key === indexIdentifier);
317-
if (!indexMeta || !_.isEqual(indexMeta.index, index)) {
353+
354+
// 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
360+
if (!indexMeta) {
318361
return true;
319362
}
320363

364+
// If the index schemas don't match - we need to migrate
365+
if (!_.isEqual(indexMeta.index, index)) {
366+
return true;
367+
}
368+
321369
// Check that indicies actually exist in the right place
322370
if (indexUsesSeparateTable(index, this._supportsFTS3)) {
323371
if (!_.includes(tableNames, indexIdentifier)) {
@@ -336,7 +384,7 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
336384
// If the table exists, check if we can view the sql statement used to create this table. Use it to determine
337385
// if a migration is needed, otherwise just make a copy and fully migrate the data over.
338386
const tableExists = _.includes(tableNames, storeSchema.name);
339-
const tableRequiresMigration = tableExists && needsMigration();
387+
const tableRequiresMigration = tableExists && needsDataMigration();
340388

341389
if (tableExists && tableRequiresMigration) {
342390
// Nuke old indexes on the original table (since they don't change names and we don't need them anymore).
@@ -364,6 +412,9 @@ export abstract class SqlProviderBase extends NoSqlProvider.DbProvider {
364412
};
365413

366414
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)));
367418
} else if (!tableExists) {
368419
// Table doesn't exist -- just go ahead and create it without the migration path
369420
tableQueries.push(tableMaker());
@@ -803,18 +854,25 @@ class SqlStore implements NoSqlProvider.DbStore {
803854
const queries = _.map(arrayOfParams, params => {
804855
let queries: SyncTasks.Promise<void>[] = [];
805856

857+
if (params.length === 0) {
858+
return undefined;
859+
}
860+
806861
// Generate as many '?' as there are params
807-
let sqlPart = Array.apply(null, new Array(params.length)).map(() => '?').join(',');
862+
let placeholder = '?';
863+
for (let i = 1; i < params.length; i++) {
864+
placeholder += ',?';
865+
}
808866

809867
_.each(this._schema.indexes, index => {
810868
if (indexUsesSeparateTable(index, this._supportsFTS3)) {
811869
queries.push(this._trans.internal_nonQuery('DELETE FROM ' + this._schema.name + '_' + index.name +
812-
' WHERE nsp_refpk IN (' + sqlPart + ')', params));
870+
' WHERE nsp_refpk IN (' + placeholder + ')', params));
813871
}
814872
});
815873

816874
queries.push(this._trans.internal_nonQuery('DELETE FROM ' + this._schema.name +
817-
' WHERE nsp_pk IN (' + sqlPart + ')', params));
875+
' WHERE nsp_pk IN (' + placeholder + ')', params));
818876

819877
return SyncTasks.all(queries).then(_.noop);
820878
});

0 commit comments

Comments
 (0)