From d007a675afc258bba3289970f94ee3ffb556739b Mon Sep 17 00:00:00 2001 From: David Fahlander Date: Sat, 6 Apr 2024 03:06:44 +0200 Subject: [PATCH] Bugfix: liveQueries of compound index involving auto-incremented primary key as part (#1948) * Repro of #1946 * Resolve #1946 * Verifying also that order is correct --- src/live-query/observability-middleware.ts | 21 +++++++++- test/tests-live-query.js | 48 ++++++++++++++++++++-- 2 files changed, 65 insertions(+), 4 deletions(-) diff --git a/src/live-query/observability-middleware.ts b/src/live-query/observability-middleware.ts index 571687034..838bff38c 100644 --- a/src/live-query/observability-middleware.ts +++ b/src/live-query/observability-middleware.ts @@ -45,8 +45,11 @@ export const observabilityMiddleware: Middleware = { table: (tableName) => { const table = core.table(tableName); const { schema } = table; - const { primaryKey } = schema; + const { primaryKey, indexes } = schema; const { extractKey, outbound } = primaryKey; + const indexesWithAutoIncPK = primaryKey.autoIncrement && indexes.filter( + (index) => index.compound && (index.keyPath as string[]).includes(primaryKey.keyPath as string) + ); const tableClone: DBCoreTable = { ...table, mutate: (req) => { @@ -117,6 +120,22 @@ export const observabilityMiddleware: Middleware = { // Less than 50 requests (keys truthy) (otherwise we've added full range anyway) // autoincrement means we might not have got all keys until now pkRangeSet.addKeys(res.results); + if (indexesWithAutoIncPK) { + // Dexie Issue 1946: + // If an auto-incremented primary key is part of a compound index, + // we need to compute the resulting value of that index after inserting + // the rows. + indexesWithAutoIncPK.forEach(idx => { + // Extract values of this compound index where primary key is not yet set: + const idxVals = req.values.map(v => idx.extractKey(v)); + // Find the position of the primary key in the index: + const pkPos = (idx.keyPath as string[]).findIndex(prop => prop === primaryKey.keyPath); + // Update idxVals with the resulting primary keys to complete the index value: + res.results!.forEach(pk => idxVals[pkPos] = pk); + // Add the updated index to the rangeset: + getRangeSet(idx.name).addKeys(idxVals); + }); + } } trans.mutatedParts = extendObservabilitySet ( trans.mutatedParts || {}, diff --git a/test/tests-live-query.js b/test/tests-live-query.js index 72f6d6b29..46d88bb36 100644 --- a/test/tests-live-query.js +++ b/test/tests-live-query.js @@ -13,7 +13,8 @@ db.version(2).stores({ foo: "++id", outbound: "++,name", friends: "++id, name, age", - multiEntry: "id, *tags" + multiEntry: "id, *tags", + issue1946: "++id, [name+age], [name+age+id]" }); db.on('populate', ()=> { @@ -508,7 +509,40 @@ const mutsAndExpects = () => [ multiEntry2: [3], multiEntry3: [{id: 2, tags: ["Apa", "x", "y"]}] } - ] + ], + // Issue https://github.com/dexie/Dexie.js/issues/1946 + [ + () => db.issue1946.add({name: "A", age: 20}), + { + compoundOrderBy: [{name: "A", age: 20, id: 1}], + compoundOrderByWithAutoIncKey: [{name: "A", age: 20, id: 1}] + } + ], + // Check that the order is valid in the 1946-case: + [ + () => db.issue1946.bulkAdd([ + {name: "A", age: 19}, + {name: "A", age: 19, mark: "x"}, + {name: "C", age: 18}, + {name: "A", age: 20, id: -1} // Override auto-increment + ]), + { + compoundOrderBy: [ + {name: "A", age: 19, id: 2}, + {name: "A", age: 19, mark: "x", id: 3}, // Even if cmp(["A",19],["A",19]) === 0, IDB orders implicitly by id. + {name: "A", age: 20, id: -1}, // Same here: order should implicitly be by PK last. + {name: "A", age: 20, id: 1}, + {name: "C", age: 18, id: 4}, + ], + compoundOrderByWithAutoIncKey: [ + {name: "A", age: 19, id: 2}, + {name: "A", age: 19, mark: "x", id: 3}, + {name: "A", age: 20, id: -1}, + {name: "A", age: 20, id: 1}, + {name: "C", age: 18, id: 4}, + ] + } + ], ] promisedTest("Full use case matrix", async ()=>{ @@ -544,6 +578,10 @@ promisedTest("Full use case matrix", async ()=>{ multiEntry1: () => db.multiEntry.where('tags').startsWith('A').primaryKeys(), multiEntry2: () => db.multiEntry.where({tags: "fooTag"}).primaryKeys(), multiEntry3: () => db.multiEntry.where({tags: "x"}).toArray(), + + // Issue https://github.com/dexie/Dexie.js/issues/1946 + compoundOrderBy: () => db.issue1946.orderBy('[name+age]').toArray(), + compoundOrderByWithAutoIncKey: () => db.issue1946.orderBy('[name+age+id]').toArray(), }; const expectedInitialResults = { itemsToArray: [{id: 1}, {id: 2}, {id: 3}], @@ -569,7 +607,11 @@ promisedTest("Full use case matrix", async ()=>{ multiEntry1: [], multiEntry2: [], - multiEntry3: [] + multiEntry3: [], + + // Issue https://github.com/dexie/Dexie.js/issues/1946 + compoundOrderBy: [], + compoundOrderByWithAutoIncKey: [] } let flyingNow = 0; //let signal = new Signal();