[fix](agg function) incorrect result of map agg

apache · Aug 21, 2024 · 1da90da · 1da90da
1 parent 70983f2
commit 1da90da
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 9 deletions.
diff --git a/be/src/vec/aggregate_functions/aggregate_function_map.h b/be/src/vec/aggregate_functions/aggregate_function_map.h
@@ -18,7 +18,6 @@
 #pragma once
 
 #include <parallel_hashmap/phmap.h>
-#include <string.h>
 
 #include "vec/aggregate_functions/aggregate_function.h"
 #include "vec/aggregate_functions/aggregate_function_simple_factory.h"
@@ -92,7 +91,7 @@ struct AggregateFunctionMapAggData {
             }
 
             if (UNLIKELY(_map.find(key) != _map.end())) {
-                return;
+                continue;
             }
 
             key.data = _arena.insert(key.data, key.size);
@@ -161,9 +160,7 @@ struct AggregateFunctionMapAggData {
         StringRef key;
         for (size_t i = 0; i < size; i++) {
             read_binary(key, buf);
-            if (_map.find(key) != _map.cend()) {
-                continue;
-            }
+            DCHECK(_map.find(key) != _map.cend());
             key.data = _arena.insert(key.data, key.size);
             assert_cast<KeyColumnType&, TypeCheckOnRelease::DISABLE>(*_key_column)
                     .insert_data(key.data, key.size);
@@ -208,9 +205,9 @@ class AggregateFunctionMapAgg final
     void add(AggregateDataPtr __restrict place, const IColumn** columns, ssize_t row_num,
              Arena* arena) const override {
         if (columns[0]->is_nullable()) {
-            auto& nullable_col =
+            const auto& nullable_col =
                     assert_cast<const ColumnNullable&, TypeCheckOnRelease::DISABLE>(*columns[0]);
-            auto& nullable_map = nullable_col.get_null_map_data();
+            const auto& nullable_map = nullable_col.get_null_map_data();
             if (nullable_map[row_num]) {
                 return;
             }
@@ -267,7 +264,7 @@ class AggregateFunctionMapAgg final
 
     void deserialize_from_column(AggregateDataPtr places, const IColumn& column, Arena* arena,
                                  size_t num_rows) const override {
-        auto& col = assert_cast<const ColumnMap&>(column);
+        const auto& col = assert_cast<const ColumnMap&>(column);
         auto* data = &(this->data(places));
         for (size_t i = 0; i != num_rows; ++i) {
             auto map = doris::vectorized::get<Map>(col[i]);
@@ -298,7 +295,7 @@ class AggregateFunctionMapAgg final
                                                  Arena* arena) const override {
         DCHECK(end <= column.size() && begin <= end)
                 << ", begin:" << begin << ", end:" << end << ", column.size():" << column.size();
-        auto& col = assert_cast<const ColumnMap&>(column);
+        const auto& col = assert_cast<const ColumnMap&>(column);
         for (size_t i = begin; i <= end; ++i) {
             auto map = doris::vectorized::get<Map>(col[i]);
             this->data(place).add(map[0], map[1]);

diff --git a/regression-test/data/query_p0/aggregate/map_agg.out b/regression-test/data/query_p0/aggregate/map_agg.out
@@ -43,3 +43,8 @@ V5_3
 -- !multi --
 1	2
 
+-- !test_dumplicate --
+1	\N
+2	bddd
+3	\N
+
diff --git a/regression-test/suites/query_p0/aggregate/map_agg.groovy b/regression-test/suites/query_p0/aggregate/map_agg.groovy
@@ -322,4 +322,35 @@ suite("map_agg") {
                , day
         ) t order by 1, 2;
     """
+
+    sql "DROP TABLE IF EXISTS `test_map_agg_2`;"
+    sql """
+        CREATE TABLE `test_map_agg_2` (
+        `k1` int NULL,
+        `k2` int NULL,
+        `v1` text NULL,
+        `v2` text NULL
+        ) ENGINE=OLAP
+        DUPLICATE KEY(`k1`, `k2`)
+        DISTRIBUTED BY HASH(`k1`) BUCKETS 4
+        PROPERTIES ( 'replication_num' = '1');
+    """
+
+    sql """
+        insert into `test_map_agg_2` values
+            (    3 ,    1 , 'k'    , 'j'    ),
+            (    3 ,    2 , 'a'    , 'a3'   ),
+            (    5 ,    2 , 'a'    , 'a5'   ),
+            (    1 ,    1 , 'ee'   , 'nn'   ),
+            (    1 ,    1 , 'a'    , 'b'    ),
+            (    1 ,    2 , 'a'    , 'b'    ),
+            (    1 ,    3 , 'c'    , 'c'    ),
+            (    2 ,    1 , 'e'    , 'f'    ),
+            (    2 ,    2 , 'a'    , 'a2'   ),
+            (    4 ,    2 , 'b'    , 'bddd' ),
+            (    4 ,    2 , 'a'    , 'a4'   );
+    """
+
+    sql "set experimental_ignore_storage_data_distribution = 0;"
+    qt_test_dumplicate "select k2, m['b'] from (select k2, map_agg(v1, v2) m from `test_map_agg_2` group  by k2) a order by k2;"
  }