From 6d63cce2bcbf707dd21216840a3deb4d0417e80e Mon Sep 17 00:00:00 2001 From: Ewout Stortenbeker Date: Wed, 16 Aug 2023 22:38:38 +0200 Subject: [PATCH 1/4] Fix #239 deletion causing empty leaf issue --- src/btree/binary-tree.ts | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/btree/binary-tree.ts b/src/btree/binary-tree.ts index fa7587f..ba2d0e2 100644 --- a/src/btree/binary-tree.ts +++ b/src/btree/binary-tree.ts @@ -1758,9 +1758,14 @@ export class BinaryBPlusTree { ? results.filter(r => options.stats ? r.totalValues > 0 : r.value !== null) : results; } + const results = [] as Array<{ key: NodeEntryKeyType, value: any, totalValues: number }>; // Get upperbound const lastLeaf = await this._getLastLeaf(); + if (!lastLeaf.parentNode && lastLeaf.entries.length === 0) { + // Empty single-leaf tree + return [] as typeof results; + } const lastEntry = lastLeaf.entries.slice(-1)[0]; const lastKey = lastEntry.key; @@ -1783,7 +1788,7 @@ export class BinaryBPlusTree { } // Some keys might be out of bounds, others must be looked up - const results = [] as Array<{ key: NodeEntryKeyType, value: any, totalValues: number }>, lookups = [] as NodeEntryKeyType[]; + const lookups = [] as NodeEntryKeyType[]; for (let i = 0; i < keys.length; i++) { const key = keys[i]; if (_isLess(key, firstKey) || _isMore(key, lastKey)) { @@ -2847,8 +2852,17 @@ export class BinaryBPlusTree { else { // Parent node has only 1 entry, removing it would also make parent node empty... // throw new DetailedError('leaf-empty', 'leaf is now empty and parent node has only 1 entry, tree will have to be rebuilt'); - // Write the empty leaf anyway, will be removed automatically upon a future tree rebuild. + // Write the empty leaf anyway, will be removed automatically with a tree rebuild. await this._writeLeaf(leaf); + + // Rebuild the tree + const options: Parameters[1] = { + allocatedBytes: this.info.byteLength, + fillFactor: this.info.fillFactor, + increaseMaxEntries: false, + }; + await this._rebuild(BinaryWriter.forFunction(this._writeFn), options); + await this._loadInfo(); // reload info } }; @@ -2881,10 +2895,7 @@ export class BinaryBPlusTree { return pointsThisDirection(node.parentNode); } else { - // There is no parent, this is the gtChild - if (!_isMoreOrEqual(key, node.entries.slice(-1)[0].key)) { - throw new Error('DEV ERROR: this tree is not right..'); - } + // There is no parent, this is a single-leaf tree return true; } }; From cae1cd6f8b07db19badc15d302e6c7155640f16c Mon Sep 17 00:00:00 2001 From: Ewout Stortenbeker Date: Wed, 16 Aug 2023 22:39:38 +0200 Subject: [PATCH 2/4] Improve readability --- src/btree/binary-tree.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/btree/binary-tree.ts b/src/btree/binary-tree.ts index ba2d0e2..ebca41c 100644 --- a/src/btree/binary-tree.ts +++ b/src/btree/binary-tree.ts @@ -3011,7 +3011,7 @@ export class BinaryBPlusTree { throw new DetailedError('small-ptrs-deprecated', 'small ptrs have deprecated, tree will have to be rebuilt'); } const entryIndex = leaf.entries.findIndex(entry => _isEqual(key, entry.key)); - if (!~entryIndex) { return; } + if (entryIndex < 0) { return; } if (this.info.isUnique || typeof recordPointer === 'undefined' || leaf.entries[entryIndex].totalValues === 1) { leaf.entries.splice(entryIndex, 1); } @@ -3020,7 +3020,7 @@ export class BinaryBPlusTree { } else { const valueIndex = leaf.entries[entryIndex].values.findIndex(val => _compareBinary(val.recordPointer, recordPointer)); - if (!~valueIndex) { return; } + if (valueIndex < 0) { return; } leaf.entries[entryIndex].values.splice(valueIndex, 1); } if (leaf.parentNode && leaf.entries.length === 0) { From fba5eded0eac48d679b25937fc82124c46328f1c Mon Sep 17 00:00:00 2001 From: Ewout Stortenbeker Date: Wed, 16 Aug 2023 22:40:27 +0200 Subject: [PATCH 3/4] Add unit test --- src/storage/binary/test.spec.ts | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 src/storage/binary/test.spec.ts diff --git a/src/storage/binary/test.spec.ts b/src/storage/binary/test.spec.ts new file mode 100644 index 0000000..8c7166e --- /dev/null +++ b/src/storage/binary/test.spec.ts @@ -0,0 +1,38 @@ +import { AceBase } from '../..'; +import { createTempDB } from '../../test/tempdb'; + +describe('issue', () => { + let db: AceBase, removeDB: () => Promise; + + beforeAll(async ()=> { + ({ db, removeDB } = await createTempDB({ config(options) { + options.logLevel = 'warn'; + }})); + }); + afterAll(async () => { + await removeDB(); + }); + + it('#239', async () => { + // Created for issue #239 ("TypeError when trying to add new records after removing old ones") + const ref = db.ref('table'); + + // add "large" dataset to the database + const songIds1 = Array(110).fill(0).map((_value, index) => `id-${index}`); + await ref.update(songIds1.reduce((obj, songId) => { + obj[songId] = { playlistId: 'playlist1' }; + return obj; + }, {} as any)); + + // remove all the added records with the query + await ref.query().filter('playlistId', '==', 'playlist1').remove(); + + // add new "small" dataset to the database -> error + const songIds2 = Array(10).fill(0).map((_value, index) => `id-${index}`); + await ref.update(songIds2.reduce((obj, songId) => { + obj[songId] = { playlistId: 'playlist1' }; + return obj; + }, {} as any)); + + }); +}); From bae638645e2ec02c9e08b8db5b5b8738782b569a Mon Sep 17 00:00:00 2001 From: Ewout Stortenbeker Date: Wed, 16 Aug 2023 23:12:49 +0200 Subject: [PATCH 4/4] Handle empty leafs --- src/btree/binary-tree.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/btree/binary-tree.ts b/src/btree/binary-tree.ts index ebca41c..8439368 100644 --- a/src/btree/binary-tree.ts +++ b/src/btree/binary-tree.ts @@ -1761,9 +1761,12 @@ export class BinaryBPlusTree { const results = [] as Array<{ key: NodeEntryKeyType, value: any, totalValues: number }>; // Get upperbound - const lastLeaf = await this._getLastLeaf(); - if (!lastLeaf.parentNode && lastLeaf.entries.length === 0) { - // Empty single-leaf tree + let lastLeaf = await this._getLastLeaf(); + while (lastLeaf.entries.length === 0 && lastLeaf.hasPrevious) { + lastLeaf = await lastLeaf.getPrevious(); + } + if (lastLeaf.entries.length === 0) { + // Empty tree return [] as typeof results; } const lastEntry = lastLeaf.entries.slice(-1)[0]; @@ -1778,7 +1781,10 @@ export class BinaryBPlusTree { } // Get lowerbound - const firstLeaf = await this._getFirstLeaf(); + let firstLeaf = await this._getFirstLeaf(); + while (firstLeaf.entries.length === 0 && firstLeaf.hasNext) { + firstLeaf = await firstLeaf.getNext(); + } const firstEntry = firstLeaf.entries[0]; const firstKey = firstEntry.key;