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


##########
be/src/vec/functions/function_cast.h:
##########
@@ -2013,8 +2013,16 @@ class FunctionCast final : public IFunctionBase {
             if (!variant.is_finalized()) {
                 variant.assume_mutable()->finalize();
             }
-
-            if (variant.is_scalar_variant()) {
+            // It's important to convert as many elements as possible in this 
context. For instance,
+            // if the root of this variant column is a number column, 
converting it to a number column
+            // is acceptable. However, if the destination type is a string and 
root is none scalar root, then
+            // we should convert the entire tree to a string.
+            bool is_root_valuable =
+                    variant.is_scalar_variant() ||
+                    (!variant.is_null_root() &&
+                     
!WhichDataType(remove_nullable(variant.get_root_type())).is_nothing() &&
+                     !WhichDataType(data_type_to).is_string());

Review Comment:
   data_type_to is not string?



##########
be/src/vec/json/path_in_data.cpp:
##########
@@ -151,10 +151,17 @@ size_t PathInData::Hash::operator()(const PathInData& 
value) const {
 }
 
 PathInData PathInData::pop_front() const {
+    return pop_nfront(1);
+}
+
+PathInData PathInData::pop_nfront(size_t n) const {
+    if (n >= parts.size()) {
+        return {};
+    }
     PathInData new_path;
     Parts new_parts;
     if (!parts.empty()) {
-        std::copy(parts.begin() + 1, parts.end(), 
std::back_inserter(new_parts));
+        std::copy(parts.begin() + n, parts.end(), 
std::back_inserter(new_parts));

Review Comment:
   Does 'pop_nfront' remove n front element using std::copy?



##########
be/src/olap/rowset/segment_v2/hierarchical_data_reader.cpp:
##########
@@ -32,11 +32,12 @@ namespace doris {
 namespace segment_v2 {
 
 Status HierarchicalDataReader::create(std::unique_ptr<ColumnIterator>* reader,
+                                      vectorized::PathInData path,
                                       const SubcolumnColumnReaders::Node* node,
                                       const SubcolumnColumnReaders::Node* root,
                                       bool output_as_raw_json) {
     // None leave node need merge with root
-    auto* stream_iter = new HierarchicalDataReader(node->path, 
output_as_raw_json);
+    auto* stream_iter = new HierarchicalDataReader(path, output_as_raw_json);

Review Comment:
   what's the difference?



##########
be/src/vec/json/path_in_data.cpp:
##########
@@ -151,10 +151,17 @@ size_t PathInData::Hash::operator()(const PathInData& 
value) const {
 }
 
 PathInData PathInData::pop_front() const {
+    return pop_nfront(1);
+}
+
+PathInData PathInData::pop_nfront(size_t n) const {

Review Comment:
   pop_nfront is only used once, why change 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: commits-unsubscr...@doris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to