Address comments and rebase

facebookincubator · Mar 30, 2024 · c4dd224 · c4dd224
1 parent b89d2ee
commit c4dd224
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 6 deletions.
diff --git a/velox/docs/functions/spark/aggregate.rst b/velox/docs/functions/spark/aggregate.rst
@@ -53,6 +53,11 @@ General Aggregate Functions
 
     ``hash`` cannot be null.
 
+.. spark:function:: collect_list(x) -> array<[same as x]>
+
+    Returns an array created from the input ``x`` elements. Ignores null
+    inputs, and return an empty array when all inputs are null.
+
 .. spark:function:: first(x) -> x
 
     Returns the first value of `x`.

diff --git a/velox/functions/sparksql/aggregates/CollectListAggregate.cpp b/velox/functions/sparksql/aggregates/CollectListAggregate.cpp
@@ -16,6 +16,9 @@
 
 #include "velox/functions/sparksql/aggregates/CollectListAggregate.h"
 
+#include "velox/exec/SimpleAggregateAdapter.h"
+#include "velox/functions/lib/aggregates/ValueList.h"
+
 using namespace facebook::velox::aggregate;
 using namespace facebook::velox::exec;
 
@@ -48,8 +51,6 @@ class CollectListAggregate {
   struct AccumulatorType {
     ValueList elements_;
 
-    AccumulatorType() = delete;
-
     explicit AccumulatorType(HashStringAllocator* /*allocator*/)
         : elements_{} {}
 
@@ -81,7 +82,7 @@ class CollectListAggregate {
         bool /*nonNullGroup*/,
         exec::out_type<IntermediateType>& out) {
       // If the group's accumulator is null, the corresponding intermediate
-      // result is an empty list.
+      // result is an empty array.
       copyValueListToArrayWriter(out, elements_);
       return true;
     }
@@ -90,7 +91,7 @@ class CollectListAggregate {
         bool /*nonNullGroup*/,
         exec::out_type<OutputType>& out) {
       // If the group's accumulator is null, the corresponding result is an
-      // empty list.
+      // empty array.
       copyValueListToArrayWriter(out, elements_);
       return true;
     }

diff --git a/velox/functions/sparksql/aggregates/CollectListAggregate.h b/velox/functions/sparksql/aggregates/CollectListAggregate.h
@@ -16,8 +16,7 @@
 
 #pragma once
 
-#include "velox/exec/SimpleAggregateAdapter.h"
-#include "velox/functions/lib/aggregates/ValueList.h"
+#include <string>
 
 namespace facebook::velox::functions::aggregate::sparksql {