Reject incompatible MinHashLSH merges

datasketch/lsh.py

@@ -239,8 +239,10 @@ def merge(self, other: MinHashLSH, check_overlap: bool = False):
         of both.
 
         Note:
-            Only num_perm, number of bands and sizes of each band is checked for equivalency of two MinHashLSH indexes.
-            Other initialization parameters threshold, weights, storage_config, prepickle and hash_func are not checked.
+            The indexes must use the same number and size of bands, key
+            serialization mode, band hash function, and byte-key requirements.
+            Threshold, weights, and storage backend may differ when those
+            effective index parameters are equivalent.
 
         Args:
             other (MinHashLSH): The other MinHashLSH.
@@ -353,10 +355,18 @@ def _insert(
 
     def __equivalent(self, other: MinHashLSH) -> bool:
         """Returns:
-        bool: If the two MinHashLSH have equal num_perm, number of bands, size of each band then two are equivalent.
+        bool: Whether stored keys and band hashes can be copied safely.
 
         """
-        return type(self) is type(other) and self.h == other.h and self.b == other.b and self.r == other.r
+        return (
+            type(self) is type(other)
+            and self.h == other.h
+            and self.b == other.b
+            and self.r == other.r
+            and self.prepickle == other.prepickle
+            and self.hashfunc is other.hashfunc
+            and self._require_bytes_keys == other._require_bytes_keys
+        )
 
     def _merge(self, other: MinHashLSH, check_overlap: bool = False, buffer: bool = False) -> None:
         if self.__equivalent(other):

test/test_lsh.py

@@ -17,6 +17,14 @@ def fake_redis(**kwargs):
     return redis
 
 
+def first_band_hash(data):
+    return data
+
+
+def second_band_hash(data):
+    return data[::-1]
+
+
 class TestMinHashLSH(unittest.TestCase):
     def test_init(self):
         lsh = MinHashLSH(threshold=0.8)
@@ -342,6 +350,28 @@ def test_merge(self):
 
         lsh1.merge(lsh4, check_overlap=False)
 
+    def test_merge_rejects_different_band_hash_functions(self):
+        minhash = MinHash(16)
+        minhash.update(b"value")
+        destination = MinHashLSH(threshold=0.5, num_perm=16, hashfunc=first_band_hash)
+        source = MinHashLSH(threshold=0.5, num_perm=16, hashfunc=second_band_hash)
+        source.insert("source", minhash)
+
+        with self.assertRaisesRegex(ValueError, "different initialization parameters"):
+            destination.merge(source)
+        self.assertNotIn("source", destination)
+
+    def test_merge_rejects_different_key_serialization(self):
+        minhash = MinHash(16)
+        minhash.update(b"value")
+        destination = MinHashLSH(threshold=0.5, num_perm=16, prepickle=False)
+        source = MinHashLSH(threshold=0.5, num_perm=16, prepickle=True)
+        source.insert("source", minhash)
+
+        with self.assertRaisesRegex(ValueError, "different initialization parameters"):
+            destination.merge(source)
+        self.assertNotIn("source", destination)
+
     def test_merge_redis(self):
         with patch("redis.Redis", fake_redis):
             lsh1 = MinHashLSH(