From 1dc53cb3ed46f80d44da81a0e9680a77cf8ee7b5 Mon Sep 17 00:00:00 2001 From: Minhan Cao Date: Fri, 22 Mar 2024 05:27:48 -0700 Subject: [PATCH 1/3] Implementing long decimal support for arbitrary function --- .../aggregates/ArbitraryAggregate.cpp | 5 ++ .../aggregates/tests/ArbitraryTest.cpp | 54 +++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp b/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp index 65897782f1e6..08e2db0af743 100644 --- a/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp +++ b/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp @@ -294,6 +294,11 @@ void registerArbitraryAggregate( return std::make_unique>(inputType); case TypeKind::BIGINT: return std::make_unique>(inputType); + case TypeKind::HUGEINT: + if (inputType->isLongDecimal()) { + return std::make_unique>(inputType); + } + VELOX_UNREACHABLE(); case TypeKind::REAL: return std::make_unique>(inputType); case TypeKind::DOUBLE: diff --git a/velox/functions/prestosql/aggregates/tests/ArbitraryTest.cpp b/velox/functions/prestosql/aggregates/tests/ArbitraryTest.cpp index 67b7f66f11dc..afb382a18c7c 100644 --- a/velox/functions/prestosql/aggregates/tests/ArbitraryTest.cpp +++ b/velox/functions/prestosql/aggregates/tests/ArbitraryTest.cpp @@ -349,6 +349,60 @@ TEST_F(ArbitraryTest, interval) { testAggregations({data}, {}, {"arbitrary(c2)"}, "SELECT null"); } +TEST_F(ArbitraryTest, longDecimal) { + auto data = makeRowVector({// Grouping key. + makeFlatVector({1, 1, 2, 2, 3, 3, 4, 4}), + makeNullableFlatVector( + {HugeInt::build(10, 100), + HugeInt::build(10, 100), + HugeInt::build(10, 200), + HugeInt::build(10, 200), + std::nullopt, + std::nullopt, + std::nullopt, + HugeInt::build(10, 400)}, + DECIMAL(38, 8))}); + + auto expectedResult = makeRowVector({ + makeFlatVector({1, 2, 3, 4}), + makeNullableFlatVector( + {HugeInt::build(10, 100), + HugeInt::build(10, 200), + std::nullopt, + HugeInt::build(10, 400)}, + DECIMAL(38, 8)), + }); + + testAggregations({data}, {"c0"}, {"arbitrary(c1)"}, {expectedResult}); +} + +TEST_F(ArbitraryTest, shortDecimal) { + auto data = makeRowVector({// Grouping key. + makeFlatVector({1, 1, 2, 2, 3, 3, 4, 4}), + makeNullableFlatVector( + {10000000000000000, + 10000000000000000, + 20000000000000000, + 20000000000000000, + std::nullopt, + std::nullopt, + std::nullopt, + 40000000000000000}, + DECIMAL(15, 2))}); + + auto expectedResult = makeRowVector({ + makeFlatVector({1, 2, 3, 4}), + makeNullableFlatVector( + {10000000000000000, + 20000000000000000, + std::nullopt, + 40000000000000000}, + DECIMAL(15, 2)), + }); + + testAggregations({data}, {"c0"}, {"arbitrary(c1)"}, {expectedResult}); +} + class ArbitraryWindowTest : public WindowTestBase {}; TEST_F(ArbitraryWindowTest, basic) { From 74ed85b032e7124f04a0d9d4b337c1ce534a5d8c Mon Sep 17 00:00:00 2001 From: Minhan Cao Date: Tue, 2 Apr 2024 13:36:28 -0700 Subject: [PATCH 2/3] Changed VELOX_UNREACHABLE to VELOX_NYI for long decimal support for arbitrary function, ArbitraryAggregate.cpp --- velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp b/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp index 08e2db0af743..568a1d4cd848 100644 --- a/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp +++ b/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp @@ -298,7 +298,7 @@ void registerArbitraryAggregate( if (inputType->isLongDecimal()) { return std::make_unique>(inputType); } - VELOX_UNREACHABLE(); + VELOX_NYI(); case TypeKind::REAL: return std::make_unique>(inputType); case TypeKind::DOUBLE: From e4ae68ed4e5fad9116e53799b927627f61d510ee Mon Sep 17 00:00:00 2001 From: Minhan Cao Date: Wed, 10 Apr 2024 14:43:49 -0700 Subject: [PATCH 3/3] Added accumulatorAlignmentSize() to ArbitraryAggregate.cpp --- .../prestosql/aggregates/ArbitraryAggregate.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp b/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp index 568a1d4cd848..bfc4bfd41521 100644 --- a/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp +++ b/velox/functions/prestosql/aggregates/ArbitraryAggregate.cpp @@ -39,6 +39,10 @@ class ArbitraryAggregate : public SimpleNumericAggregate { return sizeof(T); } + int32_t accumulatorAlignmentSize() const override { + return 1; + } + void initializeNewGroups( char** groups, folly::Range indices) override { @@ -136,6 +140,14 @@ class ArbitraryAggregate : public SimpleNumericAggregate { } }; +/// Override 'accumulatorAlignmentSize' for UnscaledLongDecimal values as it +/// uses int128_t type. Some CPUs don't support misaligned access to int128_t +/// type. +template <> +inline int32_t ArbitraryAggregate::accumulatorAlignmentSize() const { + return static_cast(sizeof(int128_t)); +} + // Arbitrary for non-numeric types. We always keep the first (non-NULL) element // seen. Arbitrary (x) will produce partial and final aggregations of type x. class NonNumericArbitrary : public exec::Aggregate {