refactor: unite key serialization method (#2195)

src/common/meta/src/key.rs

@@ -70,7 +70,7 @@ use table_name::{TableNameKey, TableNameManager, TableNameValue};
 use self::catalog_name::{CatalogManager, CatalogNameValue};
 use self::schema_name::{SchemaManager, SchemaNameValue};
 use self::table_route::{TableRouteManager, TableRouteValue};
-use crate::error::{self, Error, InvalidTableMetadataSnafu, Result, SerdeJsonSnafu};
+use crate::error::{self, Result, SerdeJsonSnafu};
 #[allow(deprecated)]
 pub use crate::key::table_route::{TableRouteKey, TABLE_ROUTE_PREFIX};
 use crate::kv_backend::txn::Txn;
@@ -477,17 +477,10 @@ macro_rules! impl_table_meta_value {
     ($($val_ty: ty), *) => {
         $(
             impl $val_ty {
-                pub fn try_from_raw_value_ref(raw_value: &[u8]) -> Result<Self> {
+                pub fn try_from_raw_value(raw_value: &[u8]) -> Result<Self> {
                     serde_json::from_slice(raw_value).context(SerdeJsonSnafu)
                 }
 
-                pub fn try_from_raw_value(raw_value: Vec<u8>) -> Result<Self> {
-                    let raw_value = String::from_utf8(raw_value).map_err(|e| {
-                        InvalidTableMetadataSnafu { err_msg: e.to_string() }.build()
-                    })?;
-                    serde_json::from_str(&raw_value).context(SerdeJsonSnafu)
-                }
-
                 pub fn try_as_raw_value(&self) -> Result<Vec<u8>> {
                     serde_json::to_vec(self).context(SerdeJsonSnafu)
                 }
@@ -496,21 +489,6 @@ macro_rules! impl_table_meta_value {
     }
 }
 
-macro_rules! impl_try_from {
-    ($($val_ty: ty), *) => {
-        $(
-            impl<'a> TryFrom<&'a Vec<u8>> for $val_ty {
-                type Error = Error;
-                fn try_from(value: &'a Vec<u8>) -> Result<Self> {
-                    serde_json::from_slice(value).context(SerdeJsonSnafu)
-                }
-            }
-        )*
-    };
-}
-
-impl_try_from! {TableInfoValue, TableRouteValue, DatanodeTableValue}
-
 impl_table_meta_value! {
     CatalogNameValue,
     SchemaNameValue,

src/common/meta/src/key/datanode_table.rs

@@ -100,7 +100,7 @@ impl DatanodeTableValue {
 
 /// Decodes `KeyValue` to ((),`DatanodeTableValue`)
 pub fn datanode_table_value_decoder(kv: KeyValue) -> Result<((), DatanodeTableValue)> {
-    let value = DatanodeTableValue::try_from(&kv.value)?;
+    let value = DatanodeTableValue::try_from_raw_value(&kv.value)?;
 
     Ok(((), value))
 }
@@ -118,7 +118,7 @@ impl DatanodeTableManager {
         self.kv_backend
             .get(&key.as_raw_key())
             .await?
-            .map(|kv| DatanodeTableValue::try_from_raw_value(kv.value))
+            .map(|kv| DatanodeTableValue::try_from_raw_value(&kv.value))
             .transpose()
     }
 
@@ -246,7 +246,7 @@ mod tests {
         let raw_value = value.try_as_raw_value().unwrap();
         assert_eq!(raw_value, literal);
 
-        let actual = DatanodeTableValue::try_from_raw_value(literal.to_vec()).unwrap();
+        let actual = DatanodeTableValue::try_from_raw_value(literal).unwrap();
         assert_eq!(actual, value);
     }
 

src/common/meta/src/key/table_info.rs

@@ -201,7 +201,7 @@ impl TableInfoManager {
                 })
                 .flat_map(|r| &r.kvs)
                 .find(|kv| kv.key == raw_key)
-                .map(|kv| TableInfoValue::try_from(&kv.value))
+                .map(|kv| TableInfoValue::try_from_raw_value(&kv.value))
                 .transpose()
         }
     }
@@ -213,7 +213,7 @@ impl TableInfoManager {
         self.kv_backend
             .get(&removed_key)
             .await?
-            .map(|x| TableInfoValue::try_from_raw_value(x.value))
+            .map(|x| TableInfoValue::try_from_raw_value(&x.value))
             .transpose()
     }
 
@@ -223,7 +223,7 @@ impl TableInfoManager {
         self.kv_backend
             .get(&raw_key)
             .await?
-            .map(|x| TableInfoValue::try_from_raw_value(x.value))
+            .map(|x| TableInfoValue::try_from_raw_value(&x.value))
             .transpose()
     }
 }
@@ -240,7 +240,7 @@ mod tests {
     #[test]
     fn test_deserialization_compatibility() {
         let s = r#"{"version":1,"table_info":{"ident":{"table_id":8714,"version":0},"name":"go_gc_duration_seconds","desc":"Created on insertion","catalog_name":"e87lehzy63d4cloud_docs_test","schema_name":"public","meta":{"schema":{"column_schemas":[{"name":"instance","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"job","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"quantile","data_type":{"String":null},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}},{"name":"greptime_timestamp","data_type":{"Timestamp":{"Millisecond":null}},"is_nullable":false,"is_time_index":true,"default_constraint":null,"metadata":{"greptime:time_index":"true"}},{"name":"greptime_value","data_type":{"Float64":{}},"is_nullable":true,"is_time_index":false,"default_constraint":null,"metadata":{}}],"timestamp_index":3,"version":0},"primary_key_indices":[0,1,2],"value_indices":[],"engine":"mito","next_column_id":5,"region_numbers":[],"engine_options":{},"options":{"write_buffer_size":null,"ttl":null,"extra_options":{}},"created_on":"1970-01-01T00:00:00Z"},"table_type":"Base"}}"#;
-        let v = TableInfoValue::try_from_raw_value(s.as_bytes().to_vec()).unwrap();
+        let v = TableInfoValue::try_from_raw_value(s.as_bytes()).unwrap();
         assert!(v.table_info.meta.partition_key_indices.is_empty());
     }
 
@@ -258,7 +258,7 @@ mod tests {
             version: 1,
         };
         let serialized = value.try_as_raw_value().unwrap();
-        let deserialized = TableInfoValue::try_from_raw_value(serialized).unwrap();
+        let deserialized = TableInfoValue::try_from_raw_value(&serialized).unwrap();
         assert_eq!(value, deserialized);
     }
 

src/common/meta/src/key/table_name.rs

@@ -205,7 +205,7 @@ impl TableNameManager {
         self.kv_backend
             .get(&raw_key)
             .await?
-            .map(|x| TableNameValue::try_from_raw_value(x.value))
+            .map(|x| TableNameValue::try_from_raw_value(&x.value))
             .transpose()
     }
 
@@ -227,7 +227,7 @@ impl TableNameManager {
         for kv in resp.kvs {
             res.push((
                 TableNameKey::strip_table_name(kv.key())?,
-                TableNameValue::try_from_raw_value(kv.value)?,
+                TableNameValue::try_from_raw_value(&kv.value)?,
             ))
         }
         Ok(res)
@@ -283,9 +283,6 @@ mod tests {
         let literal = br#"{"table_id":1}"#;
 
         assert_eq!(value.try_as_raw_value().unwrap(), literal);
-        assert_eq!(
-            TableNameValue::try_from_raw_value(literal.to_vec()).unwrap(),
-            value
-        );
+        assert_eq!(TableNameValue::try_from_raw_value(literal).unwrap(), value);
     }
 }

src/common/meta/src/key/table_region.rs

@@ -20,7 +20,7 @@ use store_api::storage::RegionNumber;
 use table::metadata::TableId;
 
 use super::TABLE_REGION_KEY_PREFIX;
-use crate::error::{InvalidTableMetadataSnafu, Result, SerdeJsonSnafu};
+use crate::error::{Result, SerdeJsonSnafu};
 use crate::key::TableMetaKey;
 use crate::{impl_table_meta_key, impl_table_meta_value, DatanodeId};
 
@@ -88,7 +88,7 @@ mod tests {
 
         assert_eq!(value.try_as_raw_value().unwrap(), literal);
         assert_eq!(
-            TableRegionValue::try_from_raw_value(literal.to_vec()).unwrap(),
+            TableRegionValue::try_from_raw_value(literal).unwrap(),
             value,
         );
     }

src/common/meta/src/key/table_route.rs

@@ -184,7 +184,7 @@ impl TableRouteManager {
                 })
                 .flat_map(|r| &r.kvs)
                 .find(|kv| kv.key == raw_key)
-                .map(|kv| TableRouteValue::try_from(&kv.value))
+                .map(|kv| TableRouteValue::try_from_raw_value(&kv.value))
                 .transpose()
         }
     }
@@ -194,7 +194,7 @@ impl TableRouteManager {
         self.kv_backend
             .get(&key.as_raw_key())
             .await?
-            .map(|kv| TableRouteValue::try_from_raw_value(kv.value))
+            .map(|kv| TableRouteValue::try_from_raw_value(&kv.value))
             .transpose()
     }
 
@@ -205,7 +205,7 @@ impl TableRouteManager {
         self.kv_backend
             .get(&removed_key)
             .await?
-            .map(|x| TableRouteValue::try_from_raw_value(x.value))
+            .map(|x| TableRouteValue::try_from_raw_value(&x.value))
             .transpose()
     }
 

tests-integration/tests/region_failover.rs

@@ -112,7 +112,7 @@ pub async fn test_region_failover(store_type: StorageType) {
     let cache_key = TableNameKey::new("greptime", "public", "my_table").as_raw_key();
 
     let cache = get_table_cache(&frontend, &cache_key).unwrap();
-    let table_name_value = TableNameValue::try_from_raw_value(cache.unwrap().value).unwrap();
+    let table_name_value = TableNameValue::try_from_raw_value(&cache.unwrap().value).unwrap();
     let table_id = table_name_value.table_id();
     assert!(get_route_cache(&frontend, table_id).is_some());