Skip to content

Commit

Permalink
Add error handling if vector is missing for point (qdrant#5211)
Browse files Browse the repository at this point in the history 
* add error handling for missing vectors

* add integration test

* add call without specifying 'using' parameter
  • Loading branch information
@JojiiOfficial @timvisee
JojiiOfficial authored and timvisee committed Nov 8, 2024
1 parent 4f7482c commit 9690ba6
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 26 deletions.
78 changes: 53 additions & 25 deletions lib/collection/src/operations/universal_query/collection_query.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -164,14 +164,14 @@ impl VectorQuery<VectorInputInternal> {
ids_to_vectors: &ReferencedVectors,
lookup_vector_name: &str,
lookup_collection: Option<&String>,
) -> VectorQuery<VectorInternal> {
) -> CollectionResult<VectorQuery<VectorInternal>> {
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(
Expand All @@ -180,7 +180,9 @@ impl VectorQuery<VectorInputInternal> {
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(
Expand All @@ -189,42 +191,64 @@ impl VectorQuery<VectorInputInternal> {
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::<CollectionResult<_>>()?;

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::<CollectionResult<_>>()?;

VectorQuery::Context(ContextQuery { pairs })
Ok(VectorQuery::Context(ContextQuery { pairs }))
}
}
}
Expand Down Expand Up @@ -262,6 +286,10 @@ impl VectorQuery<VectorInputInternal> {
}
}

fn vector_not_found_error(vector_name: &str) -> CollectionError {
CollectionError::not_found(format!("Vector with name {vector_name:?} for point"))
}

impl VectorQuery<VectorInternal> {
fn into_query_enum(self, using: String) -> CollectionResult<QueryEnum> {
let query_enum = match self {
Expand Down
41 changes: 40 additions & 1 deletion tests/openapi/test_query_full.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
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']

0 comments on commit 9690ba6

Please sign in to comment.