Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
58 changes: 51 additions & 7 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 @@ -2703,23 +2739,31 @@ def upsert(
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)
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"
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:
Expand Down
98 changes: 95 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,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",))
110 changes: 110 additions & 0 deletions tests/congruence_tests/test_updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down