fix: cr comments

Author: v0y4g3r

Cargo.lock

@@ -15,8 +15,8 @@ datafusion-expr.workspace = true
 datatypes = { path = "../datatypes" }
 meta-client = { path = "../meta-client" }
 moka = { version = "0.9", features = ["future"] }
-snafu = { version = "0.7", features = ["backtraces"] }
+snafu.workspace = true
 store-api = { path = "../store-api" }
-serde = "1.0"
+serde.workspace = true
 serde_json = "1.0"
 table = { path = "../table" }

src/partition/Cargo.toml

@@ -137,8 +137,6 @@ impl RangeColumnsPartitionRule {
 }
 
 impl PartitionRule for RangeColumnsPartitionRule {
-    type Error = Error;
-
     fn as_any(&self) -> &dyn Any {
         self
     }
@@ -147,7 +145,7 @@ impl PartitionRule for RangeColumnsPartitionRule {
         self.column_list.clone()
     }
 
-    fn find_region(&self, values: &[Value]) -> Result<RegionNumber, Self::Error> {
+    fn find_region(&self, values: &[Value]) -> Result<RegionNumber, Error> {
         ensure!(
             values.len() == self.column_list.len(),
             error::RegionKeysSizeSnafu {
@@ -168,7 +166,7 @@ impl PartitionRule for RangeColumnsPartitionRule {
         })
     }
 
-    fn find_regions(&self, exprs: &[PartitionExpr]) -> Result<Vec<RegionNumber>, Self::Error> {
+    fn find_regions(&self, exprs: &[PartitionExpr]) -> Result<Vec<RegionNumber>, Error> {
         let regions = if exprs.iter().all(|x| self.column_list.contains(&x.column)) {
             let PartitionExpr {
                 column: _,

src/partition/src/columns.rs

@@ -14,10 +14,6 @@
 
 #![feature(assert_matches)]
 
-use std::sync::Arc;
-
-use crate::partition::PartitionRule;
-
 pub mod columns;
 pub mod error;
 pub mod manager;
@@ -26,4 +22,4 @@ pub mod range;
 pub mod route;
 pub mod splitter;
 
-pub type PartitionRuleRef = Arc<dyn PartitionRule<Error = error::Error>>;
+pub use crate::partition::{PartitionRule, PartitionRuleRef};

src/partition/src/lib.rs

@@ -33,13 +33,14 @@ use crate::{error, PartitionRuleRef};
 
 pub type PartitionRuleManagerRef = Arc<PartitionRuleManager>;
 
+/// PartitionRuleManager manages the table routes and partition rules.
+/// It provides methods to find regions by:
+/// - values (in case of insertion)
+/// - filters (in case of select, deletion and update)
 pub struct PartitionRuleManager {
     table_routes: Arc<TableRoutes>,
 }
 
-/// Provides methods to find regions by:
-/// - values (in case of insertion)
-/// - filters (in case of select, deletion and update)
 impl PartitionRuleManager {
     pub fn new(table_routes: Arc<TableRoutes>) -> Self {
         Self { table_routes }

src/partition/src/manager.rs

@@ -14,6 +14,7 @@
 
 use std::any::Any;
 use std::fmt::Debug;
+use std::sync::Arc;
 
 use datafusion_expr::Operator;
 use datatypes::prelude::Value;
@@ -24,18 +25,18 @@ use store_api::storage::RegionNumber;
 
 use crate::error::{self, Error};
 
-pub trait PartitionRule: Sync + Send {
-    type Error: Debug;
+pub type PartitionRuleRef = Arc<dyn PartitionRule>;
 
+pub trait PartitionRule: Sync + Send {
     fn as_any(&self) -> &dyn Any;
 
     fn partition_columns(&self) -> Vec<String>;
 
     // TODO(LFC): Unify `find_region` and `find_regions` methods when distributed read and write features are both merged into develop.
     // Or find better names since one is mainly for writes and the other is for reads.
-    fn find_region(&self, values: &[Value]) -> Result<RegionNumber, Self::Error>;
+    fn find_region(&self, values: &[Value]) -> Result<RegionNumber, Error>;
 
-    fn find_regions(&self, exprs: &[PartitionExpr]) -> Result<Vec<RegionNumber>, Self::Error>;
+    fn find_regions(&self, exprs: &[PartitionExpr]) -> Result<Vec<RegionNumber>, Error>;
 }
 
 /// The right bound(exclusive) of partition range.

src/partition/src/partition.rs

@@ -97,8 +97,6 @@ impl RangePartitionRule {
 }
 
 impl PartitionRule for RangePartitionRule {
-    type Error = Error;
-
     fn as_any(&self) -> &dyn Any {
         self
     }
@@ -107,7 +105,7 @@ impl PartitionRule for RangePartitionRule {
         vec![self.column_name().to_string()]
     }
 
-    fn find_region(&self, values: &[Value]) -> Result<RegionNumber, Self::Error> {
+    fn find_region(&self, values: &[Value]) -> Result<RegionNumber, Error> {
         debug_assert_eq!(
             values.len(),
             1,
@@ -122,7 +120,7 @@ impl PartitionRule for RangePartitionRule {
         })
     }
 
-    fn find_regions(&self, exprs: &[PartitionExpr]) -> Result<Vec<RegionNumber>, Self::Error> {
+    fn find_regions(&self, exprs: &[PartitionExpr]) -> Result<Vec<RegionNumber>, Error> {
         if exprs.is_empty() {
             return Ok(self.regions.clone());
         }