chore: fix get_auto_increment_next_value & next_val return error

Author: KKould

src/meta/api/src/auto_increment_api.rs

@@ -15,12 +15,13 @@
 use databend_common_meta_app::schema::GetAutoIncrementNextValueReply;
 use databend_common_meta_app::schema::GetAutoIncrementNextValueReq;
 
-use crate::kv_app_error::KVAppError;
+use crate::errors::AutoIncrementError;
+use crate::meta_txn_error::MetaTxnError;
 
 #[async_trait::async_trait]
 pub trait AutoIncrementApi: Send + Sync {
     async fn get_auto_increment_next_value(
         &self,
         req: GetAutoIncrementNextValueReq,
-    ) -> Result<GetAutoIncrementNextValueReply, KVAppError>;
+    ) -> Result<Result<GetAutoIncrementNextValueReply, AutoIncrementError>, MetaTxnError>;
 }

src/meta/api/src/auto_increment_impl.rs

@@ -21,15 +21,16 @@ use log::debug;
 
 use crate::auto_increment_api::AutoIncrementApi;
 use crate::auto_increment_nextval_impl::NextVal;
-use crate::kv_app_error::KVAppError;
+use crate::errors::AutoIncrementError;
+use crate::meta_txn_error::MetaTxnError;
 
 #[async_trait::async_trait]
 #[tonic::async_trait]
 impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> AutoIncrementApi for KV {
     async fn get_auto_increment_next_value(
         &self,
         req: GetAutoIncrementNextValueReq,
-    ) -> Result<GetAutoIncrementNextValueReply, KVAppError> {
+    ) -> Result<Result<GetAutoIncrementNextValueReply, AutoIncrementError>, MetaTxnError> {
         debug!(req :? =(&req); "AutoIncrementApi: {}", func_name!());
 
         let next_val = NextVal {
@@ -38,12 +39,17 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> AutoIncrementApi for KV {
             expr: req.expr.clone(),
         };
 
-        let resp = next_val.next_val(&req.tenant, req.count).await?;
+        let (start, end) = match next_val.next_val(&req.tenant, req.count).await? {
+            Ok(resp) => (resp.before, resp.after),
+            Err(err) => {
+                return Ok(Err(err));
+            }
+        };
 
-        Ok(GetAutoIncrementNextValueReply {
-            start: resp.before,
+        Ok(Ok(GetAutoIncrementNextValueReply {
+            start,
             step: req.expr.step,
-            end: resp.after,
-        })
+            end,
+        }))
     }
 }

src/meta/api/src/auto_increment_nextval_impl.rs

@@ -13,9 +13,6 @@
 // limitations under the License.
 
 use databend_common_expression::AutoIncrementExpr;
-use databend_common_meta_app::app_error::AppError;
-use databend_common_meta_app::app_error::AutoIncrementError;
-use databend_common_meta_app::app_error::OutOfAutoIncrementRange;
 use databend_common_meta_app::principal::AutoIncrementKey;
 use databend_common_meta_app::schema::AutoIncrementStorageIdent;
 use databend_common_meta_app::tenant::ToTenant;
@@ -28,7 +25,8 @@ use databend_common_meta_types::TxnOp;
 use databend_common_meta_types::TxnRequest;
 use log::debug;
 
-use crate::kv_app_error::KVAppError;
+use crate::errors::AutoIncrementError;
+use crate::meta_txn_error::MetaTxnError;
 use crate::send_txn;
 
 /// The implementation of `next_val` for sequence number.
@@ -48,7 +46,7 @@ where KV: kvapi::KVApi<Error = MetaError> + ?Sized
         self,
         tenant: impl ToTenant,
         count: u64,
-    ) -> Result<FetchAddU64Response, KVAppError> {
+    ) -> Result<Result<FetchAddU64Response, AutoIncrementError>, MetaTxnError> {
         debug!("{}", func_name!());
 
         // Key for the sequence number value.
@@ -75,17 +73,15 @@ where KV: kvapi::KVApi<Error = MetaError> + ?Sized
         let got_delta = resp.delta();
 
         if got_delta < delta {
-            return Err(KVAppError::AppError(AppError::AutoIncrementError(
-                AutoIncrementError::OutOfAutoIncrementRange(OutOfAutoIncrementRange::new(
-                    self.key,
-                    format!(
-                        "{}: count: {count}; expected delta: {delta}, but got: {resp}",
-                        self.expr.step,
-                    ),
-                )),
-            )));
+            return Ok(Err(AutoIncrementError::OutOfAutoIncrementRange {
+                key: self.key,
+                context: format!(
+                    "{}: count: {count}; expected delta: {delta}, but got: {resp}",
+                    self.expr.step,
+                ),
+            }));
         }
 
-        Ok(resp.clone())
+        Ok(Ok(resp.clone()))
     }
 }

