xiaokang commented on code in PR #24087:
URL: https://github.com/apache/doris/pull/24087#discussion_r1323792031


##########
be/src/vec/data_types/serde/data_type_nullable_serde.cpp:
##########
@@ -121,10 +120,11 @@ Status 
DataTypeNullableSerDe::deserialize_one_cell_from_json(IColumn& column, Sl
     auto& null_column = assert_cast<ColumnNullable&>(column);
     // TODO(Amory) make null literal configurable
 
+    // only slice trim quote return true make sure slice is quoted and 
converted_from_string make
+    // sure slice is from string parse , we can parse this "null" literal as 
string "null" to
+    // nested column , otherwise we insert null to null column
     if (!(options.converted_from_string && slice.trim_quote())) {
-        //for map<string,string> type : {"abc","NULL"} , the NULL is string , 
instead of null values
-        if (slice.size == 4 && slice[0] == 'N' && slice[1] == 'U' && slice[2] 
== 'L' &&
-            slice[3] == 'L') {
+        if (Slice::mem_equal("null", slice.data, 4)) {

Review Comment:
   why change NULL to null ?



##########
be/src/vec/functions/function_cast.h:
##########
@@ -571,8 +574,9 @@ struct ConvertImplGenericFromString {
                     }
                     continue;
                 }
-                ReadBuffer read_buffer((char*)(val.data), val.size);
-                Status st = data_type_to->from_string(read_buffer, col_to);
+                Slice string_slice(val.data, val.size);
+                Status st = serde->deserialize_one_cell_from_json(*col_to, 
string_slice,

Review Comment:
   deserialize_one_cell_from_json, not deserialize_one_cell_from_text?



##########
be/src/vec/data_types/serde/data_type_bitmap_serde.cpp:
##########
@@ -33,6 +33,25 @@ namespace doris {
 namespace vectorized {
 class IColumn;
 
+Status DataTypeBitMapSerDe::deserialize_column_from_json_vector(

Review Comment:
   unrelasted to this pr



##########
be/src/vec/data_types/serde/data_type_map_serde.cpp:
##########
@@ -280,6 +280,15 @@ Status 
DataTypeMapSerDe::deserialize_one_cell_from_json(IColumn& column, Slice&
         ++elem_deserialized;
     }
 
+    if (nested_key_column.size() != nested_val_column.size()) {
+        // nested key and value should always same size otherwise we should 
popback wrong data

Review Comment:
   why not just return error status? Doing popback will lose some key/value.



##########
be/src/vec/data_types/serde/data_type_nullable_serde.cpp:
##########
@@ -121,10 +120,11 @@ Status 
DataTypeNullableSerDe::deserialize_one_cell_from_json(IColumn& column, Sl
     auto& null_column = assert_cast<ColumnNullable&>(column);
     // TODO(Amory) make null literal configurable
 
+    // only slice trim quote return true make sure slice is quoted and 
converted_from_string make
+    // sure slice is from string parse , we can parse this "null" literal as 
string "null" to
+    // nested column , otherwise we insert null to null column
     if (!(options.converted_from_string && slice.trim_quote())) {
-        //for map<string,string> type : {"abc","NULL"} , the NULL is string , 
instead of null values
-        if (slice.size == 4 && slice[0] == 'N' && slice[1] == 'U' && slice[2] 
== 'L' &&
-            slice[3] == 'L') {
+        if (Slice::mem_equal("null", slice.data, 4)) {

Review Comment:
   slice.size == 4 should be checked since Slice::mem_equal does not check it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to