Fix: Remove from a read-only index (#506)

The index read by `view()` is read-only. When I did a `remove()` on that index, it crashed. --------- Co-authored-by: Ash Vardanian <[email protected]>
unum-cloud · Oct 22, 2024 · c27c99d · c27c99d
1 parent 8fa3090
commit c27c99d
Show file tree

Hide file tree

Showing 3 changed files with 108 additions and 25 deletions.
diff --git 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 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));

diff --git 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<Napi::BigUint64Array>();
     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<default_key_t>(keys[i])).completed);
+        auto result = native_->remove(static_cast<default_key_t>(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
@@ -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"
+            }
+        );
+    });
 });