src/meta/api/src/errors.rs

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use databend_common_exception::ErrorCode;
+use databend_common_meta_app::principal::AutoIncrementKey;
 
 /// Table logic error, unrelated to the backend service providing Table management, or dependent component.
 #[derive(Clone, Debug, thiserror::Error)]
@@ -37,3 +38,21 @@ impl From<TableError> for ErrorCode {
         }
     }
 }
+
+#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)]
+pub enum AutoIncrementError {
+    #[error("OutOfAutoIncrementRange: `{key}` while `{context}`")]
+    OutOfAutoIncrementRange {
+        key: AutoIncrementKey,
+        context: String,
+    },
+}
+
+impl From<AutoIncrementError> for ErrorCode {
+    fn from(value: AutoIncrementError) -> Self {
+        let s = value.to_string();
+        match value {
+            AutoIncrementError::OutOfAutoIncrementRange { .. } => ErrorCode::AutoIncrementError(s),
+        }
+    }
+}

src/meta/app/src/app_error.rs

@@ -19,7 +19,6 @@ use databend_common_meta_types::MatchSeq;
 
 use crate::data_mask::data_mask_name_ident;
 use crate::principal::procedure_name_ident;
-use crate::principal::AutoIncrementKey;
 use crate::principal::ProcedureIdentity;
 use crate::row_access_policy::row_access_policy_name_ident;
 use crate::schema::catalog_name_ident;
@@ -968,22 +967,6 @@ impl UnsupportedSequenceStorageVersion {
     }
 }
 
-#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)]
-#[error("OutOfAutoIncrementRange: `{key}` while `{context}`")]
-pub struct OutOfAutoIncrementRange {
-    key: AutoIncrementKey,
-    context: String,
-}
-
-impl OutOfAutoIncrementRange {
-    pub fn new(key: AutoIncrementKey, context: impl ToString) -> Self {
-        Self {
-            key,
-            context: context.to_string(),
-        }
-    }
-}
-
 /// Application error.
 ///
 /// The application does not get expected result but there is nothing wrong with meta-service.
@@ -1165,9 +1148,6 @@ pub enum AppError {
     #[error(transparent)]
     SequenceError(#[from] SequenceError),
 
-    #[error(transparent)]
-    AutoIncrementError(#[from] AutoIncrementError),
-
     #[error(transparent)]
     UpdateStreamMetasFailed(#[from] UpdateStreamMetasFailed),
 
@@ -1236,12 +1216,6 @@ pub enum SequenceError {
     UnsupportedSequenceStorageVersion(#[from] UnsupportedSequenceStorageVersion),
 }
 
-#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)]
-pub enum AutoIncrementError {
-    #[error(transparent)]
-    OutOfAutoIncrementRange(#[from] OutOfAutoIncrementRange),
-}
-
 impl AppErrorMessage for TenantIsEmpty {
     fn message(&self) -> String {
         self.to_string()
@@ -1594,22 +1568,6 @@ impl AppErrorMessage for SequenceError {
     }
 }
 
-impl AppErrorMessage for OutOfAutoIncrementRange {
-    fn message(&self) -> String {
-        format!("AutoIncrement '{}' out of range", self.key)
-    }
-}
-
-impl AppErrorMessage for AutoIncrementError {
-    fn message(&self) -> String {
-        match self {
-            AutoIncrementError::OutOfAutoIncrementRange(e) => {
-                format!("OutOfAutoIncrementRange: '{}'", e.message())
-            }
-        }
-    }
-}
-
 impl AppErrorMessage for VirtualColumnIdOutBound {}
 
 impl AppErrorMessage for VirtualColumnTooMany {}
@@ -1737,7 +1695,6 @@ impl From<AppError> for ErrorCode {
             AppError::CleanDbIdTableNamesFailed(err) => {
                 ErrorCode::GeneralDbGcFailure(err.message())
             }
-            AppError::AutoIncrementError(err) => ErrorCode::AutoIncrementError(err.message()),
         }
     }
 }

src/query/service/src/catalogs/default/mutable_catalog.rs

@@ -937,7 +937,7 @@ impl Catalog for MutableCatalog {
         &self,
         req: GetAutoIncrementNextValueReq,
     ) -> Result<GetAutoIncrementNextValueReply> {
-        let res = self.ctx.meta.get_auto_increment_next_value(req).await?;
+        let res = self.ctx.meta.get_auto_increment_next_value(req).await??;
         Ok(res)
     }
 }

tests/suites/0_stateless/02_ddl/02_0001_autoincrement.sh

@@ -5,7 +5,6 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 
 echo "create or replace database test" | $BENDSQL_CLIENT_CONNECT
 
-
 stmt """
 create or replace table test_autoincrement (c1 int, c2 string autoincrement);
 """