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]