From 5fa249bf42b89a8d20c54df1ffebed36c9410f08 Mon Sep 17 00:00:00 2001 From: shashvat-singham Date: Sat, 15 Aug 2026 21:08:27 +0530 Subject: [PATCH 1/5] fix: validate sparse vectors with raises instead of asserts validate_sparse_vector checks user input, but does so with `assert`. python -O strips assert statements, so under -O the checks disappear entirely and a malformed sparse vector is accepted into a local collection: $ python -O >>> client.upsert("t", [PointStruct(id=1, vector={"s": SparseVector( ... indices=[1, 1, 1], values=[1.0, 1.0, 1.0])})]) # accepted The damage surfaces later rather than at the point of the mistake. A vector whose indices and values have different lengths is stored, and a subsequent query raises from deep inside the search path: >>> client.query_points("t", query=SparseVector(indices=[3], values=[1.0]), using="s") IndexError: list index out of range Raise ValueError instead. This also stops user input being reported as an AssertionError, which is inconsistent with the rest of the client. --- qdrant_client/local/sparse.py | 13 ++++++---- .../local/tests/test_sparse_validation.py | 25 +++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 qdrant_client/local/tests/test_sparse_validation.py diff --git a/qdrant_client/local/sparse.py b/qdrant_client/local/sparse.py index 36eed891c..67b16e462 100644 --- a/qdrant_client/local/sparse.py +++ b/qdrant_client/local/sparse.py @@ -11,11 +11,14 @@ def empty_sparse_vector() -> SparseVector: def validate_sparse_vector(vector: SparseVector) -> None: - assert len(vector.indices) == len( - vector.values - ), "Indices and values must have the same length" - assert not np.isnan(vector.values).any(), "Values must not contain NaN" - assert len(vector.indices) == len(set(vector.indices)), "Indices must be unique" + # these validate user input, so they must not be `assert`s: python -O strips those, + # which would let a malformed vector into the collection + if len(vector.indices) != len(vector.values): + raise ValueError("Indices and values must have the same length") + if np.isnan(vector.values).any(): + raise ValueError("Values must not contain NaN") + if len(vector.indices) != len(set(vector.indices)): + raise ValueError("Indices must be unique") def is_sorted(vector: SparseVector) -> bool: diff --git a/qdrant_client/local/tests/test_sparse_validation.py b/qdrant_client/local/tests/test_sparse_validation.py new file mode 100644 index 000000000..b930cddfe --- /dev/null +++ b/qdrant_client/local/tests/test_sparse_validation.py @@ -0,0 +1,25 @@ +import pytest + +from qdrant_client.http.models import SparseVector +from qdrant_client.local.sparse import validate_sparse_vector + + +def test_validate_sparse_vector_accepts_valid() -> None: + validate_sparse_vector(SparseVector(indices=[], values=[])) + validate_sparse_vector(SparseVector(indices=[1, 2, 3], values=[0.1, 0.2, 0.3])) + # indices do not have to be sorted to be valid + validate_sparse_vector(SparseVector(indices=[3, 1], values=[0.1, 0.2])) + + +@pytest.mark.parametrize( + ("vector", "message"), + [ + (SparseVector(indices=[1, 2], values=[0.1]), "same length"), + (SparseVector(indices=[1], values=[float("nan")]), "NaN"), + (SparseVector(indices=[1, 1], values=[0.1, 0.2]), "unique"), + ], +) +def test_validate_sparse_vector_rejects_invalid(vector: SparseVector, message: str) -> None: + # ValueError rather than AssertionError, so the check survives `python -O` + with pytest.raises(ValueError, match=message): + validate_sparse_vector(vector) From a3cfd708974582d6b92744ccf52c83281843d14f Mon Sep 17 00:00:00 2001 From: George Panchuk Date: Fri, 11 Sep 2026 14:49:17 +0700 Subject: [PATCH 2/5] refactor: replace assert error with value error --- qdrant_client/local/async_qdrant_local.py | 5 +-- qdrant_client/local/distances.py | 30 ++++++++----- qdrant_client/local/local_collection.py | 16 ++++--- qdrant_client/local/multi_distances.py | 24 +++++++---- qdrant_client/local/qdrant_local.py | 5 +-- qdrant_client/local/tests/test_distances.py | 42 ++++++++++++++++++- tests/congruence_tests/test_discovery.py | 6 +-- .../test_multivector_discovery_queries.py | 6 +-- .../test_multivector_search_queries.py | 4 +- tests/congruence_tests/test_query.py | 4 +- tests/congruence_tests/test_recommendation.py | 4 +- tests/congruence_tests/test_search.py | 4 +- .../congruence_tests/test_sparse_discovery.py | 2 +- .../congruence_tests/test_sparse_recommend.py | 4 +- tests/congruence_tests/test_sparse_search.py | 2 +- 15 files changed, 109 insertions(+), 49 deletions(-) diff --git a/qdrant_client/local/async_qdrant_local.py b/qdrant_client/local/async_qdrant_local.py index 22687fac5..964cf4537 100644 --- a/qdrant_client/local/async_qdrant_local.py +++ b/qdrant_client/local/async_qdrant_local.py @@ -817,9 +817,8 @@ def uuid_generator() -> Generator[str, None, None]: if isinstance(vectors, dict) and any( (isinstance(v, np.ndarray) for v in vectors.values()) ): - assert ( - len(set([arr.shape[0] for arr in vectors.values()])) == 1 - ), "Each named vector should have the same number of vectors" + if len(set([arr.shape[0] for arr in vectors.values()])) != 1: + raise ValueError("Each named vector should have the same number of vectors") num_vectors = next(iter(vectors.values())).shape[0] vectors = [ {name: vectors[name][i].tolist() for name in vectors.keys()} diff --git a/qdrant_client/local/distances.py b/qdrant_client/local/distances.py index 1c252881f..d09ad3ea7 100644 --- a/qdrant_client/local/distances.py +++ b/qdrant_client/local/distances.py @@ -34,8 +34,10 @@ def __init__( self.positive: list[types.NumpyArray] = [np.array(vector) for vector in positive] self.negative: list[types.NumpyArray] = [np.array(vector) for vector in negative] - assert not np.isnan(self.positive).any(), "Positive vectors must not contain NaN" - assert not np.isnan(self.negative).any(), "Negative vectors must not contain NaN" + if np.isnan(self.positive).any(): + raise ValueError("Positive vectors must not contain NaN") + if np.isnan(self.negative).any(): + raise ValueError("Negative vectors must not contain NaN") class ContextPair: @@ -43,8 +45,10 @@ def __init__(self, positive: list[float], negative: list[float]): self.positive: types.NumpyArray = np.array(positive) self.negative: types.NumpyArray = np.array(negative) - assert not np.isnan(self.positive).any(), "Positive vector must not contain NaN" - assert not np.isnan(self.negative).any(), "Negative vector must not contain NaN" + if np.isnan(self.positive).any(): + raise ValueError("Positive vector must not contain NaN") + if np.isnan(self.negative).any(): + raise ValueError("Negative vector must not contain NaN") class DiscoveryQuery: @@ -52,7 +56,8 @@ def __init__(self, target: list[float], context: list[ContextPair]): self.target: types.NumpyArray = np.array(target) self.context = context - assert not np.isnan(self.target).any(), "Target vector must not contain NaN" + if np.isnan(self.target).any(): + raise ValueError("Target vector must not contain NaN") class ContextQuery: @@ -64,7 +69,8 @@ class FeedbackItem: def __init__(self, vector: list[float], score: float): self.vector = np.array(vector) self.score = score - assert not np.isnan(self.vector).any(), "Feedback vector must not contain NaN" + if np.isnan(self.vector).any(): + raise ValueError("Feedback vector must not contain NaN") class NaiveFeedbackCoefficients: @@ -94,9 +100,11 @@ def __init__( self.feedback = feedback self.coefficients = coefficients - assert not np.isnan(self.target).any(), "Target vector must not contain NaN" + if np.isnan(self.target).any(): + raise ValueError("Target vector must not contain NaN") for item in self.feedback: - assert not np.isnan(item.vector).any(), "Feedback vector must not contain NaN" + if np.isnan(item.vector).any(): + raise ValueError("Feedback vector must not contain NaN") DenseQueryVector: TypeAlias = DiscoveryQuery | ContextQuery | RecoQuery | NaiveFeedbackQuery @@ -192,7 +200,8 @@ def manhattan_distance(query: types.NumpyArray, vectors: types.NumpyArray) -> ty def calculate_distance( query: types.NumpyArray, vectors: types.NumpyArray, distance_type: models.Distance ) -> types.NumpyArray: - assert not np.isnan(query).any(), "Query vector must not contain NaN" + if np.isnan(query).any(): + raise ValueError("Query vector must not contain NaN") if distance_type == models.Distance.COSINE: return cosine_similarity(query, vectors) @@ -212,7 +221,8 @@ def calculate_distance_core( """ Calculate same internal distances as in core, rather than the final displayed distance """ - assert not np.isnan(query).any(), "Query vector must not contain NaN" + if np.isnan(query).any(): + raise ValueError("Query vector must not contain NaN") if distance_type == models.Distance.EUCLID: return -np.square(vectors - query, dtype=np.float32).sum(axis=1, dtype=np.float32) diff --git a/qdrant_client/local/local_collection.py b/qdrant_client/local/local_collection.py index b8bc7cdd6..acb28d701 100644 --- a/qdrant_client/local/local_collection.py +++ b/qdrant_client/local/local_collection.py @@ -2496,7 +2496,8 @@ def _update_point(self, point: models.PointStruct) -> None: vector = vectors.get(vector_name) if vector is not None: params = self.get_vector_params(vector_name) - assert not np.isnan(vector).any(), "Vector contains NaN values" + if np.isnan(vector).any(): + raise ValueError("Vector contains NaN values") if params.distance == models.Distance.COSINE: norm = np.linalg.norm(vector) vector = np.array(vector) / norm if norm > EPSILON else vector @@ -2525,7 +2526,8 @@ def _update_point(self, point: models.PointStruct) -> None: vector = vectors.get(vector_name) if vector is not None: params = self.get_vector_params(vector_name) - assert not np.isnan(vector).any(), "Vector contains NaN values" + if np.isnan(vector).any(): + raise ValueError("Vector contains NaN values") if params.distance == models.Distance.COSINE: vector_norm = np.linalg.norm(vector, axis=-1)[:, np.newaxis] @@ -2572,7 +2574,8 @@ def _add_point(self, point: models.PointStruct) -> None: ) else: vector_np = np.array(vector, dtype=np.float32) - assert not np.isnan(vector_np).any(), "Vector contains NaN values" + if np.isnan(vector_np).any(): + raise ValueError("Vector contains NaN values") params = self.get_vector_params(vector_name) if params.distance == models.Distance.COSINE: norm = np.linalg.norm(vector_np) @@ -2628,7 +2631,8 @@ def _add_point(self, point: models.PointStruct) -> None: ) else: vector_np = np.array(vector, dtype=np.float32) - assert not np.isnan(vector_np).any(), "Vector contains NaN values" + if np.isnan(vector_np).any(): + raise ValueError("Vector contains NaN values") params = self.get_vector_params(vector_name) if params.distance == models.Distance.COSINE: vector_norm = np.linalg.norm(vector_np, axis=-1)[:, np.newaxis] @@ -2768,7 +2772,9 @@ def _update_named_vectors( 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" + + if np.isnan(vector_np).any(): + raise ValueError("Vector contains NaN values") validated.append((vector_name, vector_np)) for vector_name, vector_np in validated: diff --git a/qdrant_client/local/multi_distances.py b/qdrant_client/local/multi_distances.py index 5c62aca56..cbaeea75a 100644 --- a/qdrant_client/local/multi_distances.py +++ b/qdrant_client/local/multi_distances.py @@ -27,9 +27,11 @@ def __init__( negative = negative if negative is not None else [] for vector in positive: - assert not np.isnan(vector).any(), "Positive vectors must not contain NaN" + if np.isnan(vector).any(): + raise ValueError("Positive vectors must not contain NaN") for vector in negative: - assert not np.isnan(vector).any(), "Negative vectors must not contain NaN" + if np.isnan(vector).any(): + raise ValueError("Negative vectors must not contain NaN") self.positive: list[types.NumpyArray] = [np.array(vector) for vector in positive] self.negative: list[types.NumpyArray] = [np.array(vector) for vector in negative] @@ -40,8 +42,10 @@ def __init__(self, positive: list[list[float]], negative: list[list[float]]): self.positive: types.NumpyArray = np.array(positive) self.negative: types.NumpyArray = np.array(negative) - assert not np.isnan(self.positive).any(), "Positive vector must not contain NaN" - assert not np.isnan(self.negative).any(), "Negative vector must not contain NaN" + if np.isnan(self.positive).any(): + raise ValueError("Positive vector must not contain NaN") + if np.isnan(self.negative).any(): + raise ValueError("Negative vector must not contain NaN") class MultiDiscoveryQuery: @@ -49,7 +53,8 @@ def __init__(self, target: list[list[float]], context: list[MultiContextPair]): self.target: types.NumpyArray = np.array(target) self.context = context - assert not np.isnan(self.target).any(), "Target vector must not contain NaN" + if np.isnan(self.target).any(): + raise ValueError("Target vector must not contain NaN") class MultiContextQuery: @@ -65,8 +70,10 @@ def calculate_multi_distance( matrices: list[types.NumpyArray], distance_type: models.Distance, ) -> types.NumpyArray: - assert not np.isnan(query_matrix).any(), "Query matrix must not contain NaN" - assert len(query_matrix.shape) == 2, "Query must be a matrix" + if np.isnan(query_matrix).any(): + raise ValueError("Query matrix must not contain NaN") + if len(query_matrix.shape) != 2: + raise ValueError("Query must be a matrix") distances = calculate_multi_distance_core(query_matrix, matrices, distance_type) @@ -88,7 +95,8 @@ def euclidean(q: types.NumpyArray, m: types.NumpyArray, *_: Any) -> types.NumpyA def manhattan(q: types.NumpyArray, m: types.NumpyArray, *_: Any) -> types.NumpyArray: return -np.abs(m - q, dtype=np.float32).sum(axis=-1, dtype=np.float32) - assert not np.isnan(query_matrix).any(), "Query vector must not contain NaN" + if np.isnan(query_matrix).any(): + raise ValueError("Query vector must not contain NaN") similarities: list[float] = [] # Euclid and Manhattan are the only ones which are calculated differently during candidate selection diff --git a/qdrant_client/local/qdrant_local.py b/qdrant_client/local/qdrant_local.py index b4c7835ec..00ba05125 100644 --- a/qdrant_client/local/qdrant_local.py +++ b/qdrant_client/local/qdrant_local.py @@ -892,9 +892,8 @@ def uuid_generator() -> Generator[str, None, None]: collection = self._get_collection(collection_name) if isinstance(vectors, dict) and any(isinstance(v, np.ndarray) for v in vectors.values()): - assert ( - len(set([arr.shape[0] for arr in vectors.values()])) == 1 - ), "Each named vector should have the same number of vectors" + if len(set([arr.shape[0] for arr in vectors.values()])) != 1: + raise ValueError("Each named vector should have the same number of vectors") num_vectors = next(iter(vectors.values())).shape[0] # convert dict[str, np.ndarray] to list[dict[str, list[float]]] diff --git a/qdrant_client/local/tests/test_distances.py b/qdrant_client/local/tests/test_distances.py index cb4bdff5b..2d16d6fe6 100644 --- a/qdrant_client/local/tests/test_distances.py +++ b/qdrant_client/local/tests/test_distances.py @@ -1,8 +1,18 @@ import numpy as np +import pytest from qdrant_client.http import models -from qdrant_client.local.distances import calculate_distance -from qdrant_client.local.multi_distances import calculate_multi_distance +from qdrant_client.local.distances import ( + ContextPair, + DiscoveryQuery, + RecoQuery, + calculate_distance, +) +from qdrant_client.local.multi_distances import ( + MultiDiscoveryQuery, + MultiRecoQuery, + calculate_multi_distance, +) from qdrant_client.local.sparse_distances import calculate_distance_sparse @@ -91,3 +101,31 @@ def test_cosine_accepts_integer_dtype_query() -> None: vectors = np.array([[6.0, 8.0], [1.0, 0.0]], dtype=np.float32) result = calculate_distance(query, vectors, models.Distance.COSINE) assert np.allclose(result, [1.0, 0.6], atol=0.0001) + + +def test_nan_rejected() -> None: + nan_vector = [1.0, float("nan"), 3.0] + vectors = np.array([[1.0, 2.0, 3.0]]) + + with pytest.raises(ValueError, match="Query vector must not contain NaN"): + calculate_distance(np.array(nan_vector), vectors, models.Distance.DOT) + + with pytest.raises(ValueError, match="Query matrix must not contain NaN"): + calculate_multi_distance( + np.array([nan_vector]), [np.array([[1.0, 2.0, 3.0]])], models.Distance.DOT + ) + + with pytest.raises(ValueError, match="Positive vectors must not contain NaN"): + RecoQuery(positive=[nan_vector], strategy=models.RecommendStrategy.BEST_SCORE) + + with pytest.raises(ValueError, match="Positive vector must not contain NaN"): + ContextPair(positive=nan_vector, negative=[1.0, 2.0, 3.0]) + + with pytest.raises(ValueError, match="Target vector must not contain NaN"): + DiscoveryQuery(target=nan_vector, context=[]) + + with pytest.raises(ValueError, match="Positive vectors must not contain NaN"): + MultiRecoQuery(positive=[[nan_vector]], strategy=models.RecommendStrategy.BEST_SCORE) + + with pytest.raises(ValueError, match="Target vector must not contain NaN"): + MultiDiscoveryQuery(target=[nan_vector], context=[]) diff --git a/tests/congruence_tests/test_discovery.py b/tests/congruence_tests/test_discovery.py index 9c89407e6..b8198490c 100644 --- a/tests/congruence_tests/test_discovery.py +++ b/tests/congruence_tests/test_discovery.py @@ -426,7 +426,7 @@ def test_query_with_nan(): init_client(local_client, fixture_points) init_client(remote_client, fixture_points) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=models.DiscoverQuery(discover=models.DiscoverInput(target=vector, context=[])), @@ -438,7 +438,7 @@ def test_query_with_nan(): query=models.DiscoverQuery(discover=models.DiscoverInput(target=vector, context=[])), using=using, ) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=models.ContextQuery(context=models.ContextPair(positive=vector, negative=1)), @@ -450,7 +450,7 @@ def test_query_with_nan(): query=models.ContextQuery(context=models.ContextPair(positive=vector, negative=1)), using=using, ) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=models.ContextQuery(context=models.ContextPair(positive=1, negative=vector)), diff --git a/tests/congruence_tests/test_multivector_discovery_queries.py b/tests/congruence_tests/test_multivector_discovery_queries.py index bde112362..22b63c800 100644 --- a/tests/congruence_tests/test_multivector_discovery_queries.py +++ b/tests/congruence_tests/test_multivector_discovery_queries.py @@ -426,7 +426,7 @@ def test_query_with_nan(): init_client(local_client, fixture_points, vectors_config=multi_vector_config) init_client(remote_client, fixture_points, vectors_config=multi_vector_config) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=models.DiscoverQuery(discover=models.DiscoverInput(target=vector, context=[])), @@ -438,7 +438,7 @@ def test_query_with_nan(): query=models.DiscoverQuery(discover=models.DiscoverInput(target=vector, context=[])), using=using, ) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=models.ContextQuery(context=[models.ContextPair(positive=vector, negative=1)]), @@ -450,7 +450,7 @@ def test_query_with_nan(): query=models.ContextQuery(context=[models.ContextPair(positive=1, negative=vector)]), using=using, ) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=models.ContextQuery(context=[models.ContextPair(positive=vector, negative=1)]), diff --git a/tests/congruence_tests/test_multivector_search_queries.py b/tests/congruence_tests/test_multivector_search_queries.py index 9e5373a86..d9c2a2507 100644 --- a/tests/congruence_tests/test_multivector_search_queries.py +++ b/tests/congruence_tests/test_multivector_search_queries.py @@ -273,7 +273,7 @@ def test_query_with_nan(): vector = generate_random_multivector(text_vector_size, 10) vector[0][4] = np.nan - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points(COLLECTION_NAME, query=vector, using="multi-text") with pytest.raises(UnexpectedResponse): remote_client.query_points(COLLECTION_NAME, query=vector, using="multi-text") @@ -296,7 +296,7 @@ def test_query_with_nan(): init_client(local_client, fixture_points, vectors_config=single_multi_vector_config) init_client(remote_client, fixture_points, vectors_config=single_multi_vector_config) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points(COLLECTION_NAME, query=vector) with pytest.raises(UnexpectedResponse): remote_client.query_points(COLLECTION_NAME, query=vector) diff --git a/tests/congruence_tests/test_query.py b/tests/congruence_tests/test_query.py index e581fa44e..3dd50775e 100644 --- a/tests/congruence_tests/test_query.py +++ b/tests/congruence_tests/test_query.py @@ -1960,7 +1960,7 @@ def test_query_with_nan(): vector = np.random.random(text_vector_size) vector[4] = np.nan query = vector.tolist() - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points(COLLECTION_NAME, query=query, using="text") with pytest.raises(UnexpectedResponse): @@ -1984,7 +1984,7 @@ def test_query_with_nan(): init_client(local_client, fixture_points, vectors_config=single_vector_config) init_client(http_client, fixture_points, vectors_config=single_vector_config) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): print(local_client.query_points(COLLECTION_NAME, query=query)) with pytest.raises(UnexpectedResponse): diff --git a/tests/congruence_tests/test_recommendation.py b/tests/congruence_tests/test_recommendation.py index 44403fe6d..752d25407 100644 --- a/tests/congruence_tests/test_recommendation.py +++ b/tests/congruence_tests/test_recommendation.py @@ -392,7 +392,7 @@ def test_query_with_nan(): init_client(local_client, fixture_points) init_client(remote_client, fixture_points) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=models.RecommendQuery( @@ -410,7 +410,7 @@ def test_query_with_nan(): using=using, ) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=models.RecommendQuery( diff --git a/tests/congruence_tests/test_search.py b/tests/congruence_tests/test_search.py index 427a7429a..b0b815e06 100644 --- a/tests/congruence_tests/test_search.py +++ b/tests/congruence_tests/test_search.py @@ -362,7 +362,7 @@ def test_query_with_nan(): vector[4] = np.nan query_vector = vector.tolist() - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points(COLLECTION_NAME, query_vector, using="text") with pytest.raises(UnexpectedResponse): remote_client.query_points(COLLECTION_NAME, query_vector, using="text") @@ -381,7 +381,7 @@ def test_query_with_nan(): init_client(local_client, fixture_points, vectors_config=single_vector_config) init_client(remote_client, fixture_points, vectors_config=single_vector_config) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points(COLLECTION_NAME, vector.tolist()) with pytest.raises(UnexpectedResponse): remote_client.query_points(COLLECTION_NAME, vector.tolist()) diff --git a/tests/congruence_tests/test_sparse_discovery.py b/tests/congruence_tests/test_sparse_discovery.py index d53ff1636..6a20624d4 100644 --- a/tests/congruence_tests/test_sparse_discovery.py +++ b/tests/congruence_tests/test_sparse_discovery.py @@ -354,7 +354,7 @@ def test_query_with_nan(): ) else: query = models.ContextQuery(context=models.ContextPair(positive=pos, negative=neg)) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=query, diff --git a/tests/congruence_tests/test_sparse_recommend.py b/tests/congruence_tests/test_sparse_recommend.py index d5fff3553..95d026e4f 100644 --- a/tests/congruence_tests/test_sparse_recommend.py +++ b/tests/congruence_tests/test_sparse_recommend.py @@ -344,7 +344,7 @@ def test_query_with_nan(): sparse_vectors_config=sparse_vectors_config, ) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=models.RecommendQuery( @@ -362,7 +362,7 @@ def test_query_with_nan(): using=using, ) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( collection_name=COLLECTION_NAME, query=models.RecommendQuery( diff --git a/tests/congruence_tests/test_sparse_search.py b/tests/congruence_tests/test_sparse_search.py index 3c4a48a9d..8055fc3f2 100644 --- a/tests/congruence_tests/test_sparse_search.py +++ b/tests/congruence_tests/test_sparse_search.py @@ -350,7 +350,7 @@ def test_query_with_nan(): sparse_vectors_config=sparse_vectors_config, ) - with pytest.raises(AssertionError): + with pytest.raises(ValueError): local_client.query_points( COLLECTION_NAME, sparse_vector["sparse-text"], using="sparse-text" ) From 5fdee9817c6369fe4c4e3bb32bc4cfbcb345dc32 Mon Sep 17 00:00:00 2001 From: George Panchuk Date: Fri, 11 Sep 2026 15:55:01 +0700 Subject: [PATCH 3/5] fix: validate vectors before write --- qdrant_client/local/local_collection.py | 83 +++++++++-------- .../local/tests/test_write_atomicity.py | 90 +++++++++++++++++++ tests/congruence_tests/test_updates.py | 41 +++++++++ 3 files changed, 175 insertions(+), 39 deletions(-) create mode 100644 qdrant_client/local/tests/test_write_atomicity.py diff --git a/qdrant_client/local/local_collection.py b/qdrant_client/local/local_collection.py index acb28d701..cd1e77287 100644 --- a/qdrant_client/local/local_collection.py +++ b/qdrant_client/local/local_collection.py @@ -101,13 +101,16 @@ def to_jsonable_python(x: Any) -> Any: def validate_dense_vector(vector: Any, vector_name: str) -> None: - """Reject empty dense vectors, as the server does at write time.""" + """Reject empty dense vectors and NaN values, 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}'") + if np.isnan(np.asarray(vector, dtype=np.float32)).any(): + raise ValueError("Vector contains NaN values") + def validate_multivector(vector: Any, vector_name: str) -> None: - """Reject empty multivectors and multivectors holding empty vectors, as the server does.""" + """Reject empty multivectors, empty sub-vectors and NaN values, as the server does.""" if len(vector) == 0: raise ValueError(f"Wrong input: Multivector must not be empty for vector '{vector_name}'") @@ -118,6 +121,9 @@ def validate_multivector(vector: Any, vector_name: str) -> None: f"for vector '{vector_name}'" ) + if np.isnan(np.asarray(vector, dtype=np.float32)).any(): + raise ValueError("Vector contains NaN values") + class LocalCollection: """ @@ -2496,8 +2502,6 @@ def _update_point(self, point: models.PointStruct) -> None: vector = vectors.get(vector_name) if vector is not None: params = self.get_vector_params(vector_name) - if np.isnan(vector).any(): - raise ValueError("Vector contains NaN values") if params.distance == models.Distance.COSINE: norm = np.linalg.norm(vector) vector = np.array(vector) / norm if norm > EPSILON else vector @@ -2526,9 +2530,6 @@ def _update_point(self, point: models.PointStruct) -> None: vector = vectors.get(vector_name) if vector is not None: params = self.get_vector_params(vector_name) - if np.isnan(vector).any(): - raise ValueError("Vector contains NaN values") - if params.distance == models.Distance.COSINE: vector_norm = np.linalg.norm(vector, axis=-1)[:, np.newaxis] vector /= np.where(vector_norm != 0.0, vector_norm, EPSILON) @@ -2574,8 +2575,6 @@ def _add_point(self, point: models.PointStruct) -> None: ) else: vector_np = np.array(vector, dtype=np.float32) - if np.isnan(vector_np).any(): - raise ValueError("Vector contains NaN values") params = self.get_vector_params(vector_name) if params.distance == models.Distance.COSINE: norm = np.linalg.norm(vector_np) @@ -2631,8 +2630,6 @@ def _add_point(self, point: models.PointStruct) -> None: ) else: vector_np = np.array(vector, dtype=np.float32) - if np.isnan(vector_np).any(): - raise ValueError("Vector contains NaN values") params = self.get_vector_params(vector_name) if params.distance == models.Distance.COSINE: vector_norm = np.linalg.norm(vector_np, axis=-1)[:, np.newaxis] @@ -2644,12 +2641,12 @@ def _add_point(self, point: models.PointStruct) -> None: self.multivectors[vector_name] = named_vectors - def _upsert_point( - self, - point: models.PointStruct, - update_filter: types.Filter | None = None, - update_mode: types.UpdateMode | None = None, - ) -> None: + def _validate_point(self, point: models.PointStruct) -> None: + """Validate a point and normalize its sparse vectors, without touching collection state. + + Every write path runs this over all of its points before applying any of them, so a + rejected point leaves the collection untouched, the way the server does. + """ if isinstance(point.id, str): # try to parse as UUID try: @@ -2685,6 +2682,13 @@ def _upsert_point( raise ValueError("Wrong input: Not existing vector name error") self._validate_dense_or_multivector(point.vector, DEFAULT_VECTOR_NAME) + def _upsert_point( + self, + point: models.PointStruct, + update_filter: types.Filter | None = None, + update_mode: types.UpdateMode | None = None, + ) -> None: + """Apply an already validated point. Call `_validate_point` first.""" if isinstance(point.id, uuid.UUID): point.id = str(point.id) @@ -2719,8 +2723,7 @@ def upsert( validate_filter(update_filter) if isinstance(points, list): - for point in points: - self._upsert_point(point, update_filter=update_filter, update_mode=update_mode) + point_structs = list(points) elif isinstance(points, models.Batch): batch = points if isinstance(batch.vectors, list): @@ -2728,25 +2731,25 @@ def upsert( else: vectors = batch.vectors - for idx, point_id in enumerate(batch.ids): - payload = None - if batch.payloads is not None: - payload = batch.payloads[idx] - - vector = {name: v[idx] for name, v in vectors.items()} - - self._upsert_point( - models.PointStruct( - id=point_id, - payload=payload, - vector=vector, - ), - update_filter=update_filter, - update_mode=update_mode, + point_structs = [ + models.PointStruct( + id=point_id, + payload=batch.payloads[idx] if batch.payloads is not None else None, + vector={name: v[idx] for name, v in vectors.items()}, ) + for idx, point_id in enumerate(batch.ids) + ] else: raise ValueError(f"Unsupported type: {type(points)}") + # Validate everything before writing anything: the server rejects the whole request, + # so a bad point in the middle of a batch must not leave the earlier ones applied. + for point in point_structs: + self._validate_point(point) + + for point in point_structs: + self._upsert_point(point, update_filter=update_filter, update_mode=update_mode) + if len(self.ids) > self.LARGE_DATA_THRESHOLD: show_warning_once( f"Local mode is not recommended for collections with more than {self.LARGE_DATA_THRESHOLD:,} " @@ -2771,11 +2774,7 @@ def _update_named_vectors( continue self._validate_dense_or_multivector(vector, vector_name) - vector_np = np.array(vector, dtype=np.float32) - - if np.isnan(vector_np).any(): - raise ValueError("Vector contains NaN values") - validated.append((vector_name, vector_np)) + validated.append((vector_name, np.array(vector, dtype=np.float32))) for vector_name, vector_np in validated: self.deleted_per_vector[vector_name][idx] = 0 @@ -2835,6 +2834,12 @@ def delete_vectors( | models.PointIdsList ), ) -> None: + # Check every name up front: the server rejects the whole request on an unknown + # vector name, rather than deleting the names it recognizes first. + for vector_name in vectors: + if vector_name not in self._all_vectors_keys: + raise ValueError(f"Wrong input: Not existing vector name error: {vector_name}") + ids = self._selector_to_ids(selector) for point_id in ids: idx = self.ids[point_id] diff --git a/qdrant_client/local/tests/test_write_atomicity.py b/qdrant_client/local/tests/test_write_atomicity.py new file mode 100644 index 000000000..a9fd4f3b4 --- /dev/null +++ b/qdrant_client/local/tests/test_write_atomicity.py @@ -0,0 +1,90 @@ +"""A rejected write must not leave the collection's internal arrays skewed. + +The observable half of this — local and remote agreeing after a refused write — lives in +`tests/congruence_tests/test_updates.py`. What can only be checked from the inside is that +`payload`, `deleted` and `deleted_per_vector` stay aligned with `ids_inv`. They used to +drift whenever validation ran after the point had already been added, and the next +successful upsert then broke every query with a shape mismatch. +""" + +import pytest + +from qdrant_client import models +from qdrant_client.local.local_collection import LocalCollection + +NAN_VECTOR = [1.0, float("nan"), 3.0] +GOOD_VECTOR = [1.0, 2.0, 3.0] + + +def assert_internally_consistent(collection: LocalCollection) -> None: + assert len(collection.ids) == len(collection.ids_inv) + assert len(collection.payload) == len(collection.ids_inv) + assert len(collection.deleted) == len(collection.ids_inv) + for vector_name, deleted in collection.deleted_per_vector.items(): + assert len(deleted) == len(collection.ids_inv), vector_name + + +def test_rejected_add_keeps_internal_arrays_aligned() -> None: + collection = LocalCollection( + models.CreateCollection( + vectors={"d": models.VectorParams(size=3, distance=models.Distance.DOT)} + ) + ) + collection.upsert([models.PointStruct(id=1, vector={"d": GOOD_VECTOR})]) + + with pytest.raises(ValueError, match="Vector contains NaN values"): + collection.upsert([models.PointStruct(id=2, vector={"d": NAN_VECTOR})]) + + assert_internally_consistent(collection) + + # the skew used to surface only here, once a healthy point arrived + collection.upsert([models.PointStruct(id=3, vector={"d": [1.0, 1.0, 1.0]})]) + assert len(collection.search(query_vector=("d", GOOD_VECTOR), limit=10)) == 2 + + +def test_rejected_multivector_add_keeps_internal_arrays_aligned() -> None: + collection = LocalCollection( + models.CreateCollection( + vectors={ + "m": models.VectorParams( + size=3, + distance=models.Distance.DOT, + multivector_config=models.MultiVectorConfig( + comparator=models.MultiVectorComparator.MAX_SIM + ), + ) + } + ) + ) + collection.upsert([models.PointStruct(id=1, vector={"m": [GOOD_VECTOR]})]) + + with pytest.raises(ValueError, match="Vector contains NaN values"): + collection.upsert([models.PointStruct(id=2, vector={"m": [NAN_VECTOR]})]) + + assert_internally_consistent(collection) + + collection.upsert([models.PointStruct(id=3, vector={"m": [[1.0, 1.0, 1.0]]})]) + assert len(collection.search(query_vector=("m", [GOOD_VECTOR]), limit=10)) == 2 + + +def test_rejected_delete_vectors_deletes_nothing() -> None: + """An unknown vector name must not take the recognized names down with it. + + This one stays local-only: the server always rejects the request, but whether it + deletes the name it recognized before noticing the unknown one varies from run to run, + so there is no server behavior for a congruence test to pin down. + """ + collection = LocalCollection( + models.CreateCollection( + vectors={ + "a": models.VectorParams(size=3, distance=models.Distance.DOT), + "b": models.VectorParams(size=3, distance=models.Distance.DOT), + } + ) + ) + collection.upsert([models.PointStruct(id=1, vector={"a": GOOD_VECTOR, "b": GOOD_VECTOR})]) + + with pytest.raises(ValueError, match="Not existing vector name error: nope"): + collection.delete_vectors(vectors=["a", "nope"], selector=[1]) + + assert sorted(collection._get_vectors(idx=0, with_vectors=True)) == ["a", "b"] diff --git a/tests/congruence_tests/test_updates.py b/tests/congruence_tests/test_updates.py index 0185ffaf1..0d96af04a 100644 --- a/tests/congruence_tests/test_updates.py +++ b/tests/congruence_tests/test_updates.py @@ -985,3 +985,44 @@ def upload( assert np.allclose(local_points[0].vector, remote_points[0].vector) assert np.allclose(local_points[0].vector, first_point.vector) assert len(local_points) == len(remote_points) == 1 + + +def nan_vectors(point: models.PointStruct) -> dict: + """A copy of the point's vectors with one NaN slipped into the dense 'text' vector.""" + vectors = deepcopy(point.vector) + vectors["text"] = list(vectors["text"]) + vectors["text"][0] = float("nan") + return vectors + + +def test_rejected_upsert_leaves_the_collection_untouched(local_client, remote_client): + """A write the client refuses must not change anything, the way the server does. + + Local mode used to validate after mutating, so a NaN vector left a half-added point + behind and skewed the per-vector arrays, which broke every later query. + """ + points = generate_fixtures(UPLOAD_NUM_VECTORS) + local_client.upload_points(COLLECTION_NAME, points, wait=True) + remote_client.upload_points(COLLECTION_NAME, points, wait=True) + + new_point = models.PointStruct( + id=UPLOAD_NUM_VECTORS + 1, vector=nan_vectors(points[0]), payload={"a": "new"} + ) + existing_point = models.PointStruct( + id=points[0].id, vector=nan_vectors(points[0]), payload={"a": "clobbered"} + ) + healthy_point = models.PointStruct( + id=UPLOAD_NUM_VECTORS + 2, vector=deepcopy(points[1].vector) + ) + + for batch in ( + [new_point], # rejected insert + [existing_point], # rejected update + [healthy_point, new_point], # rejected batch, healthy point first + ): + with pytest.raises(ValueError): + local_client.upsert(COLLECTION_NAME, batch) + with pytest.raises(qdrant_client.http.exceptions.UnexpectedResponse): + remote_client.upsert(COLLECTION_NAME, batch, wait=True) + + compare_collections(local_client, remote_client, UPLOAD_NUM_VECTORS) From 55063adb05d8dc0f939573e5f8a7e7ad1f466a36 Mon Sep 17 00:00:00 2001 From: George Panchuk Date: Fri, 11 Sep 2026 17:22:05 +0700 Subject: [PATCH 4/5] fix: add validation for update vectors and batch update points --- qdrant_client/local/local_collection.py | 95 +++++++++++++++++++------ tests/congruence_tests/test_updates.py | 47 ++++++++++++ 2 files changed, 119 insertions(+), 23 deletions(-) diff --git a/qdrant_client/local/local_collection.py b/qdrant_client/local/local_collection.py index cd1e77287..89cde31ee 100644 --- a/qdrant_client/local/local_collection.py +++ b/qdrant_client/local/local_collection.py @@ -2714,24 +2714,22 @@ def _upsert_point( if self.storage is not None: self.storage.persist(point) - def upsert( - self, + @staticmethod + def _materialize_points( points: Sequence[models.PointStruct] | models.Batch, - update_filter: types.Filter | None = None, - update_mode: types.UpdateMode | None = None, - ) -> None: - validate_filter(update_filter) - + ) -> list[models.PointStruct]: + """Flatten either accepted upsert shape into a plain list of points.""" if isinstance(points, list): - point_structs = list(points) - elif isinstance(points, models.Batch): + return list(points) + + if isinstance(points, models.Batch): batch = points if isinstance(batch.vectors, list): vectors = {DEFAULT_VECTOR_NAME: batch.vectors} else: vectors = batch.vectors - point_structs = [ + return [ models.PointStruct( id=point_id, payload=batch.payloads[idx] if batch.payloads is not None else None, @@ -2739,8 +2737,18 @@ def upsert( ) for idx, point_id in enumerate(batch.ids) ] - else: - raise ValueError(f"Unsupported type: {type(points)}") + + raise ValueError(f"Unsupported type: {type(points)}") + + def upsert( + self, + points: Sequence[models.PointStruct] | models.Batch, + update_filter: types.Filter | None = None, + update_mode: types.UpdateMode | None = None, + ) -> None: + validate_filter(update_filter) + + point_structs = self._materialize_points(points) # Validate everything before writing anything: the server rejects the whole request, # so a bad point in the middle of a batch must not leave the earlier ones applied. @@ -2760,9 +2768,10 @@ def upsert( stacklevel=6, ) - def _update_named_vectors( - self, idx: int, vectors: dict[str, list[float] | SparseVector | list[list[float]]] - ) -> None: + def _validate_named_vectors( + self, vectors: dict[str, list[float] | SparseVector | list[list[float]]] + ) -> list[tuple[str, Any]]: + """Validate and normalize named vectors, without touching collection state.""" validated: list[tuple[str, Any]] = [] for vector_name, vector in vectors.items(): if vector_name not in self._all_vectors_keys: @@ -2776,6 +2785,10 @@ def _update_named_vectors( self._validate_dense_or_multivector(vector, vector_name) validated.append((vector_name, np.array(vector, dtype=np.float32))) + return validated + + def _apply_named_vectors(self, idx: int, validated: list[tuple[str, Any]]) -> None: + """Apply already validated named vectors. Call `_validate_named_vectors` first.""" for vector_name, vector_np in validated: self.deleted_per_vector[vector_name][idx] = 0 @@ -2803,14 +2816,23 @@ def update_vectors( ) -> None: validate_filter(update_filter) - for point in points: - point_id = str(point.id) if isinstance(point.id, uuid.UUID) else point.id + # Same rule as upsert: validate every point in the request before writing any of it. + # The point id itself is looked up in the apply pass, because the server does apply + # the points preceding an unknown id before answering 404. + prepared = [ + ( + str(point.id) if isinstance(point.id, uuid.UUID) else point.id, + self._validate_named_vectors( + {DEFAULT_VECTOR_NAME: point.vector} + if isinstance(point.vector, list) + else point.vector + ), + ) + for point in points + ] + + for point_id, validated in prepared: idx = self.ids[point_id] - vector_struct = point.vector - if isinstance(vector_struct, list): - fixed_vectors = {DEFAULT_VECTOR_NAME: vector_struct} - else: - fixed_vectors = vector_struct if not self.deleted[idx] and update_filter is not None: has_vector = {} @@ -2821,7 +2843,7 @@ def update_vectors( update_filter, self.payload[idx], self.ids_inv[idx], has_vector ): continue - self._update_named_vectors(idx, fixed_vectors) + self._apply_named_vectors(idx, validated) self._persist_by_id(point_id) def delete_vectors( @@ -2984,10 +3006,37 @@ def clear_payload( self.payload[idx] = {} self._persist_by_id(point_id) + def _validate_update_operation(self, update_op: types.UpdateOperation) -> None: + """Validate the vectors an operation carries, without touching collection state.""" + if isinstance(update_op, models.UpsertOperation): + upsert_struct = update_op.upsert + if isinstance(upsert_struct, models.PointsBatch): + points: Sequence[models.PointStruct] | models.Batch = upsert_struct.batch + elif isinstance(upsert_struct, models.PointsList): + points = upsert_struct.points + else: + raise ValueError(f"Unsupported upsert type: {type(update_op.upsert)}") + + for point in self._materialize_points(points): + self._validate_point(point) + + elif isinstance(update_op, models.UpdateVectorsOperation): + for point in update_op.update_vectors.points: + self._validate_named_vectors( + {DEFAULT_VECTOR_NAME: point.vector} + if isinstance(point.vector, list) + else point.vector + ) + def batch_update_points( self, update_operations: Sequence[types.UpdateOperation], ) -> None: + # The server rejects the whole request, so one bad operation must not leave the + # operations before it applied. + for update_op in update_operations: + self._validate_update_operation(update_op) + for update_op in update_operations: if isinstance(update_op, models.UpsertOperation): upsert_struct = update_op.upsert diff --git a/tests/congruence_tests/test_updates.py b/tests/congruence_tests/test_updates.py index 0d96af04a..5d23fb5c5 100644 --- a/tests/congruence_tests/test_updates.py +++ b/tests/congruence_tests/test_updates.py @@ -1026,3 +1026,50 @@ def test_rejected_upsert_leaves_the_collection_untouched(local_client, remote_cl remote_client.upsert(COLLECTION_NAME, batch, wait=True) compare_collections(local_client, remote_client, UPLOAD_NUM_VECTORS) + + +def test_rejected_update_vectors_leaves_the_collection_untouched(local_client, remote_client): + """A rejected point must not take the valid points sent alongside it down with it.""" + points = generate_fixtures(UPLOAD_NUM_VECTORS) + local_client.upload_points(COLLECTION_NAME, points, wait=True) + remote_client.upload_points(COLLECTION_NAME, points, wait=True) + + replacement = list(points[1].vector["text"]) + for bad_vector in (nan_vectors(points[0])["text"], []): + batch = [ + models.PointVectors(id=points[0].id, vector={"text": replacement}), # valid, first + models.PointVectors(id=points[1].id, vector={"text": bad_vector}), # rejected + ] + with pytest.raises(ValueError): + local_client.update_vectors(COLLECTION_NAME, points=batch) + with pytest.raises(qdrant_client.http.exceptions.UnexpectedResponse): + remote_client.update_vectors(COLLECTION_NAME, points=batch, wait=True) + + compare_collections(local_client, remote_client, UPLOAD_NUM_VECTORS) + + +def test_rejected_batch_update_leaves_the_collection_untouched(local_client, remote_client): + """One bad operation must not leave the operations before it applied.""" + points = generate_fixtures(UPLOAD_NUM_VECTORS) + local_client.upload_points(COLLECTION_NAME, points, wait=True) + remote_client.upload_points(COLLECTION_NAME, points, wait=True) + + operations = [ + models.SetPayloadOperation( + set_payload=models.SetPayload(payload={"a": "changed"}, points=[points[0].id]) + ), + models.UpsertOperation( + upsert=models.PointsList( + points=[ + models.PointStruct(id=UPLOAD_NUM_VECTORS + 1, vector=nan_vectors(points[0])) + ] + ) + ), + ] + + with pytest.raises(ValueError): + local_client.batch_update_points(COLLECTION_NAME, update_operations=operations) + with pytest.raises(qdrant_client.http.exceptions.UnexpectedResponse): + remote_client.batch_update_points(COLLECTION_NAME, update_operations=operations, wait=True) + + compare_collections(local_client, remote_client, UPLOAD_NUM_VECTORS) From b70ed91855762a6f296bb3261753866768a9159b Mon Sep 17 00:00:00 2001 From: George Panchuk Date: Fri, 11 Sep 2026 18:45:33 +0700 Subject: [PATCH 5/5] fix: validate vector dimensions and batch arguments before write --- qdrant_client/local/local_collection.py | 53 ++++++++++-- .../local/tests/test_write_atomicity.py | 84 +++++++++++++++++++ qdrant_client/uploader/uploader.py | 5 +- 3 files changed, 132 insertions(+), 10 deletions(-) diff --git a/qdrant_client/local/local_collection.py b/qdrant_client/local/local_collection.py index 89cde31ee..9d540a67b 100644 --- a/qdrant_client/local/local_collection.py +++ b/qdrant_client/local/local_collection.py @@ -109,6 +109,19 @@ def validate_dense_vector(vector: Any, vector_name: str) -> None: raise ValueError("Vector contains NaN values") +def validate_vector_dimension(got: int, expected: int, vector_name: str) -> None: + """Reject a vector whose size does not match the collection's, as the server does. + + Without this a wrong-size dense vector reached numpy and died with a raw broadcast + error part-way through the write, and a wrong-size multivector was stored silently. + """ + if got != expected: + raise ValueError( + f"Wrong input: Vector dimension error: expected dim: {expected}, " + f"got {got} for vector '{vector_name}'" + ) + + def validate_multivector(vector: Any, vector_name: str) -> None: """Reject empty multivectors, empty sub-vectors and NaN values, as the server does.""" if len(vector) == 0: @@ -419,7 +432,7 @@ 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. + """Reject vectors the server would refuse on the write path: empty, NaN, wrong size. Sparse vectors are validated by `validate_sparse_vector`; an empty sparse vector is legitimate, so it is not routed here. @@ -429,8 +442,14 @@ def _validate_dense_or_multivector(self, vector: Any, vector_name: str) -> None: if vector_name in self.multivectors: validate_multivector(vector, vector_name) + expected_dim = self.get_vector_params(vector_name).size + for sub_vector in vector: + validate_vector_dimension(len(sub_vector), expected_dim, vector_name) elif vector_name in self.vectors: validate_dense_vector(vector, vector_name) + validate_vector_dimension( + len(vector), self.get_vector_params(vector_name).size, vector_name + ) @classmethod def _check_include_pattern(cls, pattern: str, key: str) -> bool: @@ -2856,11 +2875,9 @@ def delete_vectors( | models.PointIdsList ), ) -> None: - # Check every name up front: the server rejects the whole request on an unknown - # vector name, rather than deleting the names it recognizes first. - for vector_name in vectors: - if vector_name not in self._all_vectors_keys: - raise ValueError(f"Wrong input: Not existing vector name error: {vector_name}") + # Check every name up front, rather than deleting the ones we recognize and then + # failing. The server errors either way, but whether it deletes first varies. + self._validate_vector_names(vectors) ids = self._selector_to_ids(selector) for point_id in ids: @@ -3006,8 +3023,19 @@ def clear_payload( self.payload[idx] = {} self._persist_by_id(point_id) + def _validate_vector_names(self, vector_names: Sequence[str]) -> None: + for vector_name in vector_names: + if vector_name not in self._all_vectors_keys: + raise ValueError(f"Wrong input: Not existing vector name error: {vector_name}") + def _validate_update_operation(self, update_op: types.UpdateOperation) -> None: - """Validate the vectors an operation carries, without touching collection state.""" + """Validate the vectors and payload keys an operation carries, without touching state. + + Filters an operation carries are left to the apply pass, so a batch whose later + operation holds an invalid filter still applies the operations ahead of it. The + server rejects the whole request there, but it also disagrees with itself about + which filters are invalid, so local mode does not try to match it. + """ if isinstance(update_op, models.UpsertOperation): upsert_struct = update_op.upsert if isinstance(upsert_struct, models.PointsBatch): @@ -3028,6 +3056,17 @@ def _validate_update_operation(self, update_op: types.UpdateOperation) -> None: else point.vector ) + elif isinstance(update_op, models.SetPayloadOperation): + if update_op.set_payload.key is not None: + parse_json_path(update_op.set_payload.key) + + elif isinstance(update_op, models.DeletePayloadOperation): + for key in update_op.delete_payload.keys: + parse_json_path(key) + + elif isinstance(update_op, models.DeleteVectorsOperation): + self._validate_vector_names(update_op.delete_vectors.vector) + def batch_update_points( self, update_operations: Sequence[types.UpdateOperation], diff --git a/qdrant_client/local/tests/test_write_atomicity.py b/qdrant_client/local/tests/test_write_atomicity.py index a9fd4f3b4..522777721 100644 --- a/qdrant_client/local/tests/test_write_atomicity.py +++ b/qdrant_client/local/tests/test_write_atomicity.py @@ -88,3 +88,87 @@ def test_rejected_delete_vectors_deletes_nothing() -> None: collection.delete_vectors(vectors=["a", "nope"], selector=[1]) assert sorted(collection._get_vectors(idx=0, with_vectors=True)) == ["a", "b"] + + +def batch_collection() -> LocalCollection: + collection = LocalCollection( + models.CreateCollection( + vectors={ + "a": models.VectorParams(size=3, distance=models.Distance.DOT), + "b": models.VectorParams(size=3, distance=models.Distance.DOT), + } + ) + ) + collection.upsert( + [ + models.PointStruct( + id=1, vector={"a": GOOD_VECTOR, "b": GOOD_VECTOR}, payload={"p": "orig"} + ) + ] + ) + return collection + + +def touch_payload() -> models.SetPayloadOperation: + return models.SetPayloadOperation( + set_payload=models.SetPayload(payload={"p": "changed"}, points=[1]) + ) + + +def test_rejected_batch_operation_applies_nothing() -> None: + """An unknown vector name in a later operation must not keep an earlier one applied. + + The server errors too, but whether it deletes the name it recognized varies run to + run, so this is asserted here rather than in the congruence tests. + """ + collection = batch_collection() + bad_operation = models.DeleteVectorsOperation( + delete_vectors=models.DeleteVectors(points=[1], vector=["a", "nope"]) + ) + + with pytest.raises(ValueError): + collection.batch_update_points([touch_payload(), bad_operation]) + + assert collection.payload[0] == {"p": "orig"} + assert sorted(collection._get_vectors(idx=0, with_vectors=True)) == ["a", "b"] + + +@pytest.mark.parametrize( + ("vectors_config", "good", "bad"), + [ + ( + {"d": models.VectorParams(size=3, distance=models.Distance.DOT)}, + {"d": GOOD_VECTOR}, + {"d": [1.0, 2.0, 3.0, 4.0, 5.0]}, + ), + ( + { + "m": models.VectorParams( + size=3, + distance=models.Distance.DOT, + multivector_config=models.MultiVectorConfig( + comparator=models.MultiVectorComparator.MAX_SIM + ), + ) + }, + {"m": [GOOD_VECTOR]}, + {"m": [[1.0, 2.0, 3.0, 4.0, 5.0]]}, + ), + ], + ids=["dense", "multivector"], +) +def test_wrong_vector_dimension_is_rejected_before_writing(vectors_config, good, bad) -> None: + """A wrong-size vector used to reach numpy: dense died mid-write, multivector was stored.""" + collection = LocalCollection(models.CreateCollection(vectors=vectors_config)) + collection.upsert([models.PointStruct(id=1, vector=good)]) + + with pytest.raises(ValueError, match="expected dim: 3, got 5"): + collection.upsert([models.PointStruct(id=2, vector=bad)]) + + assert len(collection.ids) == 1 + assert_internally_consistent(collection) + + with pytest.raises(ValueError, match="expected dim: 3, got 5"): + collection.update_vectors([models.PointVectors(id=1, vector=bad)]) + + assert collection._get_vectors(idx=0, with_vectors=True) == good diff --git a/qdrant_client/uploader/uploader.py b/qdrant_client/uploader/uploader.py index bdbefe6e5..c23bfb9a6 100644 --- a/qdrant_client/uploader/uploader.py +++ b/qdrant_client/uploader/uploader.py @@ -79,9 +79,8 @@ def _vector_batches_from_numpy(vectors: types.NumpyArray, batch_size: int) -> It def _vector_batches_from_numpy_named_vectors( vectors: dict[str, types.NumpyArray], batch_size: int ) -> Iterable[dict[str, list[float]]]: - assert ( - len(set([arr.shape[0] for arr in vectors.values()])) == 1 - ), "Each named vector should have the same number of vectors" + if len(set([arr.shape[0] for arr in vectors.values()])) != 1: + raise ValueError("Each named vector should have the same number of vectors") num_vectors = next(iter(vectors.values())).shape[0] # Convert dict[str, np.ndarray] to Generator(dict[str, list[float]])