You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/10/19 10:26:06 UTC

[GitHub] [doris] xy720 commented on a diff in pull request #13428: [feature-wip](array) remove array config and check array nested depth

xy720 commented on code in PR #13428:
URL: https://github.com/apache/doris/pull/13428#discussion_r999229603


##########
be/src/vec/functions/function_cast.h:
##########
@@ -332,9 +334,16 @@ struct ConvertImplGenericFromString {
                     continue;
                 }
                 ReadBuffer read_buffer((char*)(val.data), val.size);
-                RETURN_IF_ERROR(data_type_to->from_string(read_buffer, col_to));
+                Status st = data_type_to->from_string(read_buffer, col_to);
+                // if parsing failed, will return null
+                (*vec_null_map_to)[i] = !st.ok();
+                if (!st.ok()) {
+                    col_to->insert_default();
+                }
             }
-            block.replace_by_position(result, std::move(col_to));
+            // block.replace_by_position(result, std::move(col_to));

Review Comment:
   remove this



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Type.java:
##########
@@ -46,12 +46,8 @@
  * as abstract methods that subclasses must implement.
  */
 public abstract class Type {
-    // Maximum nesting depth of a type. This limit was determined experimentally by
-    // org.apache.doris.rewrite.FoldConstantsRule.apply generating and scanning
-    // deeply nested Parquet and Avro files. In those experiments, we exceeded
-    // the stack space in the scanner (which uses recursion for dealing with
-    // nested types) at a nesting depth between 200 and 300 (200 worked, 300 crashed).
-    public static int MAX_NESTING_DEPTH = 2;
+    // Currently only support Array type with max 9 depths.
+    public static int MAX_NESTING_DEPTH = 9;

Review Comment:
   Why is 9?



##########
be/src/vec/exec/vbroker_scanner.cpp:
##########
@@ -91,9 +91,10 @@ Status VBrokerScanner::_fill_dest_columns(const Slice& line,
         return Status::OK();
     }
 
-    if (!check_array_format(_split_values)) {
-        return Status::OK();
-    }
+    // This check is meaningless, should be removed

Review Comment:
   If we remove this, may be we should also covert the invalid array value in this row to null. Keep the same logic as cast function.



-- 
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-unsubscribe@doris.apache.org

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


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