Vector Sets: VISMEMBER and other fixes (#13941)

In this PR there is also a VADD leak fixed (when wrong arity is
reported) and improvements to the test.

modules/vector-sets/README.md

@@ -11,6 +11,8 @@ Moreover, Vector sets implement optional filtered search capabilities: it is pos
 
 ## Installation
 
+**WARNING:** If you are running **Redis 8.0 RC1 or greater** you don't need to install anything, just compile Redis, and the Vector Sets commands will be part of the default install. Otherwise to test Vector Sets with older Redis versions follow the following instructions.
+
 Build with:
 
     make
@@ -171,6 +173,14 @@ For q8 quantization, an additional elements is also returned: the quantization
 range, so the integers from -127 to 127 represent (normalized) components
 in the range `-range`, `+range`.
 
+**VISMEMBER: test if a given element already exists**
+
+This command will return 1 (or true) if the specified element is already in the vector set, otherwise 0 (or false) is returned.
+
+    VISMEMBER key element
+
+As with other existence check Redis commands, if the key does not exist it is considered as if it was empty, thus the element is reported as non existing.
+
 **VLINKS: introspection command that shows neighbors for a node**
 
     VLINKS key element [WITHSCORES]

modules/vector-sets/test.py

@@ -10,7 +10,6 @@
 # (RSALv2) or the Server Side Public License v1 (SSPLv1).
 #
 
-#!/usr/bin/env python3
 import redis
 import random
 import struct
@@ -20,13 +19,15 @@ import sys
 import os
 import importlib
 import inspect
+import argparse
 from typing import List, Tuple, Optional
 from dataclasses import dataclass
 
 def colored(text: str, color: str) -> str:
     colors = {
         'red': '\033[91m',
-        'green': '\033[92m'
+        'green': '\033[92m',
+        'yellow': '\033[93m'
     }
     reset = '\033[0m'
     return f"{colors.get(color, '')}{text}{reset}"
@@ -63,7 +64,7 @@ def generate_random_vector(dim: int) -> List[float]:
     norm = math.sqrt(sum(x*x for x in vec))
     return [x/norm for x in vec]
 
-def fill_redis_with_vectors(r: redis.Redis, key: str, count: int, dim: int, 
+def fill_redis_with_vectors(r: redis.Redis, key: str, count: int, dim: int,
                           with_reduce: Optional[int] = None) -> VectorData:
     """Fill Redis with random vectors and return a VectorData object for verification."""
     vectors = []
@@ -86,16 +87,19 @@ def fill_redis_with_vectors(r: redis.Redis, key: str, count: int, dim: int,
     return VectorData(vectors=vectors, names=names)
 
 class TestCase:
-    def __init__(self):
+    def __init__(self, primary_port=6379, replica_port=6380):
         self.error_msg = None
         self.error_details = None
         self.test_key = f"test:{self.__class__.__name__.lower()}"
-        # Primary Redis instance (default port)
-        self.redis = redis.Redis()
-        # Replica Redis instance (port 6380)
-        self.replica = redis.Redis(port=6380)
+        # Primary Redis instance
+        self.redis = redis.Redis(port=primary_port)
+        # Replica Redis instance
+        self.replica = redis.Redis(port=replica_port)
         # Replication status
         self.replication_setup = False
+        # Ports
+        self.primary_port = primary_port
+        self.replica_port = replica_port
 
     def setup(self):
         self.redis.delete(self.test_key)
@@ -109,7 +113,7 @@ class TestCase:
         Returns True if replication is successfully established, False otherwise.
         """
         # Configure replica to replicate from primary
-        self.replica.execute_command('REPLICAOF', '127.0.0.1', 6379)
+        self.replica.execute_command('REPLICAOF', '127.0.0.1', self.primary_port)
 
         # Wait for replication to be established
         max_attempts = 10
@@ -120,7 +124,7 @@ class TestCase:
             # Check if replication is established
             if (repl_info.get('role') == 'slave' and
                 repl_info.get('master_host') == '127.0.0.1' and
-                repl_info.get('master_port') == 6379 and
+                repl_info.get('master_port') == self.primary_port and
                 repl_info.get('master_link_status') == 'up'):
 
                 self.replication_setup = True
@@ -162,7 +166,7 @@ class TestCase:
         """"Each test class should override this if it takes a significant amount of time to run. Default is 100ms"""
         return 0.1
 
-def find_test_classes():
+def find_test_classes(primary_port, replica_port):
     test_classes = []
     tests_dir = 'tests'
 
@@ -176,20 +180,67 @@ def find_test_classes():
                 module = importlib.import_module(module_name)
                 for name, obj in inspect.getmembers(module):
                     if inspect.isclass(obj) and obj.__name__ != 'TestCase' and hasattr(obj, 'test'):
-                        test_classes.append(obj())
+                        # Create test instance with specified ports
+                        test_instance = obj()
+                        test_instance.redis = redis.Redis(port=primary_port)
+                        test_instance.replica = redis.Redis(port=replica_port)
+                        test_instance.primary_port = primary_port
+                        test_instance.replica_port = replica_port
+                        test_classes.append(test_instance)
             except Exception as e:
                 print(f"Error loading {file}: {e}")
 
     return test_classes
 
-def run_tests():
-    print("================================================\n"+
-          "Make sure to have Redis running in the localhost\n"+
-          "with --enable-debug-command yes\n"+
-          "Both primary (6379) and replica (6380) instances\n"+
-          "================================================\n")
+def check_redis_empty(r, instance_name):
+    """Check if Redis instance is empty"""
+    try:
+        dbsize = r.dbsize()
+        if dbsize > 0:
+            print(colored(f"ERROR: {instance_name} Redis instance is not empty (dbsize: {dbsize}).", "red"))
+            print(colored("Make sure you're not using a production instance and that all data is safe to delete.", "red"))
+            sys.exit(1)
+    except redis.exceptions.ConnectionError:
+        print(colored(f"ERROR: Cannot connect to {instance_name} Redis instance.", "red"))
+        sys.exit(1)
 
-    tests = find_test_classes()
+def check_replica_running(replica_port):
+    """Check if replica Redis instance is running"""
+    r = redis.Redis(port=replica_port)
+    try:
+        r.ping()
+        return True
+    except redis.exceptions.ConnectionError:
+        print(colored(f"WARNING: Replica Redis instance (port {replica_port}) is not running.", "yellow"))
+        print(colored("Replication tests will fail. Make sure to start the replica instance.", "yellow"))
+        return False
+
+def run_tests():
+    # Parse command line arguments
+    parser = argparse.ArgumentParser(description='Run Redis vector tests.')
+    parser.add_argument('--primary-port', type=int, default=6379, help='Primary Redis instance port (default: 6379)')
+    parser.add_argument('--replica-port', type=int, default=6380, help='Replica Redis instance port (default: 6380)')
+    args = parser.parse_args()
+
+    print("================================================")
+    print(f"Make sure to have Redis running on localhost")
+    print(f"Primary port: {args.primary_port}")
+    print(f"Replica port: {args.replica_port}")
+    print("with --enable-debug-command yes")
+    print("================================================\n")
+
+    # Check if Redis instances are empty
+    primary = redis.Redis(port=args.primary_port)
+    replica = redis.Redis(port=args.replica_port)
+
+    check_redis_empty(primary, "Primary")
+
+    # Check if replica is running
+    replica_running = check_replica_running(args.replica_port)
+    if replica_running:
+        check_redis_empty(replica, "Replica")
+
+    tests = find_test_classes(args.primary_port, args.replica_port)
     if not tests:
         print("No tests found!")
         return

modules/vector-sets/tests/vismember.py

@@ -0,0 +1,47 @@
+from test import TestCase, generate_random_vector
+import struct
+
+class BasicVISMEMBER(TestCase):
+    def getname(self):
+        return "VISMEMBER basic functionality"
+
+    def test(self):
+        # Add multiple vectors to the vector set
+        vec1 = generate_random_vector(4)
+        vec2 = generate_random_vector(4)
+        vec_bytes1 = struct.pack('4f', *vec1)
+        vec_bytes2 = struct.pack('4f', *vec2)
+
+        # Create item keys
+        item1 = f'{self.test_key}:item:1'
+        item2 = f'{self.test_key}:item:2'
+        nonexistent_item = f'{self.test_key}:item:nonexistent'
+
+        # Add the vectors
+        self.redis.execute_command('VADD', self.test_key, 'FP32', vec_bytes1, item1)
+        self.redis.execute_command('VADD', self.test_key, 'FP32', vec_bytes2, item2)
+
+        # Test VISMEMBER with existing elements
+        result1 = self.redis.execute_command('VISMEMBER', self.test_key, item1)
+        assert result1 == 1, f"VISMEMBER should return 1 for existing item, got {result1}"
+
+        result2 = self.redis.execute_command('VISMEMBER', self.test_key, item2)
+        assert result2 == 1, f"VISMEMBER should return 1 for existing item, got {result2}"
+
+        # Test VISMEMBER with non-existent element
+        result3 = self.redis.execute_command('VISMEMBER', self.test_key, nonexistent_item)
+        assert result3 == 0, f"VISMEMBER should return 0 for non-existent item, got {result3}"
+
+        # Test VISMEMBER with non-existent key
+        nonexistent_key = f'{self.test_key}_nonexistent'
+        result4 = self.redis.execute_command('VISMEMBER', nonexistent_key, item1)
+        assert result4 == 0, f"VISMEMBER should return 0 for non-existent key, got {result4}"
+
+        # Test VISMEMBER after removing an element
+        self.redis.execute_command('VREM', self.test_key, item1)
+        result5 = self.redis.execute_command('VISMEMBER', self.test_key, item1)
+        assert result5 == 0, f"VISMEMBER should return 0 after element removal, got {result5}"
+
+        # Verify item2 still exists
+        result6 = self.redis.execute_command('VISMEMBER', self.test_key, item2)
+        assert result6 == 1, f"VISMEMBER should still return 1 for remaining item, got {result6}"

modules/vector-sets/vset.c

@@ -577,7 +577,10 @@ int VADD_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
         return RedisModule_ReplyWithError(ctx,"ERR invalid vector specification");
 
     /* Missing element string at the end? */
-    if (argc-2-consumed_args < 1) return RedisModule_WrongArity(ctx);
+    if (argc-2-consumed_args < 1) {
+        RedisModule_Free(vec);
+        return RedisModule_WrongArity(ctx);
+    }
 
     /* Parse options after the element string. */
     uint32_t quant_type = HNSW_QUANT_Q8; // Default quantization type.
@@ -1646,6 +1649,37 @@ int VRANDMEMBER_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int
     return REDISMODULE_OK;
 }
 
+/* VISMEMBER key element
+ * Check if an element exists in a vector set.
+ * Returns 1 if the element exists, 0 if not. */
+int VISMEMBER_RedisCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
+    RedisModule_AutoMemory(ctx);
+    if (argc != 3) return RedisModule_WrongArity(ctx);
+
+    RedisModuleString *key = argv[1];
+    RedisModuleString *element = argv[2];
+
+    /* Open key. */
+    RedisModuleKey *keyptr = RedisModule_OpenKey(ctx, key, REDISMODULE_READ);
+    int type = RedisModule_KeyType(keyptr);
+
+    /* Handle non-existing key or wrong type. */
+    if (type == REDISMODULE_KEYTYPE_EMPTY) {
+        /* An element of a non existing key does not exist, like
+         * SISMEMBER & similar. */
+        return RedisModule_ReplyWithBool(ctx, 0);
+    }
+    if (RedisModule_ModuleTypeGetType(keyptr) != VectorSetType) {
+        return RedisModule_ReplyWithError(ctx, REDISMODULE_ERRORMSG_WRONGTYPE);
+    }
+
+    /* Get the object and test membership via the dictionary in constant
+     * time (assuming a member of average size). */
+    struct vsetObject *vset = RedisModule_ModuleTypeGetValue(keyptr);
+    hnswNode *node = RedisModule_DictGet(vset->dict, element, NULL);
+    return RedisModule_ReplyWithBool(ctx, node != NULL);
+}
+
 /* ============================== vset type methods ========================= */
 
 #define SAVE_FLAG_HAS_PROJMATRIX    (1<<0)
@@ -1969,6 +2003,10 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
         VRANDMEMBER_RedisCommand, "readonly", 1, 1, 1) == REDISMODULE_ERR)
         return REDISMODULE_ERR;
 
+    if (RedisModule_CreateCommand(ctx, "VISMEMBER",
+        VISMEMBER_RedisCommand, "readonly", 1, 1, 1) == REDISMODULE_ERR)
+        return REDISMODULE_ERR;
+
     hnsw_set_allocator(RedisModule_Free, RedisModule_Alloc,
                        RedisModule_Realloc);
 

utils/req-res-log-validator.py

@@ -70,6 +70,7 @@ IGNORED_COMMANDS = {
     "VREM",
     "VSETATTR",
     "VSIM",
+    "VISMEMBER",
 }
 
 class Request(object):