From 106fb45978e157f0497c48b3eed6a2509de486e0 Mon Sep 17 00:00:00 2001
From: Amit Dutta <adutta@fb.com>
Date: Wed, 22 Nov 2023 08:31:24 -0800
Subject: [PATCH] Add any_value alias for arbitrary aggregate. (#7689)

Summary:

Presto added `any_value` alias for arbitrary aggregate in
https://github.com/prestodb/presto/pull/21389

Differential Revision: D51521293
---
 velox/docs/functions/presto/aggregate.rst             |  4 ++++
 velox/functions/prestosql/aggregates/AggregateNames.h |  1 +
 .../prestosql/aggregates/ArbitraryAggregate.cpp       | 11 +++++------
 .../aggregates/RegisterAggregateFunctions.cpp         |  3 +--
 .../prestosql/aggregates/tests/ArbitraryTest.cpp      |  4 ++--
 5 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/velox/docs/functions/presto/aggregate.rst b/velox/docs/functions/presto/aggregate.rst
index 79d53496e71..89e891a0adf 100644
--- a/velox/docs/functions/presto/aggregate.rst
+++ b/velox/docs/functions/presto/aggregate.rst
@@ -21,6 +21,10 @@ General Aggregate Functions
 
     Returns an arbitrary non-null value of ``x``, if one exists.
 
+.. function:: any_value(x) -> [same as x]
+
+    This is an alias for :func:`arbitrary(x)`.
+
 .. function:: array_agg(x) -> array<[same as x]>
 
     Returns an array created from the input ``x`` elements. Ignores null
diff --git a/velox/functions/prestosql/aggregates/AggregateNames.h b/velox/functions/prestosql/aggregates/AggregateNames.h
index 8c6691588ef..1412ae84ea2 100644
--- a/velox/functions/prestosql/aggregates/AggregateNames.h
+++ b/velox/functions/prestosql/aggregates/AggregateNames.h
@@ -23,6 +23,7 @@ const char* const kApproxMostFrequent = "approx_most_frequent";
 const char* const kApproxPercentile = "approx_percentile";
 const char* const kApproxSet = "approx_set";
 const char* const kArbitrary = "arbitrary";
+const char* const kAnyValue = "any_value";
 const char* const kArrayAgg = "array_agg";
 const char* const kAvg = "avg";
 const char* const kBitwiseAnd = "bitwise_and_agg";
diff --git a/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp b/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp
index cf570f3fa2d..fd7197430ba 100644
--- a/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp
+++ b/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp
@@ -256,8 +256,7 @@ class NonNumericArbitrary : public exec::Aggregate {
 
 } // namespace
 
-exec::AggregateRegistrationResult registerArbitraryAggregate(
-    const std::string& prefix) {
+void registerArbitraryAggregate(const std::string& prefix) {
   std::vector<std::shared_ptr<exec::AggregateFunctionSignature>> signatures{
       exec::AggregateFunctionSignatureBuilder()
           .typeVariable("T")
@@ -266,11 +265,11 @@ exec::AggregateRegistrationResult registerArbitraryAggregate(
           .argumentType("T")
           .build()};
 
-  auto name = prefix + kArbitrary;
-  return exec::registerAggregateFunction(
-      name,
+  std::vector<std::string> names = {prefix + kArbitrary, prefix + kAnyValue};
+  exec::registerAggregateFunction(
+      names,
       std::move(signatures),
-      [name](
+      [name = names.front()](
           core::AggregationNode::Step step,
           const std::vector<TypePtr>& argTypes,
           const TypePtr& /*resultType*/,
diff --git a/velox/functions/prestosql/aggregates/RegisterAggregateFunctions.cpp b/velox/functions/prestosql/aggregates/RegisterAggregateFunctions.cpp
index 1e9ffdbfad5..c7d2e3c6f13 100644
--- a/velox/functions/prestosql/aggregates/RegisterAggregateFunctions.cpp
+++ b/velox/functions/prestosql/aggregates/RegisterAggregateFunctions.cpp
@@ -23,8 +23,7 @@ extern exec::AggregateRegistrationResult registerApproxMostFrequentAggregate(
 extern exec::AggregateRegistrationResult registerApproxPercentileAggregate(
     const std::string& prefix,
     bool withCompanionFunctions);
-extern exec::AggregateRegistrationResult registerArbitraryAggregate(
-    const std::string& prefix);
+void registerArbitraryAggregate(const std::string& prefix);
 extern exec::AggregateRegistrationResult registerArrayAggAggregate(
     const std::string& prefix,
     bool withCompanionFunctions);
diff --git a/velox/functions/prestosql/aggregates/tests/ArbitraryTest.cpp b/velox/functions/prestosql/aggregates/tests/ArbitraryTest.cpp
index edfbaa69fbd..fcab97ce63d 100644
--- a/velox/functions/prestosql/aggregates/tests/ArbitraryTest.cpp
+++ b/velox/functions/prestosql/aggregates/tests/ArbitraryTest.cpp
@@ -43,10 +43,10 @@ TEST_F(ArbitraryTest, noNulls) {
   std::vector<std::string> aggregates = {
       "arbitrary(c1)",
       "arbitrary(c2)",
-      "arbitrary(c3)",
+      "any_value(c3)",
       "arbitrary(c4)",
       "arbitrary(c5)",
-      "arbitrary(c6)"};
+      "any_value(c6)"};
 
   // We do not test with TableScan because having two input splits makes the
   // result non-deterministic.