From ddae6b281f5ac07e7721eab68ce41fac3ae216a5 Mon Sep 17 00:00:00 2001 From: Abe Tomoaki Date: Sun, 20 Oct 2024 12:55:26 +0900 Subject: [PATCH 1/2] Fix: Fix crash when trying to remove with read-only data The index read by view() is read-only. When I did a remove() on that index, it crashed. Although it is an invalid operation to do a remove() on a view() index, it is not a better way to crash, so we have fixed it to return an error. --- include/usearch/index_dense.hpp | 2 + javascript/lib.cpp | 9 ++- javascript/usearch.test.js | 122 ++++++++++++++++++++++++++------ 3 files changed, 108 insertions(+), 25 deletions(-) diff --git a/include/usearch/index_dense.hpp b/include/usearch/index_dense.hpp index 7c7b9b55..0868face 100644 --- a/include/usearch/index_dense.hpp +++ b/include/usearch/index_dense.hpp @@ -1460,6 +1460,8 @@ class index_dense_gt { labeling_result_t remove(vector_key_t key) { usearch_assert_m(config().enable_key_lookups, "Key lookups are disabled"); labeling_result_t result; + if (typed_->is_immutable()) + return result.failed("Can't remove to an immutable index"); unique_lock_t lookup_lock(slot_lookup_mutex_); auto matching_slots = slot_lookup_.equal_range(key_and_slot_t::any_slot(key)); diff --git a/javascript/lib.cpp b/javascript/lib.cpp index 03307a1a..b259f0b0 100644 --- a/javascript/lib.cpp +++ b/javascript/lib.cpp @@ -254,11 +254,14 @@ Napi::Value CompiledIndex::Remove(Napi::CallbackInfo const& ctx) { Napi::Env env = ctx.Env(); Napi::BigUint64Array keys = ctx[0].As(); std::size_t length = keys.ElementLength(); - Napi::Array result = Napi::Array::New(env, length); + Napi::Array results = Napi::Array::New(env, length); for (std::size_t i = 0; i < length; ++i) { - result[i] = Napi::Number::New(env, native_->remove(static_cast(keys[i])).completed); + auto result = native_->remove(static_cast(keys[i])); + if (!result) + Napi::Error::New(ctx.Env(), result.error.release()).ThrowAsJavaScriptException(); + results[i] = Napi::Number::New(env, result.completed); } - return result; + return results; } Napi::Value CompiledIndex::Contains(Napi::CallbackInfo const& ctx) { diff --git a/javascript/usearch.test.js b/javascript/usearch.test.js index c21b80ae..c04bc436 100644 --- a/javascript/usearch.test.js +++ b/javascript/usearch.test.js @@ -15,38 +15,84 @@ function assertAlmostEqual(actual, expected, tolerance = 1e-6) { } -test('Single-entry operations', () => { - const index = new usearch.Index(2, 'l2sq'); +test('Single-entry operations', async (t) => { + await t.test('index info', () => { + const index = new usearch.Index(2, 'l2sq'); - assert.equal(index.connectivity(), 16, 'connectivity should be 16'); - assert.equal(index.dimensions(), 2, 'dimensions should be 2'); - assert.equal(index.size(), 0, 'initial size should be 0'); + assert.equal(index.connectivity(), 16, 'connectivity should be 16'); + assert.equal(index.dimensions(), 2, 'dimensions should be 2'); + assert.equal(index.size(), 0, 'initial size should be 0'); + }); + + await t.test('add', () => { + const index = new usearch.Index(2, 'l2sq'); + + index.add(15n, new Float32Array([10, 20])); + index.add(16n, new Float32Array([10, 25])); - index.add(15n, new Float32Array([10, 20])); - index.add(16n, new Float32Array([10, 25])); + assert.equal(index.size(), 2, 'size after adding elements should be 2'); + assert.equal(index.contains(15), true, 'entry must be present after insertion'); + + const results = index.search(new Float32Array([13, 14]), 2); + + assert.deepEqual(results.keys, new BigUint64Array([15n, 16n]), 'keys should be 15 and 16'); + assert.deepEqual(results.distances, new Float32Array([45, 130]), 'distances should be 45 and 130'); + }); - assert.equal(index.size(), 2, 'size after adding elements should be 2'); - assert.equal(index.contains(15), true, 'entry must be present after insertion'); + await t.test('remove', () => { + const index = new usearch.Index(2, 'l2sq'); - const results = index.search(new Float32Array([13, 14]), 2); + index.add(15n, new Float32Array([10, 20])); + index.add(16n, new Float32Array([10, 25])); + index.add(25n, new Float32Array([20, 40])); + index.add(26n, new Float32Array([20, 45])); - assert.deepEqual(results.keys, new BigUint64Array([15n, 16n]), 'keys should be 15 and 16'); - assert.deepEqual(results.distances, new Float32Array([45, 130]), 'distances should be 45 and 130'); + assert.equal(index.remove(15n), 1); + + assert.equal(index.size(), 3, 'size after remoing elements should be 3'); + assert.equal(index.contains(15), false, 'entry must be absent after insertion'); + + const results = index.search(new Float32Array([13, 14]), 2); + + assert.deepEqual(results.keys, new BigUint64Array([16n, 25n]), 'keys should not include 15'); + }); }); -test('Batch operations', () => { - const indexBatch = new usearch.Index(2, 'l2sq'); +test('Batch operations', async (t) => { + await t.test('add', () => { + const indexBatch = new usearch.Index(2, 'l2sq'); - const keys = [15n, 16n]; - const vectors = [new Float32Array([10, 20]), new Float32Array([10, 25])]; + const keys = [15n, 16n]; + const vectors = [new Float32Array([10, 20]), new Float32Array([10, 25])]; - indexBatch.add(keys, vectors); - assert.equal(indexBatch.size(), 2, 'size after adding batch should be 2'); + indexBatch.add(keys, vectors); + assert.equal(indexBatch.size(), 2, 'size after adding batch should be 2'); - const results = indexBatch.search(new Float32Array([13, 14]), 2); + const results = indexBatch.search(new Float32Array([13, 14]), 2); - assert.deepEqual(results.keys, new BigUint64Array([15n, 16n]), 'keys should be 15 and 16'); - assert.deepEqual(results.distances, new Float32Array([45, 130]), 'distances should be 45 and 130'); + assert.deepEqual(results.keys, new BigUint64Array([15n, 16n]), 'keys should be 15 and 16'); + assert.deepEqual(results.distances, new Float32Array([45, 130]), 'distances should be 45 and 130'); + }); + + await t.test('remove', () => { + const indexBatch = new usearch.Index(2, 'l2sq'); + + const keys = [15n, 16n, 25n, 26n]; + const vectors = [ + new Float32Array([10, 20]), + new Float32Array([10, 25]), + new Float32Array([20, 40]), + new Float32Array([20, 45]) + ]; + indexBatch.add(keys, vectors); + + assert.deepEqual(indexBatch.remove([15n, 25n]), [1, 1]) + assert.equal(indexBatch.size(), 2, 'size after removing batch should be 2'); + + const results = indexBatch.search(new Float32Array([13, 14]), 2); + + assert.deepEqual(results.keys, new BigUint64Array([16n, 26n]), 'keys should not include 15 and 25'); + }); }); test("Expected results", () => { @@ -161,7 +207,7 @@ test('Serialization', async (t) => { // todo: Skip as the test fails only on windows. // The following error in afterEach(). // `error: "EBUSY: resource busy or locked, unlink` - await t.test('view', {skip: process.platform === 'win32'}, () => { + await t.test('view: Read data', {skip: process.platform === 'win32'}, () => { const index = new usearch.Index({ metric: "l2sq", connectivity: 16, @@ -174,4 +220,36 @@ test('Serialization', async (t) => { assert.deepEqual(results.keys, new BigUint64Array([42n])); assertAlmostEqual(results.distances[0], new Float32Array([0])); }); + + await t.test('view: Invalid operations: add', {skip: process.platform === 'win32'}, () => { + const index = new usearch.Index({ + metric: "l2sq", + connectivity: 16, + dimensions: 3, + }); + index.view(indexPath); + assert.throws( + () => index.add(43n, new Float32Array([0.2, 0.6, 0.4])), + { + name: 'Error', + message: "Can't add to an immutable index" + } + ); + }); + + await t.test('view: Invalid operations: remove', {skip: process.platform === 'win32'}, () => { + const index = new usearch.Index({ + metric: "l2sq", + connectivity: 16, + dimensions: 3, + }); + index.view(indexPath); + assert.throws( + () => index.remove(42n), + { + name: 'Error', + message: "Can't remove to an immutable index" + } + ); + }); }); From 24f97d6260edcfb6d2bb00359e91dc5359212850 Mon Sep 17 00:00:00 2001 From: Ash Vardanian <1983160+ashvardanian@users.noreply.github.com> Date: Tue, 22 Oct 2024 14:49:50 +0100 Subject: [PATCH 2/2] Docs: Error message --- include/usearch/index_dense.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/usearch/index_dense.hpp b/include/usearch/index_dense.hpp index 0868face..93b55877 100644 --- a/include/usearch/index_dense.hpp +++ b/include/usearch/index_dense.hpp @@ -1461,7 +1461,7 @@ class index_dense_gt { usearch_assert_m(config().enable_key_lookups, "Key lookups are disabled"); labeling_result_t result; if (typed_->is_immutable()) - return result.failed("Can't remove to an immutable index"); + return result.failed("Can't remove from an immutable index"); unique_lock_t lookup_lock(slot_lookup_mutex_); auto matching_slots = slot_lookup_.equal_range(key_and_slot_t::any_slot(key));