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


##########
be/src/vec/functions/array/function_array_intersect.cpp:
##########
@@ -79,8 +76,66 @@ struct IntersectAction {
     }
 };
 
-using FunctionArrayIntersect =
-        FunctionArrayBinary<ArraySetImpl<SetOperation::INTERSECT>, 
NameArrayIntersect>;
+class FunctionArrayIntersect : public IFunction {
+public:
+    static constexpr auto name = "array_intersect";
+    static FunctionPtr create() { return 
std::make_shared<FunctionArrayIntersect>(); }
+
+    /// Get function name.
+    String get_name() const override { return name; }
+
+    bool is_variadic() const override { return true; }
+
+    bool use_default_implementation_for_constants() const override { return 
true; }
+
+    size_t get_number_of_arguments() const override { return 0; }
+
+    DataTypePtr get_return_type_impl(const DataTypes& arguments) const 
override {
+        DCHECK(arguments.size() >= 2)
+                << "function: " << get_name() << ", arguments should not less 
than 2";

Review Comment:
   ```suggestion
                   << "function: " << get_name() << ", arguments should not 
larger equal than 2";
   ```



##########
be/src/vec/functions/array/function_array_intersect.cpp:
##########
@@ -79,8 +76,66 @@ struct IntersectAction {
     }
 };
 
-using FunctionArrayIntersect =
-        FunctionArrayBinary<ArraySetImpl<SetOperation::INTERSECT>, 
NameArrayIntersect>;
+class FunctionArrayIntersect : public IFunction {
+public:
+    static constexpr auto name = "array_intersect";
+    static FunctionPtr create() { return 
std::make_shared<FunctionArrayIntersect>(); }
+
+    /// Get function name.
+    String get_name() const override { return name; }
+
+    bool is_variadic() const override { return true; }
+
+    bool use_default_implementation_for_constants() const override { return 
true; }
+
+    size_t get_number_of_arguments() const override { return 0; }
+
+    DataTypePtr get_return_type_impl(const DataTypes& arguments) const 
override {
+        DCHECK(arguments.size() >= 2)
+                << "function: " << get_name() << ", arguments should not less 
than 2";
+        for (size_t i = 0; i < arguments.size(); ++i) {
+            DCHECK(is_array(arguments[i])) << i << "-th element is not array 
type";
+            const auto* array_type = 
check_and_get_data_type<DataTypeArray>(arguments[i].get());

Review Comment:
   is_array(arguments[i])) already check the data type, why check again?



##########
be/src/vec/functions/array/function_array_intersect.cpp:
##########
@@ -79,8 +76,66 @@ struct IntersectAction {
     }
 };
 
-using FunctionArrayIntersect =
-        FunctionArrayBinary<ArraySetImpl<SetOperation::INTERSECT>, 
NameArrayIntersect>;
+class FunctionArrayIntersect : public IFunction {

Review Comment:
   How about create a new class FunctionArrayNary, Just like 
FunctionArrayBinary but it can have n arguments.



##########
be/src/vec/functions/array/function_array_intersect.cpp:
##########
@@ -79,8 +76,66 @@ struct IntersectAction {
     }
 };
 
-using FunctionArrayIntersect =
-        FunctionArrayBinary<ArraySetImpl<SetOperation::INTERSECT>, 
NameArrayIntersect>;
+class FunctionArrayIntersect : public IFunction {
+public:
+    static constexpr auto name = "array_intersect";
+    static FunctionPtr create() { return 
std::make_shared<FunctionArrayIntersect>(); }
+
+    /// Get function name.
+    String get_name() const override { return name; }
+
+    bool is_variadic() const override { return true; }
+
+    bool use_default_implementation_for_constants() const override { return 
true; }
+
+    size_t get_number_of_arguments() const override { return 0; }
+
+    DataTypePtr get_return_type_impl(const DataTypes& arguments) const 
override {
+        DCHECK(arguments.size() >= 2)
+                << "function: " << get_name() << ", arguments should not less 
than 2";
+        for (size_t i = 0; i < arguments.size(); ++i) {
+            DCHECK(is_array(arguments[i])) << i << "-th element is not array 
type";
+            const auto* array_type = 
check_and_get_data_type<DataTypeArray>(arguments[i].get());
+            DCHECK(array_type) << "function: " << get_name() << " " << i + 1
+                               << "-th argument is not array";
+        }
+        DataTypePtr res_data_type =
+                
ArraySetImpl<SetOperation::INTERSECT>::get_return_type(arguments);
+        return res_data_type;
+    }
+
+    Status execute_impl(FunctionContext* context, Block& block, const 
ColumnNumbers& arguments,
+                        size_t result, size_t input_rows_count) override {
+        CHECK(arguments.size() >= 2);
+        const auto& [left_column, left_const] =
+                unpack_if_const(block.get_by_position(arguments[0]).column);
+        ColumnPtr res_ptr = left_column;
+        ColumnArrayExecutionData left_data;
+        ColumnArrayExecutionData right_data;
+        for (int i = 1; i < arguments.size(); ++i) {

Review Comment:
   You should put this loop in ArraySetImpl to avoid calling 
`assemble_column_array` and `extract_column_array_info` too many times.
   
   For example, a draft:
   
   ```
   using ColumnArrayExecutionDatas = std::vector<ColumnArrayExecutionData>;
   
   template <typename Impl, typename Name>
   class FunctionArrayNary {
   execute_impl(FunctionContext* context, Block& block, const ColumnNumbers& 
arguments,
                           size_t result, size_t input_rows_count) override {
       ...
       Columns cols(arguments.size());
       std::unique_ptr<bool[]> col_const = 
std::make_unique<bool[]>(arguments.size());
       for (int i = 0; i < arguments.size(); ++i) {
           std::tie(cols[i], col_const[i]) =
                   unpack_if_const(block.get_by_position(arguments[i]).column);
           extract_column_array_info(*col[i], datas[i]);
       }
       ...    
       Impl::execute(res_ptr, datas, col_const);
       ...
   }
   }
   
   struct ArraySetImpl {
   ...
   
   static Status execute(ColumnPtr& res_ptr, ColumnArrayExecutionDatas datas, 
std::unique_ptr<bool[]> col_const) {
       ...
       ColumnArrayMutableData dst;
       dst = create_mutable_data(datas[0].nested_col);
       for (int i = 1; i < datas.size(); ++i) {
           if (col_const[i - 1]) {
               _execute_internal<true, false, ALL_COLUMNS_SIMPLE>(dst, 
dst.to_execution_data(), datas[i]);
           } else if (col_const[i]) {
               _execute_internal<false, true, ALL_COLUMNS_SIMPLE>(dst, 
dst.to_execution_data(), datas[i]);
           } else {
               _execute_internal<false, false, ALL_COLUMNS_SIMPLE>(dst, 
dst.to_execution_data(), datas[i]);
           }
       }
       res_ptr = assemble_column_array(dst);
       ...
   }
   
   ...
   }
   ```



-- 
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