From 4a0383b11aa459bf34a450ff338f3c370b654d6c Mon Sep 17 00:00:00 2001 From: cindyyyang Date: Mon, 29 Apr 2024 14:22:06 -0700 Subject: [PATCH] Mark 'arbitrary' Presto aggregate function order sensitive (#9646) Summary: As discussed in https://github.com/facebookincubator/velox/issues/9274#issuecomment-2044702864, mark arbitrary aggregate as order sensitive, as a prerequisite for https://github.com/facebookincubator/velox/pull/9482 Pull Request resolved: https://github.com/facebookincubator/velox/pull/9646 Reviewed By: pedroerp Differential Revision: D56710594 Pulled By: kagamiori fbshipit-source-id: ce37c1d6bd707360531fd7bc8da03d638fbc55ac --- velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp | 1 - .../aggregates/tests/AggregationFunctionRegTest.cpp | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp b/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp index 4225b0e88591..d237a1ae5433 100644 --- a/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp +++ b/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp @@ -343,7 +343,6 @@ void registerArbitraryAggregate( inputType->kindName()); } }, - {false /*orderSensitive*/}, withCompanionFunctions, overwrite); } diff --git a/velox/functions/prestosql/aggregates/tests/AggregationFunctionRegTest.cpp b/velox/functions/prestosql/aggregates/tests/AggregationFunctionRegTest.cpp index 85587647d790..40777e874efb 100644 --- a/velox/functions/prestosql/aggregates/tests/AggregationFunctionRegTest.cpp +++ b/velox/functions/prestosql/aggregates/tests/AggregationFunctionRegTest.cpp @@ -64,7 +64,6 @@ TEST_F(AggregationFunctionRegTest, orderSensitive) { "min", "max", "count", - "arbitrary", "bool_and", "bool_or", "bitwise_and_agg", @@ -74,8 +73,7 @@ TEST_F(AggregationFunctionRegTest, orderSensitive) { "count_if", "geometric_mean", "histogram", - "reduce_agg", - "any_value"}; + "reduce_agg"}; aggregate::prestosql::registerAllAggregateFunctions(); exec::aggregateFunctions().withRLock([&](const auto& aggrFuncMap) { for (const auto& entry : aggrFuncMap) { @@ -88,7 +86,7 @@ TEST_F(AggregationFunctionRegTest, orderSensitive) { // Test some but not all order sensitive functions std::set orderSensitiveFunctions = { - "array_agg", "map_agg", "map_union", "set_agg"}; + "array_agg", "arbitrary", "any_value", "map_agg", "map_union", "set_agg"}; exec::aggregateFunctions().withRLock([&](const auto& aggrFuncMap) { for (const auto& entry : aggrFuncMap) { if (entry.second.metadata.orderSensitive) {