feat: improve and distinguish different errors for IllegalInsertData (#1503)

* feat: improve and distinguish different errors for IllegalInsertData

* feat: change error code for UnexpectedValuesLength and ColumnAlreadyExists

* chore: improve readability of error message

src/common/grpc-expr/src/error.rs

@@ -31,9 +31,6 @@ pub enum Error {
     #[snafu(display("Failed to convert bytes to insert batch, source: {}", source))]
     DecodeInsert { source: DecodeError },
 
-    #[snafu(display("Illegal insert data"))]
-    IllegalInsertData { location: Location },
-
     #[snafu(display("Illegal delete request, reason: {reason}"))]
     IllegalDeleteRequest { reason: String, location: Location },
 
@@ -90,6 +87,12 @@ pub enum Error {
         #[snafu(backtrace)]
         source: table::error::Error,
     },
+
+    #[snafu(display("Unexpected values length, reason: {}", reason))]
+    UnexpectedValuesLength { reason: String, location: Location },
+
+    #[snafu(display("The column name already exists, column: {}", column))]
+    ColumnAlreadyExists { column: String, location: Location },
 }
 
 pub type Result<T> = std::result::Result<T, Error>;
@@ -99,9 +102,9 @@ impl ErrorExt for Error {
         match self {
             Error::ColumnNotFound { .. } => StatusCode::TableColumnNotFound,
 
-            Error::DecodeInsert { .. }
-            | Error::IllegalInsertData { .. }
-            | Error::IllegalDeleteRequest { .. } => StatusCode::InvalidArguments,
+            Error::DecodeInsert { .. } | Error::IllegalDeleteRequest { .. } => {
+                StatusCode::InvalidArguments
+            }
 
             Error::ColumnDataType { .. } => StatusCode::Internal,
             Error::DuplicatedTimestampColumn { .. } | Error::MissingTimestampColumn { .. } => {
@@ -113,6 +116,9 @@ impl ErrorExt for Error {
             Error::ColumnDefaultConstraint { source, .. } => source.status_code(),
             Error::InvalidColumnDef { source, .. } => source.status_code(),
             Error::UnrecognizedTableOption { .. } => StatusCode::InvalidArguments,
+            Error::UnexpectedValuesLength { .. } | Error::ColumnAlreadyExists { .. } => {
+                StatusCode::InvalidArguments
+            }
         }
     }
 

src/common/grpc-expr/src/insert.rs

@@ -34,8 +34,9 @@ use table::metadata::TableId;
 use table::requests::InsertRequest;
 
 use crate::error::{
-    ColumnDataTypeSnafu, CreateVectorSnafu, DuplicatedTimestampColumnSnafu, IllegalInsertDataSnafu,
-    InvalidColumnProtoSnafu, MissingTimestampColumnSnafu, Result,
+    ColumnAlreadyExistsSnafu, ColumnDataTypeSnafu, CreateVectorSnafu,
+    DuplicatedTimestampColumnSnafu, InvalidColumnProtoSnafu, MissingTimestampColumnSnafu, Result,
+    UnexpectedValuesLengthSnafu,
 };
 const TAG_SEMANTIC_TYPE: i32 = SemanticType::Tag as i32;
 const TIMESTAMP_SEMANTIC_TYPE: i32 = SemanticType::Timestamp as i32;
@@ -292,9 +293,11 @@ pub fn to_table_insert_request(
 
         ensure!(
             columns_values
-                .insert(column_name, vector_builder.to_vector())
+                .insert(column_name.clone(), vector_builder.to_vector())
                 .is_none(),
-            IllegalInsertDataSnafu
+            ColumnAlreadyExistsSnafu {
+                column: column_name
+            }
         );
     }
 
@@ -317,7 +320,12 @@ pub(crate) fn add_values_to_builder(
     let values = convert_values(&data_type, values);
 
     if null_mask.is_empty() {
-        ensure!(values.len() == row_count, IllegalInsertDataSnafu);
+        ensure!(
+            values.len() == row_count,
+            UnexpectedValuesLengthSnafu {
+                reason: "If null_mask is empty, the length of values must be equal to row_count."
+            }
+        );
 
         values.iter().try_for_each(|value| {
             builder
@@ -328,7 +336,9 @@ pub(crate) fn add_values_to_builder(
         let null_mask = BitVec::from_vec(null_mask);
         ensure!(
             null_mask.count_ones() + values.len() == row_count,
-            IllegalInsertDataSnafu
+            UnexpectedValuesLengthSnafu {
+                reason: "If null_mask is not empty, the sum of the number of nulls and the length of values must be equal to row_count."
+            }
         );
 
         let mut idx_of_values = 0;