Skip to content

Commit

Permalink
Bugfix: liveQueries of compound index involving auto-incremented prim…
Browse files Browse the repository at this point in the history
…ary key as part (#1948)

* Repro of #1946

* Resolve #1946

* Verifying also that order is correct
  • Loading branch information
dfahlander committed Apr 6, 2024
1 parent bdbbf96 commit d007a67
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 4 deletions.
21 changes: 20 additions & 1 deletion src/live-query/observability-middleware.ts
Expand Up @@ -45,8 +45,11 @@ export const observabilityMiddleware: Middleware<DBCore> = {
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) => {
Expand Down Expand Up @@ -117,6 +120,22 @@ export const observabilityMiddleware: Middleware<DBCore> = {
// 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 || {},
Expand Down
48 changes: 45 additions & 3 deletions test/tests-live-query.js
Expand Up @@ -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', ()=> {
Expand Down Expand Up @@ -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 ()=>{
Expand Down Expand Up @@ -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}],
Expand All @@ -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();
Expand Down

0 comments on commit d007a67

Please sign in to comment.