diff --git a/lib/collection/src/operations/universal_query/collection_query.rs b/lib/collection/src/operations/universal_query/collection_query.rs index 08b3e90bd9..3b0e36c637 100644 --- a/lib/collection/src/operations/universal_query/collection_query.rs +++ b/lib/collection/src/operations/universal_query/collection_query.rs @@ -105,7 +105,7 @@ impl Query { Query::Vector(vector_query) => { let query_enum = vector_query // Homogenize the input into raw vectors - .ids_into_vectors(ids_to_vectors, lookup_vector_name, lookup_collection) + .ids_into_vectors(ids_to_vectors, lookup_vector_name, lookup_collection)? // Turn into QueryEnum .into_query_enum(using)?; ScoringQuery::Vector(query_enum) @@ -164,14 +164,14 @@ impl VectorQuery { ids_to_vectors: &ReferencedVectors, lookup_vector_name: &str, lookup_collection: Option<&String>, - ) -> VectorQuery { + ) -> CollectionResult> { match self { VectorQuery::Nearest(vector_input) => { let vector = ids_to_vectors .resolve_reference(lookup_collection, lookup_vector_name, vector_input) - .unwrap(); + .ok_or_else(|| vector_not_found_error(lookup_vector_name))?; - VectorQuery::Nearest(vector) + Ok(VectorQuery::Nearest(vector)) } VectorQuery::RecommendAverageVector(reco) => { let (positives, negatives) = Self::resolve_reco_reference( @@ -180,7 +180,9 @@ impl VectorQuery { lookup_vector_name, lookup_collection, ); - VectorQuery::RecommendAverageVector(RecoQuery::new(positives, negatives)) + Ok(VectorQuery::RecommendAverageVector(RecoQuery::new( + positives, negatives, + ))) } VectorQuery::RecommendBestScore(reco) => { let (positives, negatives) = Self::resolve_reco_reference( @@ -189,42 +191,64 @@ impl VectorQuery { lookup_vector_name, lookup_collection, ); - VectorQuery::RecommendBestScore(RecoQuery::new(positives, negatives)) + Ok(VectorQuery::RecommendBestScore(RecoQuery::new( + positives, negatives, + ))) } VectorQuery::Discover(discover) => { let target = ids_to_vectors .resolve_reference(lookup_collection, lookup_vector_name, discover.target) - .unwrap(); + .ok_or_else(|| vector_not_found_error(lookup_vector_name))?; let pairs = discover .pairs .into_iter() - .map(|pair| ContextPair { - positive: ids_to_vectors - .resolve_reference(lookup_collection, lookup_vector_name, pair.positive) - .unwrap(), - negative: ids_to_vectors - .resolve_reference(lookup_collection, lookup_vector_name, pair.negative) - .unwrap(), + .map(|pair| { + Ok(ContextPair { + positive: ids_to_vectors + .resolve_reference( + lookup_collection, + lookup_vector_name, + pair.positive, + ) + .ok_or_else(|| vector_not_found_error(lookup_vector_name))?, + negative: ids_to_vectors + .resolve_reference( + lookup_collection, + lookup_vector_name, + pair.negative, + ) + .ok_or_else(|| vector_not_found_error(lookup_vector_name))?, + }) }) - .collect(); + .collect::>()?; - VectorQuery::Discover(DiscoveryQuery { target, pairs }) + Ok(VectorQuery::Discover(DiscoveryQuery { target, pairs })) } VectorQuery::Context(context) => { let pairs = context .pairs .into_iter() - .map(|pair| ContextPair { - positive: ids_to_vectors - .resolve_reference(lookup_collection, lookup_vector_name, pair.positive) - .unwrap(), - negative: ids_to_vectors - .resolve_reference(lookup_collection, lookup_vector_name, pair.negative) - .unwrap(), + .map(|pair| { + Ok(ContextPair { + positive: ids_to_vectors + .resolve_reference( + lookup_collection, + lookup_vector_name, + pair.positive, + ) + .ok_or_else(|| vector_not_found_error(lookup_vector_name))?, + negative: ids_to_vectors + .resolve_reference( + lookup_collection, + lookup_vector_name, + pair.negative, + ) + .ok_or_else(|| vector_not_found_error(lookup_vector_name))?, + }) }) - .collect(); + .collect::>()?; - VectorQuery::Context(ContextQuery { pairs }) + Ok(VectorQuery::Context(ContextQuery { pairs })) } } } @@ -262,6 +286,10 @@ impl VectorQuery { } } +fn vector_not_found_error(vector_name: &str) -> CollectionError { + CollectionError::not_found(format!("Vector with name {vector_name:?} for point")) +} + impl VectorQuery { fn into_query_enum(self, using: String) -> CollectionResult { let query_enum = match self { diff --git a/tests/openapi/test_query_full.py b/tests/openapi/test_query_full.py index 41c21eec8b..c0d0ee5613 100644 --- a/tests/openapi/test_query_full.py +++ b/tests/openapi/test_query_full.py @@ -1652,4 +1652,43 @@ def test_discover_batch(query_filter): query_result = response.json()["result"] assert search_result[0] == query_result[0]["points"] - assert search_result[1] == query_result[1]["points"] \ No newline at end of file + assert search_result[1] == query_result[1]["points"] + + +# Qdrant did panic for some Query API requests when using a vector name that is not existing +# for the given point. This tests ensures that a proper error response gets returned. +# See https://github.com/qdrant/qdrant/issues/5208 for more details. +def test_query_with_missing_vector(): + response = request_with_validation( + api="/collections/{collection_name}/points/query", + method="POST", + path_params={"collection_name": collection_name}, + body={ + "query": 7, + "using": "sparse-text" + }, + ) + assert response.ok + + response = request_with_validation( + api="/collections/{collection_name}/points/query", + method="POST", + path_params={"collection_name": collection_name}, + body={ + "query": 8, # Point with ID=8 doesn't have a vector 'sparse-text' which caused Qdrant to panic before. + "using": "sparse-text" + }, + ) + assert not response.ok + assert 'error' in response.json()['status'] + + response2 = request_with_validation( + api="/collections/{collection_name}/points/query", + method="POST", + path_params={"collection_name": collection_name}, + body={ + "query": 8, # Point with ID=8 doesn't have a default vector which caused Qdrant to panic before. + }, + ) + assert not response2.ok + assert 'error' in response2.json()['status']