From 83cd887fcd70b9ccce906672ce25c968de1171ac Mon Sep 17 00:00:00 2001
From: Arnaud Gourlay <arnaud.gourlay@gmail.com>
Date: Thu, 28 Nov 2024 12:27:12 +0100
Subject: [PATCH] Return complete error for sparse vector validation (#5536)

* Return complete error for sparse vector validation

* remove unecessary path

* update test to show new value
---
 lib/api/src/conversions/vectors.rs           | 11 +++++------
 lib/api/src/grpc/conversions.rs              | 14 ++++++++------
 lib/collection/src/operations/conversions.rs | 10 ++++++----
 tests/basic_sparse_grpc_test.sh              |  2 +-
 4 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/lib/api/src/conversions/vectors.rs b/lib/api/src/conversions/vectors.rs
index 44f05ebb09..1d39122db9 100644
--- a/lib/api/src/conversions/vectors.rs
+++ b/lib/api/src/conversions/vectors.rs
@@ -464,12 +464,11 @@ impl TryFrom<grpc::Vector> for VectorInternal {
         // sparse vector
         if let Some(indices) = vector.indices {
             return Ok(VectorInternal::Sparse(
-                sparse::common::sparse_vector::SparseVector::new(indices.data, vector.data)
-                    .map_err(|e| {
-                        Status::invalid_argument(format!(
-                            "Sparse indices does not match sparse vector conditions: {e}"
-                        ))
-                    })?,
+                SparseVector::new(indices.data, vector.data).map_err(|e| {
+                    Status::invalid_argument(format!(
+                        "Sparse indices does not match sparse vector conditions: {e}"
+                    ))
+                })?,
             ));
         }
 
diff --git a/lib/api/src/grpc/conversions.rs b/lib/api/src/grpc/conversions.rs
index 752a26a701..be81b2f236 100644
--- a/lib/api/src/grpc/conversions.rs
+++ b/lib/api/src/grpc/conversions.rs
@@ -1709,8 +1709,10 @@ pub fn into_named_vector_struct(
         Some(indices) => NamedVectorStruct::Sparse(NamedSparseVector {
             name: vector_name
                 .ok_or_else(|| Status::invalid_argument("Sparse vector must have a name"))?,
-            vector: SparseVector::new(indices.data, vector).map_err(|_| {
-                Status::invalid_argument("Sparse indices does not match sparse vector conditions")
+            vector: SparseVector::new(indices.data, vector).map_err(|e| {
+                Status::invalid_argument(format!(
+                    "Sparse indices does not match sparse vector conditions: {e}"
+                ))
             })?,
         }),
         None => {
@@ -1957,10 +1959,10 @@ impl TryFrom<SearchPointGroups> for rest::SearchGroupsRequestInternal {
 
         if let Some(sparse_indices) = &search_points.sparse_indices {
             validate_sparse_vector_impl(&sparse_indices.data, &search_points.vector).map_err(
-                |_| {
-                    Status::invalid_argument(
-                        "Sparse indices does not match sparse vector conditions",
-                    )
+                |e| {
+                    Status::invalid_argument(format!(
+                        "Sparse indices does not match sparse vector conditions: {e}"
+                    ))
                 },
             )?;
         }
diff --git a/lib/collection/src/operations/conversions.rs b/lib/collection/src/operations/conversions.rs
index 75b8d3f362..865801ed35 100644
--- a/lib/collection/src/operations/conversions.rs
+++ b/lib/collection/src/operations/conversions.rs
@@ -932,8 +932,10 @@ impl TryFrom<api::grpc::qdrant::SearchPoints> for CoreSearchRequest {
         } = value;
 
         if let Some(sparse_indices) = &sparse_indices {
-            validate_sparse_vector_impl(&sparse_indices.data, &vector).map_err(|_| {
-                Status::invalid_argument("Sparse indices does not match sparse vector conditions")
+            validate_sparse_vector_impl(&sparse_indices.data, &vector).map_err(|e| {
+                Status::invalid_argument(format!(
+                    "Sparse indices does not match sparse vector conditions: {e}"
+                ))
             })?;
         }
 
@@ -1265,9 +1267,9 @@ impl TryFrom<api::grpc::qdrant::VectorExample> for RecommendExample {
                     match vector.indices {
                         Some(indices) => {
                             validate_sparse_vector_impl(&indices.data, &vector.data).map_err(
-                                |_| {
+                                |e| {
                                     Status::invalid_argument(
-                                        "Sparse indices does not match sparse vector conditions",
+                                        format!("Sparse indices does not match sparse vector conditions: {e}"),
                                     )
                                 },
                             )?;
diff --git a/tests/basic_sparse_grpc_test.sh b/tests/basic_sparse_grpc_test.sh
index f84857d9e8..5bfa128f9b 100755
--- a/tests/basic_sparse_grpc_test.sh
+++ b/tests/basic_sparse_grpc_test.sh
@@ -175,7 +175,7 @@ response=$(
     "limit": 3
   }' $QDRANT_HOST qdrant.Points/Search 2>&1
 )
-if [[ $response != *"Sparse indices does not match sparse vector conditions"* ]]; then
+if [[ $response != *"Sparse indices does not match sparse vector conditions: values: Validation error: must be the same length as indices [{}]"* ]]; then
     echo Unexpected response, expected validation error: $response
     exit 1
 fi