From 69a2036ceeb345df6fba7520dd6f3861918c30f5 Mon Sep 17 00:00:00 2001
From: Weny Xu <wenymedia@gmail.com>
Date: Tue, 15 Aug 2023 11:36:58 +0800
Subject: [PATCH] feat!: add deserializer for Partition (#2169)

* feat!: add deserializer for Partition

* fix: fix tests
---
 src/common/meta/src/rpc/router.rs        | 45 ++++++++++++++++++------
 src/frontend/src/instance/distributed.rs |  4 +--
 src/partition/src/partition.rs           |  2 +-
 3 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/src/common/meta/src/rpc/router.rs b/src/common/meta/src/rpc/router.rs
index ac5d815015..71fe8ccb4c 100644
--- a/src/common/meta/src/rpc/router.rs
+++ b/src/common/meta/src/rpc/router.rs
@@ -19,6 +19,7 @@ use api::v1::meta::{
     RouteRequest as PbRouteRequest, RouteResponse as PbRouteResponse, Table as PbTable,
     TableId as PbTableId, TableRoute as PbTableRoute, TableRouteValue as PbTableRouteValue,
 };
+use serde::ser::SerializeSeq;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use snafu::OptionExt;
 use store_api::storage::{RegionId, RegionNumber};
@@ -307,9 +308,9 @@ impl From<Region> for PbRegion {
 
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
 pub struct Partition {
-    #[serde(serialize_with = "as_utf8_vec")]
+    #[serde(serialize_with = "as_utf8_vec", deserialize_with = "from_utf8_vec")]
     pub column_list: Vec<Vec<u8>>,
-    #[serde(serialize_with = "as_utf8_vec")]
+    #[serde(serialize_with = "as_utf8_vec", deserialize_with = "from_utf8_vec")]
     pub value_list: Vec<Vec<u8>>,
 }
 
@@ -334,15 +335,24 @@ fn as_utf8_vec<S: Serializer>(
     val: &[Vec<u8>],
     serializer: S,
 ) -> std::result::Result<S::Ok, S::Error> {
-    serializer.serialize_str(
-        val.iter()
-            .map(|v| {
-                String::from_utf8(v.clone()).unwrap_or_else(|_| "<unknown-not-UTF8>".to_string())
-            })
-            .collect::<Vec<String>>()
-            .join(",")
-            .as_str(),
-    )
+    let mut seq = serializer.serialize_seq(Some(val.len()))?;
+    for v in val {
+        seq.serialize_element(&String::from_utf8_lossy(v))?;
+    }
+    seq.end()
+}
+
+pub fn from_utf8_vec<'de, D>(deserializer: D) -> std::result::Result<Vec<Vec<u8>>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let values = Vec::<String>::deserialize(deserializer)?;
+
+    let values = values
+        .into_iter()
+        .map(|value| value.into_bytes())
+        .collect::<Vec<_>>();
+    Ok(values)
 }
 
 impl From<Partition> for PbPartition {
@@ -373,6 +383,19 @@ mod tests {
 
     use super::*;
 
+    #[test]
+    fn test_de_serialize_partition() {
+        let p = Partition {
+            column_list: vec![b"a".to_vec(), b"b".to_vec()],
+            value_list: vec![b"hi".to_vec(), b",".to_vec()],
+        };
+
+        let output = serde_json::to_string(&p).unwrap();
+        let got: Partition = serde_json::from_str(&output).unwrap();
+
+        assert_eq!(got, p);
+    }
+
     #[test]
     fn test_route_request_trans() {
         let req = RouteRequest {
diff --git a/src/frontend/src/instance/distributed.rs b/src/frontend/src/instance/distributed.rs
index 430c9a936b..130805265f 100644
--- a/src/frontend/src/instance/distributed.rs
+++ b/src/frontend/src/instance/distributed.rs
@@ -947,7 +947,7 @@ PARTITION BY RANGE COLUMNS (b) (
   PARTITION r2 VALUES LESS THAN (MAXVALUE),
 )
 ENGINE=mito",
-                r#"[{"column_list":"b","value_list":"{\"Value\":{\"String\":\"hz\"}}"},{"column_list":"b","value_list":"{\"Value\":{\"String\":\"sh\"}}"},{"column_list":"b","value_list":"\"MaxValue\""}]"#,
+                r#"[{"column_list":["b"],"value_list":["{\"Value\":{\"String\":\"hz\"}}"]},{"column_list":["b"],"value_list":["{\"Value\":{\"String\":\"sh\"}}"]},{"column_list":["b"],"value_list":["\"MaxValue\""]}]"#,
             ),
             (
                 r"
@@ -958,7 +958,7 @@ PARTITION BY RANGE COLUMNS (b, a) (
   PARTITION r2 VALUES LESS THAN (MAXVALUE, MAXVALUE),
 )
 ENGINE=mito",
-                r#"[{"column_list":"b,a","value_list":"{\"Value\":{\"String\":\"hz\"}},{\"Value\":{\"Int32\":10}}"},{"column_list":"b,a","value_list":"{\"Value\":{\"String\":\"sh\"}},{\"Value\":{\"Int32\":20}}"},{"column_list":"b,a","value_list":"\"MaxValue\",\"MaxValue\""}]"#,
+                r#"[{"column_list":["b","a"],"value_list":["{\"Value\":{\"String\":\"hz\"}}","{\"Value\":{\"Int32\":10}}"]},{"column_list":["b","a"],"value_list":["{\"Value\":{\"String\":\"sh\"}}","{\"Value\":{\"Int32\":20}}"]},{"column_list":["b","a"],"value_list":["\"MaxValue\"","\"MaxValue\""]}]"#,
             ),
         ];
         for (sql, expected) in cases {
diff --git a/src/partition/src/partition.rs b/src/partition/src/partition.rs
index 6db2aa5d86..ea2d520900 100644
--- a/src/partition/src/partition.rs
+++ b/src/partition/src/partition.rs
@@ -164,7 +164,7 @@ mod tests {
         };
         let partition: MetaPartition = def.try_into().unwrap();
         assert_eq!(
-            r#"{"column_list":"a,b","value_list":"\"MaxValue\",{\"Value\":{\"Int32\":1}}"}"#,
+            r#"{"column_list":["a","b"],"value_list":["\"MaxValue\"","{\"Value\":{\"Int32\":1}}"]}"#,
             serde_json::to_string(&partition).unwrap(),
         );