refactor: Make TableEngine object safe (#119)

* refactor: Make TableEngine object safe

* define TableEngineRef

* fix some comments

* replace table::engine::Error with table::error::Error

src/datanode/src/error.rs

@@ -6,7 +6,6 @@ use common_error::prelude::*;
 use datatypes::prelude::ConcreteDataType;
 use storage::error::Error as StorageError;
 use table::error::Error as TableError;
-use table_engine::error::Error as TableEngineError;
 
 /// Business error of datanode.
 #[derive(Debug, Snafu)]
@@ -28,7 +27,7 @@ pub enum Error {
     CreateTable {
         table_name: String,
         #[snafu(backtrace)]
-        source: TableEngineError,
+        source: TableError,
     },
 
     #[snafu(display("Fail to get table: {}, {}", table_name, source))]

src/table-engine/src/engine.rs

@@ -14,12 +14,13 @@ use store_api::storage::{
 };
 use table::engine::{EngineContext, TableEngine};
 use table::requests::{AlterTableRequest, CreateTableRequest, DropTableRequest};
+use table::Result as TableResult;
 use table::{
     metadata::{TableId, TableInfoBuilder, TableMetaBuilder, TableType},
     table::TableRef,
 };
 
-use crate::error::{self, Error, Result};
+use crate::error::{self, Result};
 use crate::table::MitoTable;
 
 pub const DEFAULT_ENGINE: &str = "mito";
@@ -43,33 +44,35 @@ impl<Store: StorageEngine> MitoEngine<Store> {
 
 #[async_trait]
 impl<Store: StorageEngine> TableEngine for MitoEngine<Store> {
-    type Error = Error;
-
     async fn create_table(
         &self,
         ctx: &EngineContext,
         request: CreateTableRequest,
-    ) -> Result<TableRef> {
-        self.inner.create_table(ctx, request).await
+    ) -> TableResult<TableRef> {
+        Ok(self.inner.create_table(ctx, request).await?)
     }
 
     async fn alter_table(
         &self,
         _ctx: &EngineContext,
         _request: AlterTableRequest,
-    ) -> Result<TableRef> {
+    ) -> TableResult<TableRef> {
         unimplemented!();
     }
 
-    fn get_table(&self, ctx: &EngineContext, name: &str) -> Result<Option<TableRef>> {
-        self.inner.get_table(ctx, name)
+    fn get_table(&self, ctx: &EngineContext, name: &str) -> TableResult<Option<TableRef>> {
+        Ok(self.inner.get_table(ctx, name)?)
     }
 
     fn table_exists(&self, _ctx: &EngineContext, _name: &str) -> bool {
         unimplemented!();
     }
 
-    async fn drop_table(&self, _ctx: &EngineContext, _request: DropTableRequest) -> Result<()> {
+    async fn drop_table(
+        &self,
+        _ctx: &EngineContext,
+        _request: DropTableRequest,
+    ) -> TableResult<()> {
         unimplemented!();
     }
 }

src/table-engine/src/error.rs

@@ -13,6 +13,12 @@ pub enum Error {
     },
 }
 
+impl From<Error> for table::error::Error {
+    fn from(e: Error) -> Self {
+        table::error::Error::new(e)
+    }
+}
+
 pub type Result<T> = std::result::Result<T, Error>;
 
 impl ErrorExt for Error {
@@ -51,4 +57,12 @@ mod tests {
         assert_eq!(StatusCode::InvalidArguments, err.status_code());
         assert!(err.backtrace_opt().is_some());
     }
+
+    #[test]
+    pub fn test_opaque_error() {
+        let error = throw_create_table(StatusCode::InvalidSyntax).err().unwrap();
+        let table_engine_error: table::error::Error = error.into();
+        assert!(table_engine_error.backtrace_opt().is_some());
+        assert_eq!(StatusCode::InvalidSyntax, table_engine_error.status_code());
+    }
 }

src/table/src/engine.rs

@@ -1,13 +1,12 @@
-use common_error::ext::ErrorExt;
+use std::sync::Arc;
 
+use crate::error::Result;
 use crate::requests::{AlterTableRequest, CreateTableRequest, DropTableRequest};
 use crate::TableRef;
 
 /// Table engine abstraction.
 #[async_trait::async_trait]
-pub trait TableEngine: Send + Sync + Clone {
-    type Error: ErrorExt + Send + Sync + 'static;
-
+pub trait TableEngine: Send + Sync {
     /// Create a table by given request.
     ///
     /// Return the created table.
@@ -15,7 +14,7 @@ pub trait TableEngine: Send + Sync + Clone {
         &self,
         ctx: &EngineContext,
         request: CreateTableRequest,
-    ) -> Result<TableRef, Self::Error>;
+    ) -> Result<TableRef>;
 
     /// Alter table schema, options etc. by given request,
     ///
@@ -24,22 +23,20 @@ pub trait TableEngine: Send + Sync + Clone {
         &self,
         ctx: &EngineContext,
         request: AlterTableRequest,
-    ) -> Result<TableRef, Self::Error>;
+    ) -> Result<TableRef>;
 
     /// Returns the table by it's name.
-    fn get_table(&self, ctx: &EngineContext, name: &str) -> Result<Option<TableRef>, Self::Error>;
+    fn get_table(&self, ctx: &EngineContext, name: &str) -> Result<Option<TableRef>>;
 
     /// Returns true when the given table is exists.
     fn table_exists(&self, ctx: &EngineContext, name: &str) -> bool;
 
     /// Drops the given table.
-    async fn drop_table(
-        &self,
-        ctx: &EngineContext,
-        request: DropTableRequest,
-    ) -> Result<(), Self::Error>;
+    async fn drop_table(&self, ctx: &EngineContext, request: DropTableRequest) -> Result<()>;
 }
 
+pub type TableEngineRef = Arc<dyn TableEngine>;
+
 /// Storage engine context.
 #[derive(Debug, Clone, Default)]
 pub struct EngineContext {}

src/table/src/lib.rs

@@ -4,4 +4,5 @@ pub mod metadata;
 pub mod requests;
 pub mod table;
 
+pub use crate::error::{Error, Result};
 pub use crate::table::{Table, TableRef};