address comments

pingcap · Aug 30, 2021 · f00e570 · f00e570
1 parent 05882d5
commit f00e570
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 6 deletions.
diff --git a/dbms/src/AggregateFunctions/AggregateFunctionNull.cpp b/dbms/src/AggregateFunctions/AggregateFunctionNull.cpp
@@ -35,6 +35,9 @@ class AggregateFunctionCombinatorNull final : public IAggregateFunctionCombinato
     AggregateFunctionPtr transformAggregateFunction(
         const AggregateFunctionPtr & nested_function, const DataTypes & arguments, const Array &) const override
     {
+        /// group_concat reuses groupArray and groupUniqArray, it has the special warp function `AggregateFunctionGroupConcat` to process
+        /// the issues of null, but the warp function needs more complex arguments, it is specially added outside,
+        /// instead of being added here, so directly return in this function.
         if (nested_function && (nested_function->getName() == "groupArray" || nested_function->getName() == "groupUniqArray"))
             return nested_function;
         bool has_nullable_types = false;

diff --git a/dbms/src/AggregateFunctions/AggregateFunctionNull.h b/dbms/src/AggregateFunctions/AggregateFunctionNull.h
@@ -1,6 +1,10 @@
 #pragma once
 
+#include <array>
+#include <map>
 #include <common/mem_utils.h>
+
+#include <AggregateFunctions/AggregateFunctionGroupUniqArray.h>
 #include <AggregateFunctions/IAggregateFunction.h>
 #include <Columns/ColumnArray.h>
 #include <Columns/ColumnNullable.h>
@@ -17,10 +21,6 @@
 #include <IO/ReadHelpers.h>
 #include <IO/WriteHelpers.h>
 #include <Interpreters/sortBlock.h>
-
-#include <array>
-#include <map>
-#include "AggregateFunctionGroupUniqArray.h"
 #include <Interpreters/SetVariants.h>
 
 namespace DB
@@ -516,7 +516,7 @@ class AggregateFunctionNullVariadic final : public AggregateFunctionNullBase<res
     std::array<char, MAX_ARGS> is_nullable;    /// Plain array is better than std::vector due to one indirection less.
 };
 
-/// a warp function on the top of groupArray and groupUniqArray
+/// a warp function on the top of groupArray and groupUniqArray, like the AggregateFunctionNull
 
 /// the input argument is in following two types:
 /// 1. only one column with original data type and without order_by items, for example: group_concat(c)
@@ -537,7 +537,7 @@ class AggregateFunctionGroupConcat final : public AggregateFunctionNullBase<resu
           all_columns_names_and_types(all_columns_names_and_types_), collators(collators_)
     {
         if (input_args.size() != 1)
-            throw Exception("Logical error: not single argument is passed to AggregateFunctionGroupConcat", ErrorCodes::LOGICAL_ERROR);
+            throw Exception("Logical error: more than 1 arguments are passed to AggregateFunctionGroupConcat", ErrorCodes::LOGICAL_ERROR);
         nested_type = std::make_shared<DataTypeArray>(removeNullable(input_args[0]));
 
         number_of_concat_items = all_columns_names_and_types.size() - sort_desc.size();