Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions qdrant_client/local/local_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -393,6 +412,20 @@ def get_vector_params(self, name: str) -> models.VectorParams:

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:
"""
Expand Down Expand Up @@ -2613,6 +2646,8 @@ def _upsert_point(
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:
Expand All @@ -2627,6 +2662,7 @@ def _upsert_point(
)
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)
Expand Down Expand Up @@ -2718,6 +2754,7 @@ def _update_named_vectors(
self._update_idf_append(new_vector, vector_name)
continue

self._validate_dense_or_multivector(vector, vector_name)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
vector_np = np.array(vector, dtype=np.float32)
assert not np.isnan(vector_np).any(), "Vector contains NaN values"
params = self.get_vector_params(vector_name)
Expand Down
53 changes: 50 additions & 3 deletions tests/congruence_tests/test_multivector_updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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,
Expand Down Expand Up @@ -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
]
Expand Down Expand Up @@ -203,3 +202,51 @@ 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",),
)
59 changes: 59 additions & 0 deletions tests/congruence_tests/test_updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,65 @@ 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_upsert_without_vector_name():
local_client = init_local()
remote_client = init_remote()
Expand Down