mirror of
https://github.com/qdrant/qdrant-client.git
synced 2026-09-21 13:37:55 -05:00
fix: do not accept empty vectors and multivectors in local mode (#1405)
* fix: do not accept empty vectors and multivectors in local mode * fix: do not modify points if vector does not pass validation
This commit is contained in:
@@ -100,6 +100,25 @@ def to_jsonable_python(x: Any) -> Any:
|
||||
return json.loads(json.dumps(x, allow_nan=True, default=_to_jsonable_python))
|
||||
|
||||
|
||||
def validate_dense_vector(vector: Any, vector_name: str) -> None:
|
||||
"""Reject empty dense vectors, as the server does at write time."""
|
||||
if len(vector) == 0:
|
||||
raise ValueError(f"Wrong input: Dense vector must not be empty for vector '{vector_name}'")
|
||||
|
||||
|
||||
def validate_multivector(vector: Any, vector_name: str) -> None:
|
||||
"""Reject empty multivectors and multivectors holding empty vectors, as the server does."""
|
||||
if len(vector) == 0:
|
||||
raise ValueError(f"Wrong input: Multivector must not be empty for vector '{vector_name}'")
|
||||
|
||||
for sub_vector in vector:
|
||||
if hasattr(sub_vector, "__len__") and len(sub_vector) == 0:
|
||||
raise ValueError(
|
||||
"Wrong input: All vectors of a multivector must be non-empty "
|
||||
f"for vector '{vector_name}'"
|
||||
)
|
||||
|
||||
|
||||
class LocalCollection:
|
||||
"""
|
||||
LocalCollection is a class that represents a collection of vectors in the local storage.
|
||||
@@ -393,6 +412,20 @@ class LocalCollection:
|
||||
|
||||
raise ValueError(f"Malformed config.vectors: {self.config.vectors}")
|
||||
|
||||
def _validate_dense_or_multivector(self, vector: Any, vector_name: str) -> None:
|
||||
"""Reject empty vectors on the write path, the way the server does.
|
||||
|
||||
Sparse vectors are validated by `validate_sparse_vector`; an empty sparse vector is
|
||||
legitimate, so it is not routed here.
|
||||
"""
|
||||
if vector is None:
|
||||
return
|
||||
|
||||
if vector_name in self.multivectors:
|
||||
validate_multivector(vector, vector_name)
|
||||
elif vector_name in self.vectors:
|
||||
validate_dense_vector(vector, vector_name)
|
||||
|
||||
@classmethod
|
||||
def _check_include_pattern(cls, pattern: str, key: str) -> bool:
|
||||
"""
|
||||
@@ -2613,6 +2646,8 @@ class LocalCollection:
|
||||
validate_sparse_vector(vector)
|
||||
# sort sparse vector by indices before persistence
|
||||
updated_sparse_vectors[vector_name] = sort_sparse_vector(vector)
|
||||
else:
|
||||
self._validate_dense_or_multivector(vector, vector_name)
|
||||
# update point.vector with the modified values after iteration
|
||||
point.vector.update(updated_sparse_vectors)
|
||||
else:
|
||||
@@ -2627,6 +2662,7 @@ class LocalCollection:
|
||||
)
|
||||
if not self.vectors and not self.multivectors:
|
||||
raise ValueError("Wrong input: Not existing vector name error")
|
||||
self._validate_dense_or_multivector(point.vector, DEFAULT_VECTOR_NAME)
|
||||
|
||||
if isinstance(point.id, uuid.UUID):
|
||||
point.id = str(point.id)
|
||||
@@ -2703,23 +2739,31 @@ class LocalCollection:
|
||||
def _update_named_vectors(
|
||||
self, idx: int, vectors: dict[str, list[float] | SparseVector | list[list[float]]]
|
||||
) -> None:
|
||||
validated: list[tuple[str, Any]] = []
|
||||
for vector_name, vector in vectors.items():
|
||||
if vector_name not in self._all_vectors_keys:
|
||||
raise ValueError(f"Wrong input: Not existing vector name error: {vector_name}")
|
||||
|
||||
self.deleted_per_vector[vector_name][idx] = 0
|
||||
|
||||
if isinstance(vector, SparseVector):
|
||||
validate_sparse_vector(vector)
|
||||
old_vector = self.sparse_vectors[vector_name][idx]
|
||||
self._update_idf_remove(old_vector, vector_name)
|
||||
new_vector = sort_sparse_vector(vector)
|
||||
self.sparse_vectors[vector_name][idx] = new_vector
|
||||
self._update_idf_append(new_vector, vector_name)
|
||||
validated.append((vector_name, sort_sparse_vector(vector)))
|
||||
continue
|
||||
|
||||
self._validate_dense_or_multivector(vector, vector_name)
|
||||
vector_np = np.array(vector, dtype=np.float32)
|
||||
assert not np.isnan(vector_np).any(), "Vector contains NaN values"
|
||||
validated.append((vector_name, vector_np))
|
||||
|
||||
for vector_name, vector_np in validated:
|
||||
self.deleted_per_vector[vector_name][idx] = 0
|
||||
|
||||
if isinstance(vector_np, SparseVector):
|
||||
old_vector = self.sparse_vectors[vector_name][idx]
|
||||
self._update_idf_remove(old_vector, vector_name)
|
||||
self.sparse_vectors[vector_name][idx] = vector_np
|
||||
self._update_idf_append(vector_np, vector_name)
|
||||
continue
|
||||
|
||||
params = self.get_vector_params(vector_name)
|
||||
if vector_name in self.vectors:
|
||||
if params.distance == models.Distance.COSINE:
|
||||
|
||||
@@ -5,6 +5,7 @@ from collections import defaultdict
|
||||
import pytest
|
||||
|
||||
from qdrant_client.http import models
|
||||
from qdrant_client.http.exceptions import UnexpectedResponse
|
||||
from tests.congruence_tests.test_common import (
|
||||
COLLECTION_NAME,
|
||||
compare_collections,
|
||||
@@ -57,9 +58,7 @@ def test_upsert():
|
||||
COLLECTION_NAME,
|
||||
scroll_filter=id_filter,
|
||||
limit=1,
|
||||
)[
|
||||
0
|
||||
][0]
|
||||
)[0][0]
|
||||
remote_old_point = remote_client.scroll(COLLECTION_NAME, scroll_filter=id_filter, limit=1)[0][
|
||||
0
|
||||
]
|
||||
@@ -203,3 +202,96 @@ def test_upload_uuid_in_batches():
|
||||
UPLOAD_NUM_VECTORS,
|
||||
attrs=("points_count",),
|
||||
)
|
||||
|
||||
|
||||
def test_upsert_empty_multivector():
|
||||
"""Both clients must reject an empty multivector on every write path."""
|
||||
points = generate_multivector_fixtures(UPLOAD_NUM_VECTORS)
|
||||
|
||||
local_client = init_local()
|
||||
init_client(local_client, points, vectors_config=multi_vector_config)
|
||||
|
||||
remote_client = init_remote()
|
||||
init_client(remote_client, points, vectors_config=multi_vector_config)
|
||||
|
||||
existing_id = points[0].id
|
||||
new_point_id = UPLOAD_NUM_VECTORS + 1
|
||||
|
||||
# a multivector with no vectors at all, and one holding an empty vector
|
||||
cases = (
|
||||
([], "Multivector must not be empty"),
|
||||
([[]], "vectors of a multivector must be non-empty"),
|
||||
)
|
||||
for empty_multivector, local_error in cases:
|
||||
upsert_structs = (
|
||||
[models.PointStruct(id=new_point_id, vector={"multi-text": empty_multivector})],
|
||||
[models.PointStruct(id=existing_id, vector={"multi-text": empty_multivector})],
|
||||
models.Batch(ids=[new_point_id], vectors={"multi-text": [empty_multivector]}),
|
||||
)
|
||||
for upsert_struct in upsert_structs:
|
||||
with pytest.raises(ValueError, match=local_error):
|
||||
local_client.upsert(COLLECTION_NAME, upsert_struct)
|
||||
|
||||
with pytest.raises(UnexpectedResponse):
|
||||
remote_client.upsert(COLLECTION_NAME, upsert_struct)
|
||||
|
||||
point_vectors = [
|
||||
models.PointVectors(id=existing_id, vector={"multi-text": empty_multivector})
|
||||
]
|
||||
with pytest.raises(ValueError, match=local_error):
|
||||
local_client.update_vectors(COLLECTION_NAME, points=point_vectors)
|
||||
|
||||
with pytest.raises(UnexpectedResponse):
|
||||
remote_client.update_vectors(COLLECTION_NAME, points=point_vectors)
|
||||
|
||||
compare_collections(
|
||||
local_client,
|
||||
remote_client,
|
||||
UPLOAD_NUM_VECTORS,
|
||||
attrs=("points_count",),
|
||||
)
|
||||
|
||||
|
||||
def test_rejected_empty_multivector_update_leaves_point_untouched():
|
||||
"""A rejected update_vectors request must not make an absent multivector visible.
|
||||
|
||||
Regression: validation used to run after `deleted_per_vector` was cleared, so rejecting
|
||||
an empty multivector still exposed the placeholder to retrieval and search.
|
||||
"""
|
||||
local_client = init_local()
|
||||
remote_client = init_remote()
|
||||
|
||||
mvc = models.MultiVectorConfig(comparator=models.MultiVectorComparator.MAX_SIM)
|
||||
vectors_config = {
|
||||
"a": models.VectorParams(size=4, distance=models.Distance.COSINE, multivector_config=mvc),
|
||||
"b": models.VectorParams(size=4, distance=models.Distance.COSINE, multivector_config=mvc),
|
||||
}
|
||||
local_client.create_collection(COLLECTION_NAME, vectors_config=vectors_config)
|
||||
if remote_client.collection_exists(collection_name=COLLECTION_NAME):
|
||||
remote_client.delete_collection(collection_name=COLLECTION_NAME)
|
||||
remote_client.create_collection(COLLECTION_NAME, vectors_config=vectors_config)
|
||||
|
||||
query = [[1.0, 0.0, 0.0, 0.0]]
|
||||
|
||||
# point 1 has no "a" at all, so "a" is backed by a placeholder marked deleted
|
||||
absent_a = [models.PointStruct(id=1, vector={"b": query})]
|
||||
local_client.upsert(COLLECTION_NAME, absent_a)
|
||||
remote_client.upsert(COLLECTION_NAME, absent_a, wait=True)
|
||||
|
||||
for empty_multivector, local_error in (
|
||||
([], "Multivector must not be empty"),
|
||||
([[]], "vectors of a multivector must be non-empty"),
|
||||
):
|
||||
point_vectors = [models.PointVectors(id=1, vector={"a": empty_multivector})]
|
||||
with pytest.raises(ValueError, match=local_error):
|
||||
local_client.update_vectors(COLLECTION_NAME, points=point_vectors)
|
||||
with pytest.raises(UnexpectedResponse):
|
||||
remote_client.update_vectors(COLLECTION_NAME, points=point_vectors, wait=True)
|
||||
|
||||
for client in (local_client, remote_client):
|
||||
record = client.retrieve(COLLECTION_NAME, ids=[1], with_vectors=True)[0]
|
||||
assert "a" not in record.vector
|
||||
hits = client.query_points(COLLECTION_NAME, query=query, using="a", limit=5).points
|
||||
assert hits == []
|
||||
|
||||
compare_collections(local_client, remote_client, 1, attrs=("points_count",))
|
||||
|
||||
@@ -343,6 +343,116 @@ def test_upload_wrong_vectors():
|
||||
)
|
||||
|
||||
|
||||
def test_upsert_empty_dense_vector():
|
||||
"""Both clients must reject an empty dense vector on every write path.
|
||||
|
||||
An empty *sparse* vector, on the other hand, is legitimate and must stay accepted.
|
||||
"""
|
||||
local_client = init_local()
|
||||
remote_client = init_remote()
|
||||
|
||||
vectors_config = {"text": models.VectorParams(size=2, distance=models.Distance.COSINE)}
|
||||
sparse_vectors_config = {"text-sparse": models.SparseVectorParams()}
|
||||
|
||||
local_client.create_collection(
|
||||
collection_name=COLLECTION_NAME,
|
||||
vectors_config=vectors_config,
|
||||
sparse_vectors_config=sparse_vectors_config,
|
||||
)
|
||||
if remote_client.collection_exists(collection_name=COLLECTION_NAME):
|
||||
remote_client.delete_collection(collection_name=COLLECTION_NAME)
|
||||
remote_client.create_collection(
|
||||
collection_name=COLLECTION_NAME,
|
||||
vectors_config=vectors_config,
|
||||
sparse_vectors_config=sparse_vectors_config,
|
||||
)
|
||||
|
||||
# a valid point to overwrite later
|
||||
valid_points = [models.PointStruct(id=1, vector={"text": [0.1, 0.3]})]
|
||||
local_client.upsert(COLLECTION_NAME, valid_points)
|
||||
remote_client.upsert(COLLECTION_NAME, valid_points)
|
||||
|
||||
empty_points = [models.PointStruct(id=2, vector={"text": []})]
|
||||
overwrite_points = [models.PointStruct(id=1, vector={"text": []})]
|
||||
empty_batch = models.Batch(ids=[3], vectors={"text": [[]]})
|
||||
|
||||
for points in (empty_points, overwrite_points, empty_batch):
|
||||
with pytest.raises(ValueError, match="Dense vector must not be empty"):
|
||||
local_client.upsert(COLLECTION_NAME, points)
|
||||
|
||||
with pytest.raises(qdrant_client.http.exceptions.UnexpectedResponse):
|
||||
remote_client.upsert(COLLECTION_NAME, points)
|
||||
|
||||
empty_point_vectors = [models.PointVectors(id=1, vector={"text": []})]
|
||||
with pytest.raises(ValueError, match="Dense vector must not be empty"):
|
||||
local_client.update_vectors(COLLECTION_NAME, points=empty_point_vectors)
|
||||
|
||||
with pytest.raises(qdrant_client.http.exceptions.UnexpectedResponse):
|
||||
remote_client.update_vectors(COLLECTION_NAME, points=empty_point_vectors)
|
||||
|
||||
# an empty sparse vector is valid input for both clients
|
||||
empty_sparse_points = [
|
||||
models.PointStruct(
|
||||
id=4, vector={"text-sparse": models.SparseVector(indices=[], values=[])}
|
||||
)
|
||||
]
|
||||
local_client.upsert(COLLECTION_NAME, empty_sparse_points)
|
||||
remote_client.upsert(COLLECTION_NAME, empty_sparse_points, wait=True)
|
||||
|
||||
compare_collections(local_client, remote_client, UPLOAD_NUM_VECTORS)
|
||||
|
||||
|
||||
def test_rejected_empty_vector_update_leaves_point_untouched():
|
||||
"""A rejected update_vectors request must not change the point in any way.
|
||||
|
||||
Regression: validation used to run after `deleted_per_vector` was cleared, so rejecting
|
||||
an empty vector still made an absent vector's placeholder visible to search, and a valid
|
||||
vector supplied alongside the rejected one was written anyway.
|
||||
"""
|
||||
local_client = init_local()
|
||||
remote_client = init_remote()
|
||||
|
||||
vectors_config = {
|
||||
"a": models.VectorParams(size=4, distance=models.Distance.COSINE),
|
||||
"b": models.VectorParams(size=4, distance=models.Distance.COSINE),
|
||||
}
|
||||
local_client.create_collection(COLLECTION_NAME, vectors_config=vectors_config)
|
||||
if remote_client.collection_exists(collection_name=COLLECTION_NAME):
|
||||
remote_client.delete_collection(collection_name=COLLECTION_NAME)
|
||||
remote_client.create_collection(COLLECTION_NAME, vectors_config=vectors_config)
|
||||
|
||||
orig = [1.0, 0.0, 0.0, 0.0]
|
||||
other = [0.0, 1.0, 0.0, 0.0]
|
||||
|
||||
# point 1 has no "a" at all, so "a" is backed by a placeholder marked deleted
|
||||
absent_a = [models.PointStruct(id=1, vector={"b": orig})]
|
||||
local_client.upsert(COLLECTION_NAME, absent_a)
|
||||
remote_client.upsert(COLLECTION_NAME, absent_a, wait=True)
|
||||
|
||||
empty_a = [models.PointVectors(id=1, vector={"a": []})]
|
||||
with pytest.raises(ValueError, match="Dense vector must not be empty"):
|
||||
local_client.update_vectors(COLLECTION_NAME, points=empty_a)
|
||||
with pytest.raises(qdrant_client.http.exceptions.UnexpectedResponse):
|
||||
remote_client.update_vectors(COLLECTION_NAME, points=empty_a, wait=True)
|
||||
|
||||
# "a" must still be absent for both clients, and must not be searchable
|
||||
for client in (local_client, remote_client):
|
||||
assert "a" not in client.retrieve(COLLECTION_NAME, ids=[1], with_vectors=True)[0].vector
|
||||
assert client.query_points(COLLECTION_NAME, query=orig, using="a", limit=5).points == []
|
||||
|
||||
# a valid vector supplied next to the rejected one must not be written either
|
||||
mixed = [models.PointVectors(id=1, vector={"b": other, "a": []})]
|
||||
with pytest.raises(ValueError, match="Dense vector must not be empty"):
|
||||
local_client.update_vectors(COLLECTION_NAME, points=mixed)
|
||||
with pytest.raises(qdrant_client.http.exceptions.UnexpectedResponse):
|
||||
remote_client.update_vectors(COLLECTION_NAME, points=mixed, wait=True)
|
||||
|
||||
for client in (local_client, remote_client):
|
||||
assert client.retrieve(COLLECTION_NAME, ids=[1], with_vectors=True)[0].vector["b"] == orig
|
||||
|
||||
compare_collections(local_client, remote_client, UPLOAD_NUM_VECTORS)
|
||||
|
||||
|
||||
def test_upsert_without_vector_name():
|
||||
local_client = init_local()
|
||||
remote_client = init_remote()
|
||||
|
||||
Reference in New Issue
Block a user