From 053b9b223072705c2eb8844eca61f25d6cb10767 Mon Sep 17 00:00:00 2001
From: taytzehao <jtth95@gmail.com>
Date: Mon, 25 Sep 2023 20:47:54 +0800
Subject: [PATCH] feedback amendment

---
 proto/expr.proto                          |  3 ++-
 src/common/src/types/jsonb.rs             |  5 ----
 src/expr/src/vector_op/jsonb_concat.rs    | 28 +++++++++++++++++++----
 src/frontend/src/binder/expr/binary_op.rs |  8 +++----
 src/frontend/src/binder/expr/function.rs  |  1 -
 5 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/proto/expr.proto b/proto/expr.proto
index f634be2d923c4..68be574349408 100644
--- a/proto/expr.proto
+++ b/proto/expr.proto
@@ -206,7 +206,7 @@ message ExprNode {
     HEX_TO_INT256 = 560;
 
     // Jsonb functions
-    JSONB_CAT = 575;
+    
 
     // jsonb -> int, jsonb -> text, jsonb #> text[] that returns jsonb
     JSONB_ACCESS_INNER = 600;
@@ -215,6 +215,7 @@ message ExprNode {
     JSONB_TYPEOF = 602;
     JSONB_ARRAY_LENGTH = 603;
     IS_JSON = 604;
+    JSONB_CAT = 605;
 
     // Non-pure functions below (> 1000)
     // ------------------------
diff --git a/src/common/src/types/jsonb.rs b/src/common/src/types/jsonb.rs
index 7e64f460be5bc..77eb757144081 100644
--- a/src/common/src/types/jsonb.rs
+++ b/src/common/src/types/jsonb.rs
@@ -210,11 +210,6 @@ impl From<Value> for JsonbVal {
     }
 }
 
-impl<'a> From<&'a Value> for JsonbRef<'a> {
-    fn from(value: &'a Value) -> Self {
-        JsonbRef(value)
-    }
-}
 
 impl<'a> JsonbRef<'a> {
     pub fn memcmp_serialize(
diff --git a/src/expr/src/vector_op/jsonb_concat.rs b/src/expr/src/vector_op/jsonb_concat.rs
index 179f869427cc2..f702b347749a5 100644
--- a/src/expr/src/vector_op/jsonb_concat.rs
+++ b/src/expr/src/vector_op/jsonb_concat.rs
@@ -41,11 +41,21 @@ use serde_json::{json, Value};
 /// SELECT '1'::jsonb || '2'::jsonb;
 /// ----
 /// [1, 2]
+/// 
+/// query T
+/// SELECT '[1,2]'::jsonb || NULL::jsonb;
+/// ----
+/// NULL
+/// 
+/// query T
+/// SELECT NULL::jsonb || '[1,2]'::jsonb;
+/// ----
+/// NULL
 /// ```
 #[function("jsonb_cat(jsonb, jsonb) -> jsonb")]
-pub fn jsonb_cat(left: JsonbRef<'_>, right: JsonbRef<'_>) -> JsonbVal {
-    let left_val = left.value().clone();
-    let right_val = right.value().clone();
+pub fn jsonb_cat(left: Option<JsonbRef<'_>>, right: Option<JsonbRef<'_>>) -> JsonbVal {
+    let left_val = left.map_or(Value::Null,|v| v.value().clone());
+    let right_val = right.map_or(Value::Null,|v| v.value().clone());
     match (left_val, right_val) {
         // left and right are object based.
         // This would have left:{'a':1}, right:{'b':2} -> {'a':1,'b':2}
@@ -65,11 +75,21 @@ pub fn jsonb_cat(left: JsonbRef<'_>, right: JsonbRef<'_>) -> JsonbVal {
         // One operand is an array, and the other is a single element.
         // This would insert the non-array value as another element into the array
         // Eg left:[1,2] right: {'a':1} -> [1,2,{'a':1}]
-        (Value::Array(mut left_arr), single_val) | (single_val, Value::Array(mut left_arr)) => {
+        (Value::Array(mut left_arr), single_val) 
+        | (single_val, Value::Array(mut left_arr))  
+        if single_val != Value::Null => {
             left_arr.push(single_val);
             JsonbVal::from(Value::Array(left_arr))
         }
 
+        // Either left/right is None
+        // This return a None value
+        // This would have left:[1,2], right:None -> None
+        (Value::Null, _) 
+        | (_, Value::Null) => {
+            JsonbVal::from(Value::Null)
+        }
+
         // Both are non-array inputs.
         // Both elements would be placed together in an array
         // Eg left:1 right: 2 -> [1,2]
diff --git a/src/frontend/src/binder/expr/binary_op.rs b/src/frontend/src/binder/expr/binary_op.rs
index d0754447e0de0..404cb6dbae5b9 100644
--- a/src/frontend/src/binder/expr/binary_op.rs
+++ b/src/frontend/src/binder/expr/binary_op.rs
@@ -108,12 +108,12 @@ impl Binder {
                         ExprType::ConcatOp
                     }
 
-                    (Some(DataType::Jsonb), Some(DataType::Jsonb)) => ExprType::JsonbCat,
+                    (Some(DataType::Jsonb), Some(DataType::Jsonb)) 
+                    | (Some(DataType::Jsonb), None) 
+                    | (None, Some(DataType::Jsonb))=> ExprType::JsonbCat,
 
                     // bytea (and varbit, tsvector, tsquery)
-                    (Some(t @ DataType::Jsonb), None)
-                    | (None, Some(t @ DataType::Jsonb))
-                    | (Some(t @ DataType::Bytea), Some(DataType::Bytea))
+                    (Some(t @ DataType::Bytea), Some(DataType::Bytea))
                     | (Some(t @ DataType::Bytea), None)
                     | (None, Some(t @ DataType::Bytea)) => {
                         return Err(ErrorCode::BindError(format!(
diff --git a/src/frontend/src/binder/expr/function.rs b/src/frontend/src/binder/expr/function.rs
index 9d59511d53465..c3bdf1febcccb 100644
--- a/src/frontend/src/binder/expr/function.rs
+++ b/src/frontend/src/binder/expr/function.rs
@@ -843,7 +843,6 @@ impl Binder {
                 // int256
                 ("hex_to_int256", raw_call(ExprType::HexToInt256)),
                 // jsonb
-                ("jsonb_cat", raw_call(ExprType::JsonbCat)),
                 ("jsonb_object_field", raw_call(ExprType::JsonbAccessInner)),
                 ("jsonb_array_element", raw_call(ExprType::JsonbAccessInner)),
                 ("jsonb_object_field_text", raw_call(ExprType::JsonbAccessStr)),