You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by yi...@apache.org on 2024/04/07 23:26:51 UTC

(doris) branch master updated: [Bug](array) fix array column core dump in get_shrinked_column as not check type (#33295)

This is an automated email from the ASF dual-hosted git repository.

yiguolei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new f9de308dfff [Bug](array) fix array column core dump in get_shrinked_column as not check type (#33295)
f9de308dfff is described below

commit f9de308dfff51890c192ce8b85e2993fbcb80ff5
Author: zhangstar333 <87...@users.noreply.github.com>
AuthorDate: Mon Apr 8 07:26:42 2024 +0800

    [Bug](array) fix array column core dump in get_shrinked_column as not check type (#33295)
    
    * [Bug](array) fix array column core dump in get_shrinked_column as not check type
    
    * add function could_shrinked_column
---
 be/src/vec/columns/column.h                              |  6 +++++-
 be/src/vec/columns/column_array.cpp                      | 10 +++++++++-
 be/src/vec/columns/column_array.h                        |  1 +
 be/src/vec/columns/column_map.cpp                        | 10 ++++++----
 be/src/vec/columns/column_map.h                          |  1 +
 be/src/vec/columns/column_nullable.cpp                   | 12 ++++++++++--
 be/src/vec/columns/column_nullable.h                     |  2 ++
 be/src/vec/columns/column_string.h                       |  1 +
 be/src/vec/columns/column_struct.cpp                     | 13 +++++++++++--
 be/src/vec/columns/column_struct.h                       |  1 +
 .../data/query_p0/test_array_orderby_limit.out           |  3 +++
 .../suites/query_p0/test_array_orderby_limit.groovy      | 16 ++++++++++++++++
 12 files changed, 66 insertions(+), 10 deletions(-)

diff --git a/be/src/vec/columns/column.h b/be/src/vec/columns/column.h
index 0dd08646d8f..f1bcee1ad7d 100644
--- a/be/src/vec/columns/column.h
+++ b/be/src/vec/columns/column.h
@@ -127,10 +127,14 @@ public:
 
     // shrink the end zeros for CHAR type or ARRAY<CHAR> type
     virtual MutablePtr get_shrinked_column() {
-        LOG(FATAL) << "Cannot clone_resized() column " << get_name();
+        LOG(FATAL) << "Cannot get_shrinked_column() column " << get_name();
         return nullptr;
     }
 
+    // check the column whether could shrinked
+    // now support only in char type, or the nested type in complex type: array{char}, struct{char}, map{char}
+    virtual bool could_shrinked_column() { return false; }
+
     /// Some columns may require finalization before using of other operations.
     virtual void finalize() {}
 
diff --git a/be/src/vec/columns/column_array.cpp b/be/src/vec/columns/column_array.cpp
index 86d31c9223b..7251f1e1798 100644
--- a/be/src/vec/columns/column_array.cpp
+++ b/be/src/vec/columns/column_array.cpp
@@ -118,8 +118,16 @@ ColumnArray::ColumnArray(MutableColumnPtr&& nested_column) : data(std::move(nest
     offsets = ColumnOffsets::create();
 }
 
+bool ColumnArray::could_shrinked_column() {
+    return data->could_shrinked_column();
+}
+
 MutableColumnPtr ColumnArray::get_shrinked_column() {
-    return ColumnArray::create(data->get_shrinked_column(), offsets->assume_mutable());
+    if (could_shrinked_column()) {
+        return ColumnArray::create(data->get_shrinked_column(), offsets->assume_mutable());
+    } else {
+        return ColumnArray::create(data->assume_mutable(), offsets->assume_mutable());
+    }
 }
 
 std::string ColumnArray::get_name() const {
diff --git a/be/src/vec/columns/column_array.h b/be/src/vec/columns/column_array.h
index aa5ded3767c..3176f7a45c6 100644
--- a/be/src/vec/columns/column_array.h
+++ b/be/src/vec/columns/column_array.h
@@ -119,6 +119,7 @@ public:
     }
 
     MutableColumnPtr get_shrinked_column() override;
+    bool could_shrinked_column() override;
 
     /** On the index i there is an offset to the beginning of the i + 1 -th element. */
     using ColumnOffsets = ColumnVector<Offset64>;
diff --git a/be/src/vec/columns/column_map.cpp b/be/src/vec/columns/column_map.cpp
index c1c668ef07c..bec58bfda7b 100644
--- a/be/src/vec/columns/column_map.cpp
+++ b/be/src/vec/columns/column_map.cpp
@@ -455,18 +455,20 @@ ColumnPtr ColumnMap::replicate(const Offsets& offsets) const {
     return res;
 }
 
+bool ColumnMap::could_shrinked_column() {
+    return keys_column->could_shrinked_column() || values_column->could_shrinked_column();
+}
+
 MutableColumnPtr ColumnMap::get_shrinked_column() {
     MutableColumns new_columns(2);
 
-    if (keys_column->is_column_string() || keys_column->is_column_array() ||
-        keys_column->is_column_map() || keys_column->is_column_struct()) {
+    if (keys_column->could_shrinked_column()) {
         new_columns[0] = keys_column->get_shrinked_column();
     } else {
         new_columns[0] = keys_column->get_ptr();
     }
 
-    if (values_column->is_column_string() || values_column->is_column_array() ||
-        values_column->is_column_map() || values_column->is_column_struct()) {
+    if (values_column->could_shrinked_column()) {
         new_columns[1] = values_column->get_shrinked_column();
     } else {
         new_columns[1] = values_column->get_ptr();
diff --git a/be/src/vec/columns/column_map.h b/be/src/vec/columns/column_map.h
index 593ccd06c9c..ed5a692defd 100644
--- a/be/src/vec/columns/column_map.h
+++ b/be/src/vec/columns/column_map.h
@@ -113,6 +113,7 @@ public:
 
     void update_hash_with_value(size_t n, SipHash& hash) const override;
     MutableColumnPtr get_shrinked_column() override;
+    bool could_shrinked_column() override;
     ColumnPtr filter(const Filter& filt, ssize_t result_size_hint) const override;
     size_t filter(const Filter& filter) override;
     ColumnPtr permute(const Permutation& perm, size_t limit) const override;
diff --git a/be/src/vec/columns/column_nullable.cpp b/be/src/vec/columns/column_nullable.cpp
index e20b87af826..dc8853e49af 100644
--- a/be/src/vec/columns/column_nullable.cpp
+++ b/be/src/vec/columns/column_nullable.cpp
@@ -48,9 +48,17 @@ ColumnNullable::ColumnNullable(MutableColumnPtr&& nested_column_, MutableColumnP
     _need_update_has_null = true;
 }
 
+bool ColumnNullable::could_shrinked_column() {
+    return get_nested_column_ptr()->could_shrinked_column();
+}
+
 MutableColumnPtr ColumnNullable::get_shrinked_column() {
-    return ColumnNullable::create(get_nested_column_ptr()->get_shrinked_column(),
-                                  get_null_map_column_ptr());
+    if (could_shrinked_column()) {
+        return ColumnNullable::create(get_nested_column_ptr()->get_shrinked_column(),
+                                      get_null_map_column_ptr());
+    } else {
+        return ColumnNullable::create(get_nested_column_ptr(), get_null_map_column_ptr());
+    }
 }
 
 void ColumnNullable::update_xxHash_with_value(size_t start, size_t end, uint64_t& hash,
diff --git a/be/src/vec/columns/column_nullable.h b/be/src/vec/columns/column_nullable.h
index 5b2d9f124e0..56dbc619a39 100644
--- a/be/src/vec/columns/column_nullable.h
+++ b/be/src/vec/columns/column_nullable.h
@@ -82,7 +82,9 @@ public:
     }
 
     MutableColumnPtr get_shrinked_column() override;
+    bool could_shrinked_column() override;
     bool is_variable_length() const override { return nested_column->is_variable_length(); }
+
     const char* get_family_name() const override { return "Nullable"; }
     std::string get_name() const override { return "Nullable(" + nested_column->get_name() + ")"; }
     MutableColumnPtr clone_resized(size_t size) const override;
diff --git a/be/src/vec/columns/column_string.h b/be/src/vec/columns/column_string.h
index c7393839501..b858ab86bf3 100644
--- a/be/src/vec/columns/column_string.h
+++ b/be/src/vec/columns/column_string.h
@@ -120,6 +120,7 @@ public:
     MutableColumnPtr clone_resized(size_t to_size) const override;
 
     MutableColumnPtr get_shrinked_column() override;
+    bool could_shrinked_column() override { return true; }
 
     Field operator[](size_t n) const override {
         assert(n < size());
diff --git a/be/src/vec/columns/column_struct.cpp b/be/src/vec/columns/column_struct.cpp
index aea9db2be50..d075b040e6d 100644
--- a/be/src/vec/columns/column_struct.cpp
+++ b/be/src/vec/columns/column_struct.cpp
@@ -295,13 +295,22 @@ ColumnPtr ColumnStruct::replicate(const Offsets& offsets) const {
     return ColumnStruct::create(new_columns);
 }
 
+bool ColumnStruct::could_shrinked_column() {
+    const size_t tuple_size = columns.size();
+    for (size_t i = 0; i < tuple_size; ++i) {
+        if (columns[i]->could_shrinked_column()) {
+            return true;
+        }
+    }
+    return false;
+}
+
 MutableColumnPtr ColumnStruct::get_shrinked_column() {
     const size_t tuple_size = columns.size();
     MutableColumns new_columns(tuple_size);
 
     for (size_t i = 0; i < tuple_size; ++i) {
-        if (columns[i]->is_column_string() || columns[i]->is_column_array() ||
-            columns[i]->is_column_map() || columns[i]->is_column_struct()) {
+        if (columns[i]->could_shrinked_column()) {
             new_columns[i] = columns[i]->get_shrinked_column();
         } else {
             new_columns[i] = columns[i]->get_ptr();
diff --git a/be/src/vec/columns/column_struct.h b/be/src/vec/columns/column_struct.h
index d064c447dae..34d1e3ecf0e 100644
--- a/be/src/vec/columns/column_struct.h
+++ b/be/src/vec/columns/column_struct.h
@@ -156,6 +156,7 @@ public:
     int compare_at(size_t n, size_t m, const IColumn& rhs_, int nan_direction_hint) const override;
 
     MutableColumnPtr get_shrinked_column() override;
+    bool could_shrinked_column() override;
 
     void reserve(size_t n) override;
     void resize(size_t n) override;
diff --git a/regression-test/data/query_p0/test_array_orderby_limit.out b/regression-test/data/query_p0/test_array_orderby_limit.out
index abcea7af965..d06cad836aa 100644
--- a/regression-test/data/query_p0/test_array_orderby_limit.out
+++ b/regression-test/data/query_p0/test_array_orderby_limit.out
@@ -2,3 +2,6 @@
 -- !select --
 100	[["abc"]]
 
+-- !select_2 --
+a	{"codes": [123, 456], "props": {"key1":["char1", "char2"]}}
+
diff --git a/regression-test/suites/query_p0/test_array_orderby_limit.groovy b/regression-test/suites/query_p0/test_array_orderby_limit.groovy
index c573b1c7986..2fa3bb9c5ed 100644
--- a/regression-test/suites/query_p0/test_array_orderby_limit.groovy
+++ b/regression-test/suites/query_p0/test_array_orderby_limit.groovy
@@ -45,4 +45,20 @@ suite("test_array_char_orderby", "query") {
     }
 
     qt_select """ select * from ${testTable} order by k1 limit 1 """
+
+    sql "DROP TABLE IF EXISTS unpart_tbl_parquet_struct_3;"
+    sql """
+          CREATE TABLE unpart_tbl_parquet_struct_3 (
+            `col1` CHAR,
+            `col20` STRUCT<codes:ARRAY<INT>,props:MAP<STRING, ARRAY<CHAR(16)>>>
+            )ENGINE=OLAP
+          DUPLICATE KEY(`col1`)
+          COMMENT 'OLAP'
+          DISTRIBUTED BY HASH(`col1`) BUCKETS 5
+          PROPERTIES (
+          "replication_allocation" = "tag.location.default: 1"
+          );
+    """
+    sql """ insert into unpart_tbl_parquet_struct_3 values ('a',STRUCT(ARRAY(123, 456), MAP('key1', ARRAY('char1', 'char2'))) ); """
+    qt_select_2 """ select * from unpart_tbl_parquet_struct_3;"""
 }


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