From 1ae7321ad760e2cd5a91a64190328d167c375e78 Mon Sep 17 00:00:00 2001 From: PHILO-HE Date: Tue, 31 Oct 2023 14:14:32 +0800 Subject: [PATCH 01/28] Implement Spark decimal add and subtract (5791) --- .../functions/sparksql/DecimalArithmetic.cpp | 283 ++++++++++++++++++ velox/functions/sparksql/DecimalUtil.h | 26 ++ .../functions/sparksql/RegisterArithmetic.cpp | 2 + .../sparksql/tests/DecimalArithmeticTest.cpp | 69 +++++ 4 files changed, 380 insertions(+) diff --git a/velox/functions/sparksql/DecimalArithmetic.cpp b/velox/functions/sparksql/DecimalArithmetic.cpp index ffb782da069..b6ab0d1b68b 100644 --- a/velox/functions/sparksql/DecimalArithmetic.cpp +++ b/velox/functions/sparksql/DecimalArithmetic.cpp @@ -22,6 +22,17 @@ namespace facebook::velox::functions::sparksql { namespace { +inline static std::pair adjustPrecisionScale( + const uint8_t rPrecision, + const uint8_t rScale) { + if (rPrecision <= 38) { + return {rPrecision, rScale}; + } else { + int32_t minScale = std::min(static_cast(rScale), 6); + int32_t delta = rPrecision - 38; + return {38, std::max(rScale - delta, minScale)}; + } +} std::string getResultScale(std::string precision, std::string scale) { return fmt::format( @@ -33,6 +44,135 @@ std::string getResultScale(std::string precision, std::string scale) { scale); } +template +inline static void +getWholeAndFraction(const A& value, uint32_t scale, A& whole, A& fraction) { + whole = A(value / velox::DecimalUtil::kPowersOfTen[scale]); + fraction = A(value - whole * velox::DecimalUtil::kPowersOfTen[scale]); +} + +template +inline static int128_t checkAndIncreaseScale(const A& in, int16_t delta) { + return (delta <= 0) ? in : in * velox::DecimalUtil::kPowersOfTen[delta]; +} + +template +inline static A checkAndReduceScale(const A& in, int32_t delta) { + if (delta <= 0) { + return in; + } else { + A r; + bool overflow; + DecimalUtil::divideWithRoundUp( + r, in, A(velox::DecimalUtil::kPowersOfTen[delta]), 0, overflow); + VELOX_DCHECK(!overflow); + return r; + } +} + +// Both x_value and y_value must be >= 0. +template +inline static R addLargePositive( + const A& a, + const B& b, + uint8_t aScale, + uint8_t bScale, + uint8_t rScale) { + VELOX_DCHECK_GE(a, 0); + VELOX_DCHECK_GE(b, 0); + + // Separate out whole/fractions. + A aLeft, aRight; + B bLeft, bRight; + getWholeAndFraction(a, aScale, aLeft, aRight); + getWholeAndFraction(b, bScale, bLeft, bRight); + + // Adjust fractional parts to higher scale. + auto higherScale = std::max(aScale, bScale); + int128_t aRightScaled = + checkAndIncreaseScale(aRight, higherScale - aScale); + int128_t bRightScaled = + checkAndIncreaseScale(bRight, higherScale - bScale); + + R right; + int64_t carryToLeft; + auto multiplier = velox::DecimalUtil::kPowersOfTen[higherScale]; + if (aRightScaled >= multiplier - bRightScaled) { + right = R(aRightScaled - (multiplier - bRightScaled)); + carryToLeft = 1; + } else { + right = R(aRightScaled + bRightScaled); + carryToLeft = 0; + } + right = checkAndReduceScale(R(right), higherScale - rScale); + + auto left = R(aLeft) + R(bLeft) + R(carryToLeft); + return R(left * velox::DecimalUtil::kPowersOfTen[rScale]) + R(right); +} + +/// A and b cannot be 0, and one must be positive and the other +/// negative. +template +inline static R addLargeNegative( + const A& a, + const B& b, + uint8_t aScale, + uint8_t bScale, + int32_t rScale) { + VELOX_DCHECK_NE(a, 0); + VELOX_DCHECK_NE(b, 0); + VELOX_DCHECK((a < 0 && b > 0) || (a > 0 && b < 0)); + + // Separate out whole/fractions. + A aLeft, aRight; + B bLeft, bRight; + getWholeAndFraction(a, aScale, aLeft, aRight); + getWholeAndFraction(b, bScale, bLeft, bRight); + + // Adjust fractional parts to higher scale. + auto higherScale = std::max(aScale, bScale); + int128_t aRightScaled = + checkAndIncreaseScale(aRight, higherScale - aScale); + int128_t bRightScaled = + checkAndIncreaseScale(bRight, higherScale - bScale); + + // Overflow not possible because one is +ve and the other is -ve. + int128_t left = static_cast(aLeft) + static_cast(bLeft); + auto right = aRightScaled + bRightScaled; + + // If the whole and fractional parts have different signs, then we need to + // make the fractional part have the same sign as the whole part. If either + // left or right is zero, then nothing needs to be done. + if (left < 0 && right > 0) { + left += 1; + right -= velox::DecimalUtil::kPowersOfTen[higherScale]; + } else if (left > 0 && right < 0) { + left -= 1; + right += velox::DecimalUtil::kPowersOfTen[higherScale]; + } + right = checkAndReduceScale(R(right), higherScale - rScale); + return R((left * velox::DecimalUtil::kPowersOfTen[rScale]) + right); +} + +template +inline static R addLarge( + const A& a, + const B& b, + uint8_t aScale, + uint8_t bScale, + int32_t rScale) { + if (a >= 0 && b >= 0) { + // Both positive or 0. + return addLargePositive(a, b, aScale, bScale, rScale); + } else if (a <= 0 && b <= 0) { + // Both negative or 0. + return R(-addLargePositive(A(-a), B(-b), aScale, bScale, rScale)); + } else { + // One positive and the other negative. + return addLargeNegative(a, b, aScale, bScale, rScale); + } +} + template < typename R /* Result Type */, typename A /* Argument1 */, @@ -195,6 +335,117 @@ class DecimalBaseFunction : public exec::VectorFunction { const uint8_t rScale_; }; +class Addition { + public: + template + inline static void apply( + R& r, + const A& a, + const B& b, + uint8_t aRescale, + uint8_t bRescale, + uint8_t /* aPrecision */, + uint8_t aScale, + uint8_t /* bPrecision */, + uint8_t bScale, + uint8_t rPrecision, + uint8_t rScale, + bool& /*overflow*/) +#if defined(__has_feature) +#if __has_feature(__address_sanitizer__) + __attribute__((__no_sanitize__("signed-integer-overflow"))) +#endif +#endif + { + if (rPrecision < LongDecimalType::kMaxPrecision) { + int128_t aRescaled = a * velox::DecimalUtil::kPowersOfTen[aRescale]; + int128_t bRescaled = b * velox::DecimalUtil::kPowersOfTen[bRescale]; + r = R(aRescaled + bRescaled); + } else { + int32_t minLz = DecimalUtil::minLeadingZeros(a, b, aScale, bScale); + if (minLz >= 3) { + // If both numbers have at least MIN_LZ leading zeros, we can add them + // directly without the risk of overflow. We want the result to have at + // least 2 leading zeros, which ensures that it fits into the maximum + // decimal because 2^126 - 1 < 10^38 - 1. If both x and y have at least + // 3 leading zeros, then we are guaranteed that the result will have at + // lest 2 leading zeros. + int128_t aRescaled = a * velox::DecimalUtil::kPowersOfTen[aRescale]; + int128_t bRescaled = b * velox::DecimalUtil::kPowersOfTen[bRescale]; + auto higherScale = std::max(aScale, bScale); + int128_t sum = aRescaled + bRescaled; + r = checkAndReduceScale(R(sum), higherScale - rScale); + } else { + // Slower-version: add whole/fraction parts separately, and then + // combine. + r = addLarge(a, b, aScale, bScale, rScale); + } + } + } + + inline static uint8_t + computeRescaleFactor(uint8_t fromScale, uint8_t toScale, uint8_t rScale = 0) { + return std::max(0, toScale - fromScale); + } + + inline static std::pair computeResultPrecisionScale( + const uint8_t aPrecision, + const uint8_t aScale, + const uint8_t bPrecision, + const uint8_t bScale) { + auto precision = std::max(aPrecision - aScale, bPrecision - bScale) + + std::max(aScale, bScale) + 1; + auto scale = std::max(aScale, bScale); + return adjustPrecisionScale(precision, scale); + } +}; + +class Subtraction { + public: + template + inline static void apply( + R& r, + const A& a, + const B& b, + uint8_t aRescale, + uint8_t bRescale, + uint8_t aPrecision, + uint8_t aScale, + uint8_t bPrecision, + uint8_t bScale, + uint8_t rPrecision, + uint8_t rScale, + bool& overflow) { + Addition::apply( + r, + a, + B(-b), + aRescale, + bRescale, + aPrecision, + aScale, + bPrecision, + bScale, + rPrecision, + rScale, + overflow); + } + + inline static uint8_t + computeRescaleFactor(uint8_t fromScale, uint8_t toScale, uint8_t rScale = 0) { + return std::max(0, toScale - fromScale); + } + + inline static std::pair computeResultPrecisionScale( + const uint8_t aPrecision, + const uint8_t aScale, + const uint8_t bPrecision, + const uint8_t bScale) { + return Addition::computeResultPrecisionScale( + aPrecision, aScale, bPrecision, bScale); + } +}; + class Multiply { public: // Derive from Arrow. @@ -348,6 +599,28 @@ class Divide { } }; +std::vector> +decimalAddSubtractSignature() { + return { + exec::FunctionSignatureBuilder() + .integerVariable("a_precision") + .integerVariable("a_scale") + .integerVariable("b_precision") + .integerVariable("b_scale") + .integerVariable( + "r_precision", + "min(38, max(a_precision - a_scale, b_precision - b_scale) + max(a_scale, b_scale) + 1)") + .integerVariable( + "r_scale", + getResultScale( + "max(a_precision - a_scale, b_precision - b_scale) + max(a_scale, b_scale) + 1", + "max(a_scale, b_scale)")) + .returnType("DECIMAL(r_precision, r_scale)") + .argumentType("DECIMAL(a_precision, a_scale)") + .argumentType("DECIMAL(b_precision, b_scale)") + .build()}; +} + std::vector> decimalMultiplySignature() { return {exec::FunctionSignatureBuilder() @@ -482,6 +755,16 @@ std::shared_ptr createDecimalFunction( } }; // namespace +VELOX_DECLARE_STATEFUL_VECTOR_FUNCTION( + udf_decimal_add, + decimalAddSubtractSignature(), + createDecimalFunction); + +VELOX_DECLARE_STATEFUL_VECTOR_FUNCTION( + udf_decimal_sub, + decimalAddSubtractSignature(), + createDecimalFunction); + VELOX_DECLARE_STATEFUL_VECTOR_FUNCTION( udf_decimal_mul, decimalMultiplySignature(), diff --git a/velox/functions/sparksql/DecimalUtil.h b/velox/functions/sparksql/DecimalUtil.h index b6aa538f3e0..25163e3f3f2 100644 --- a/velox/functions/sparksql/DecimalUtil.h +++ b/velox/functions/sparksql/DecimalUtil.h @@ -109,6 +109,21 @@ class DecimalUtil { return value; } + template + inline static int32_t + minLeadingZeros(const A& a, const B& b, uint8_t aScale, uint8_t bScale) { + int32_t aLeadingZeros = bits::countLeadingZeros(absValue(a)); + int32_t bLeadingZeros = bits::countLeadingZeros(absValue(b)); + if (aScale < bScale) { + aLeadingZeros = + minLeadingZerosAfterScaling(aLeadingZeros, bScale - aScale); + } else if (aScale > bScale) { + bLeadingZeros = + minLeadingZerosAfterScaling(bLeadingZeros, aScale - bScale); + } + return std::min(aLeadingZeros, bLeadingZeros); + } + /// Derives from Arrow BasicDecimal128 Divide. /// https://github.com/apache/arrow/blob/release-12.0.1-rc1/cpp/src/gandiva/precompiled/decimal_ops.cc#L350 /// @@ -211,5 +226,16 @@ class DecimalUtil { int32_t numOccupied = sizeof(A) * 8 - bits::countLeadingZeros(valueAbs); return numOccupied + kMaxBitsRequiredIncreaseAfterScaling[aRescale]; } + + /// If we have a number with 'numLeadingZeros' leading zeros, and we scale it + /// up by 10^scale_by, this function returns the minimum number of leading + /// zeros the result can have. + inline static int32_t minLeadingZerosAfterScaling( + int32_t numLeadingZeros, + int32_t scaleBy) { + int32_t result = + numLeadingZeros - kMaxBitsRequiredIncreaseAfterScaling[scaleBy]; + return result; + } }; } // namespace facebook::velox::functions::sparksql diff --git a/velox/functions/sparksql/RegisterArithmetic.cpp b/velox/functions/sparksql/RegisterArithmetic.cpp index 5435cdf6601..08851f9e0d9 100644 --- a/velox/functions/sparksql/RegisterArithmetic.cpp +++ b/velox/functions/sparksql/RegisterArithmetic.cpp @@ -91,6 +91,8 @@ void registerArithmeticFunctions(const std::string& prefix) { registerFunction({prefix + "log10"}); registerRandFunctions(prefix); + VELOX_REGISTER_VECTOR_FUNCTION(udf_decimal_add, prefix + "add"); + VELOX_REGISTER_VECTOR_FUNCTION(udf_decimal_sub, prefix + "subtract"); VELOX_REGISTER_VECTOR_FUNCTION(udf_decimal_mul, prefix + "multiply"); VELOX_REGISTER_VECTOR_FUNCTION(udf_decimal_div, prefix + "divide"); } diff --git a/velox/functions/sparksql/tests/DecimalArithmeticTest.cpp b/velox/functions/sparksql/tests/DecimalArithmeticTest.cpp index a370809a946..0dc7a7ab067 100644 --- a/velox/functions/sparksql/tests/DecimalArithmeticTest.cpp +++ b/velox/functions/sparksql/tests/DecimalArithmeticTest.cpp @@ -59,6 +59,75 @@ class DecimalArithmeticTest : public SparkFunctionBaseTest { } }; // namespace +TEST_F(DecimalArithmeticTest, add) { + // The result can be obtained by Spark unit test + // test("add") { + // val l1 = Literal.create( + // Decimal(BigDecimal(1), 17, 3), + // DecimalType(17, 3)) + // val l2 = Literal.create( + // Decimal(BigDecimal(1), 17, 3), + // DecimalType(17, 3)) + // checkEvaluation(Add(l1, l2), null) + // } + + // Precision < 38. + testDecimalExpr( + makeFlatVector(std::vector{502}, DECIMAL(31, 3)), + "add(c0, c1)", + {makeFlatVector(std::vector{201}, DECIMAL(30, 3)), + makeFlatVector(std::vector{301}, DECIMAL(30, 3))}); + + // Min leading zero >= 3. + testDecimalExpr( + makeFlatVector(std::vector{2123210}, DECIMAL(38, 6)), + "add(c0, c1)", + {makeFlatVector(std::vector{11232100}, DECIMAL(38, 7)), + makeFlatVector(std::vector{1}, DECIMAL(10, 0))}); + + // Carry to left 0. + testDecimalExpr( + makeLongDecimalVector({"99999999999999999999999999999990000010"}, 38, 6), + "add(c0, c1)", + {makeLongDecimalVector({"9999999999999999999999999999999000000"}, 38, 5), + makeFlatVector(std::vector{100}, DECIMAL(38, 7))}); + + // Carry to left 1. + testDecimalExpr( + makeLongDecimalVector({"99999999999999999999999999999991500000"}, 38, 6), + "add(c0, c1)", + {makeLongDecimalVector({"9999999999999999999999999999999070000"}, 38, 5), + makeFlatVector(std::vector{8000000}, DECIMAL(38, 7))}); + + // Both -ve. + testDecimalExpr( + makeFlatVector(std::vector{-3211}, DECIMAL(32, 3)), + "add(c0, c1)", + {makeFlatVector(std::vector{-201}, DECIMAL(30, 3)), + makeFlatVector(std::vector{-301}, DECIMAL(30, 2))}); + + // -Ve and max precision. + testDecimalExpr( + makeLongDecimalVector({"-99999999999999999999999999999990000010"}, 38, 6), + "add(c0, c1)", + {makeLongDecimalVector( + {"-09999999999999999999999999999999000000"}, 38, 5), + makeFlatVector(std::vector{-100}, DECIMAL(38, 7))}); + // Ve and -ve. + testDecimalExpr( + makeLongDecimalVector({"99999999999999999999999999999989999990"}, 38, 6), + "add(c0, c1)", + {makeLongDecimalVector({"9999999999999999999999999999999000000"}, 38, 5), + makeFlatVector(std::vector{-100}, DECIMAL(38, 7))}); + // -Ve and ve. + testDecimalExpr( + makeLongDecimalVector({"99999999999999999999999999999989999990"}, 38, 6), + "add(c0, c1)", + {makeFlatVector(std::vector{-100}, DECIMAL(38, 7)), + makeLongDecimalVector( + {"9999999999999999999999999999999000000"}, 38, 5)}); +} + TEST_F(DecimalArithmeticTest, multiply) { // The result can be obtained by Spark unit test // test("multiply") { From f0a5faaea53c251dcb62788c028138931a84cd78 Mon Sep 17 00:00:00 2001 From: rui-mo Date: Tue, 19 Sep 2023 16:01:07 +0800 Subject: [PATCH 02/28] Add CAST(varchar as decimal) (5307) --- velox/docs/functions/presto/conversion.rst | 37 ++- velox/expression/CastExpr-inl.h | 265 ++++++++++++++++++++- velox/expression/CastExpr.cpp | 4 + velox/expression/CastExpr.h | 8 + velox/expression/tests/CastExprTest.cpp | 224 +++++++++++++++++ velox/type/DecimalUtil.h | 19 +- velox/type/Type.h | 1 + 7 files changed, 549 insertions(+), 9 deletions(-) diff --git a/velox/docs/functions/presto/conversion.rst b/velox/docs/functions/presto/conversion.rst index 7e439f78baf..3dd6bdaff34 100644 --- a/velox/docs/functions/presto/conversion.rst +++ b/velox/docs/functions/presto/conversion.rst @@ -149,7 +149,7 @@ supported conversions to/from JSON are listed in :doc:`json`. - Y - - Y - - + - Y * - timestamp - - @@ -803,3 +803,38 @@ Invalid example SELECT cast(decimal '-1000.000' as decimal(6, 4)); -- Out of range SELECT cast(decimal '123456789' as decimal(9, 1)); -- Out of range + +From varchar +^^^^^^^^^^^^ + +Casting varchar to a decimal of given precision and scale is allowed +if the input value can be represented by the precision and scale. When casting from +a larger scale to a smaller one, the fraction part is rounded. Casting from invalid input value throws. + +Valid example + +:: + + SELECT cast('9999999999.99' as decimal(12, 2)); -- decimal '9999999999.99' + SELECT cast('1.556' as decimal(12, 2)); -- decimal '1.56' + SELECT cast('1.554' as decimal(12, 2)); -- decimal '1.55' + SELECT cast('-1.554' as decimal(12, 2)); -- decimal '-1.55' + SELECT cast('+09' as decimal(12, 2)); -- decimal '9.00' + SELECT cast('9.' as decimal(12, 2)); -- decimal '9.00' + SELECT cast('.9' as decimal(12, 2)); -- decimal '0.90' + SELECT cast('3E+2' as decimal(12, 2)); -- decimal '300.00' + SELECT cast('3e+2' as decimal(12, 2)); -- decimal '300.00' + SELECT cast('31.423e+2' as decimal(12, 2)); -- decimal '3142.30' + SELECT cast('1.2e-2' as decimal(12, 2)); -- decimal '0.01' + SELECT cast('1.2e-5' as decimal(12, 2)); -- decimal '0.00' + SELECT cast('0000.123' as decimal(12, 2)); -- decimal '0.12' + SELECT cast('.123000000' as decimal(12, 2)); -- decimal '0.12' + +Invalid example + +:: + + SELECT cast('1.23e67' as decimal(38, 0)); -- Value too large + SELECT cast('0.0446a' as decimal(9, 1)); -- Value is not a number + SELECT cast('' as decimal(9, 1)); -- Value is not a number + SELECT cast('23e-5d' as decimal(9, 1)); -- Value is not a number diff --git a/velox/expression/CastExpr-inl.h b/velox/expression/CastExpr-inl.h index 2b3139d19f9..4a0de5ea3cf 100644 --- a/velox/expression/CastExpr-inl.h +++ b/velox/expression/CastExpr-inl.h @@ -51,6 +51,22 @@ inline std::exception_ptr makeBadCastException( false)); } +/// Represent the varchar fragment. +/// +/// For example: +/// | value | wholeDigits | fractionalDigits | exponent | sign +/// | 9999999999.99 | 9999999999 | 99 | nullopt | 1 +/// | 15 | 15 | | nullopt | 1 +/// | 1.5 | 1 | 5 | nullopt | 1 +/// | -1.5 | 1 | 5 | nullopt | -1 +/// | 31.523e-2 | 31 | 523 | -2 | 1 +struct DecimalComponents { + std::string_view wholeDigits; + std::string_view fractionalDigits; + std::optional exponent = std::nullopt; + int8_t sign = 1; +}; + // Copied from format.h of fmt. inline int countDigits(uint128_t n) { int count = 1; @@ -132,6 +148,215 @@ StringView convertToStringView( return StringView(startPosition, writePosition - startPosition); } +size_t parseDigitsRun( + const char* s, + size_t start, + size_t size, + std::string_view& out) { + size_t pos = start; + for (; pos < size; ++pos) { + if (!std::isdigit(s[pos])) { + break; + } + } + out = std::string_view(s + start, pos - start); + return pos; +} + +std::optional parseDecimalComponents( + const char* s, + size_t size) { + if (size == 0) { + return std::nullopt; + } + DecimalComponents out; + size_t pos = 0; + // Sign of the number. + if (s[pos] == '-') { + out.sign = -1; + ++pos; + } else if (s[pos] == '+') { + out.sign = 1; + ++pos; + } + // First run of digits. + pos = parseDigitsRun(s, pos, size, out.wholeDigits); + if (pos == size) { + return out.wholeDigits.empty() ? std::nullopt + : std::optional(out); + } + // Optional dot (if given in fractional form). + if (s[pos] == '.') { + // Second run of digits. + ++pos; + pos = parseDigitsRun(s, pos, size, out.fractionalDigits); + } + if (out.wholeDigits.empty() && out.fractionalDigits.empty()) { + // Need at least some digits (whole or fractional). + return std::nullopt; + } + if (pos == size) { + return out; + } + // Optional exponent. + if (s[pos] == 'e' || s[pos] == 'E') { + ++pos; + if (pos != size && s[pos] == '+') { + ++pos; + } + folly::StringPiece p = {s + pos, size - pos}; + auto tryExp = + folly::tryTo(folly::StringPiece(s + pos, size - pos)); + if (tryExp.hasError()) { + return std::nullopt; + } + out.exponent = tryExp.value(); + return out; + } + return pos == size ? std::optional(out) : std::nullopt; +} + +/// Multiple out by the appropriate power of 10 necessary to add source parsed +/// as int128_t and then adds the parsed value of source. +bool shiftAndAdd(std::string_view input, int128_t& out) { + auto length = input.size(); + if (length == 0) { + return true; + } + + bool overflow = + __builtin_mul_overflow(out, DecimalUtil::kPowersOfTen[length], &out); + if (overflow) { + return false; + } + auto tryValue = + folly::tryTo(folly::StringPiece(input.data(), length)); + if (tryValue.hasError()) { + return false; + } + + overflow = __builtin_add_overflow(out, tryValue.value(), &out); + VELOX_DCHECK(!overflow) + return true; +} + +/// Derives from Arrow function DecimalFromString. +/// Arrow implementation: +/// https://github.com/apache/arrow/blob/main/cpp/src/arrow/util/decimal.cc#L637 +/// +/// Firstly, it will parse the varchar to DecimalComponents which contains the +/// message that can represent a value. Secondly, process the exponent to get +/// the value parsedScale. Thirdly, compute the rescaled value. +/// The caller should test if `error` is empty +template +std::optional rescaleVarchar( + const StringView s, + int toPrecision, + int toScale, + std::string& error) { + auto decimalComponentsOpt = parseDecimalComponents(s.data(), s.size()); + if (!decimalComponentsOpt.has_value()) { + error = "Value is not a number."; + return std::nullopt; + } + auto decimalComponents = decimalComponentsOpt.value(); + + // Count number of significant digits (without leading zeros). + size_t firstNonZero = decimalComponents.wholeDigits.find_first_not_of('0'); + size_t significantDigits = decimalComponents.fractionalDigits.size(); + if (firstNonZero != std::string::npos) { + significantDigits += decimalComponents.wholeDigits.size() - firstNonZero; + } + int32_t parsedPrecision = static_cast(significantDigits); + + int32_t parsedScale = 0; + bool addOne = false; + int32_t fractionalDigitsSize = decimalComponents.fractionalDigits.size(); + if (decimalComponents.exponent.has_value()) { + auto adjustedExponent = decimalComponents.exponent.value(); + parsedScale = -adjustedExponent + fractionalDigitsSize; + // Truncate the fractionalDigits. + if (parsedScale > toScale) { + // adjustedExponent is negative, fractionalDigits only consider the last + // digit to round up. + if (-adjustedExponent >= toScale) { + if (fractionalDigitsSize > 0 && + decimalComponents.fractionalDigits[0] >= '5') { + addOne = true; + } + decimalComponents.fractionalDigits = ""; + parsedScale -= fractionalDigitsSize; + } else { + auto reduceDigits = adjustedExponent + toScale; + if (fractionalDigitsSize > reduceDigits && + decimalComponents.fractionalDigits[reduceDigits] >= '5') { + addOne = true; + } + decimalComponents.fractionalDigits = std::string_view( + decimalComponents.fractionalDigits.data(), + std::min(reduceDigits, fractionalDigitsSize)); + parsedScale -= + fractionalDigitsSize - decimalComponents.fractionalDigits.size(); + } + } + } else { + if (fractionalDigitsSize > toScale) { + if (decimalComponents.fractionalDigits[toScale] >= '5') { + addOne = true; + } + parsedScale = toScale; + decimalComponents.fractionalDigits = + std::string_view(decimalComponents.fractionalDigits.data(), toScale); + } else { + parsedScale = fractionalDigitsSize; + } + } + + int128_t out = 0; + if (!shiftAndAdd(decimalComponents.wholeDigits, out)) { + error = "Value too large."; + return std::nullopt; + } + + if (!shiftAndAdd(decimalComponents.fractionalDigits, out)) { + error = "Value too large."; + return std::nullopt; + } + if (addOne) { + bool overflow = __builtin_add_overflow(out, 1, &out); + if (UNLIKELY(overflow)) { + error = "Value too large."; + return std::nullopt; + } + } + out = out * decimalComponents.sign; + + if (parsedScale < 0) { + /// Force the scale to zero, to avoid negative scales (due to + /// compatibility issues with external systems such as databases). + if (-parsedScale + toScale > LongDecimalType::kMaxScale) { + error = "Value too large."; + return std::nullopt; + } + + bool overflow = __builtin_mul_overflow( + out, DecimalUtil::kPowersOfTen[-parsedScale + toScale], &out); + if (UNLIKELY(overflow)) { + error = "Value too large."; + return std::nullopt; + } + parsedPrecision -= parsedScale; + parsedScale = toScale; + } + bool overflow = false; + auto rescaledValue = DecimalUtil::rescaleWithRoundUp( + out, parsedPrecision, parsedScale, toPrecision, toScale, overflow, false); + if (overflow) { + error = "Value too large."; + return std::nullopt; + } + return rescaledValue; +} } // namespace template @@ -268,12 +493,14 @@ void CastExpr::applyDecimalCastKernel( applyToSelectedNoThrowLocal( context, rows, castResult, [&](vector_size_t row) { + bool overflow = false; auto rescaledValue = DecimalUtil::rescaleWithRoundUp( sourceVector->valueAt(row), fromPrecisionScale.first, fromPrecisionScale.second, toPrecisionScale.first, - toPrecisionScale.second); + toPrecisionScale.second, + overflow); if (rescaledValue.has_value()) { castResultRawBuffer[row] = rescaledValue.value(); } else { @@ -307,6 +534,42 @@ void CastExpr::applyIntToDecimalCastKernel( }); } +template +void CastExpr::applyVarcharToDecimalCastKernel( + const SelectivityVector& rows, + const BaseVector& input, + exec::EvalCtx& context, + const TypePtr& toType, + VectorPtr& result) { + auto sourceVector = input.as>(); + auto rawBuffer = result->asUnchecked>()->mutableRawValues(); + const auto toPrecisionScale = getDecimalPrecisionScale(*toType); + auto setError = [&](vector_size_t row, const std::string& details) { + if (setNullInResultAtError()) { + result->setNull(row, true); + } else { + context.setVeloxExceptionError( + row, makeBadCastException(toType, input, row, details)); + } + }; + + rows.applyToSelected([&](auto row) { + std::string error; + auto rescaledValue = rescaleVarchar( + sourceVector->valueAt(row), + toPrecisionScale.first, + toPrecisionScale.second, + error); + if (!error.empty()) { + setError(row, error); + } else if (rescaledValue.has_value()) { + rawBuffer[row] = rescaledValue.value(); + } else { + result->setNull(row, true); + } + }); +} + template VectorPtr CastExpr::applyDecimalToFloatCast( const SelectivityVector& rows, diff --git a/velox/expression/CastExpr.cpp b/velox/expression/CastExpr.cpp index 2ade0a28b30..a17cea6548c 100644 --- a/velox/expression/CastExpr.cpp +++ b/velox/expression/CastExpr.cpp @@ -485,6 +485,10 @@ VectorPtr CastExpr::applyDecimal( } [[fallthrough]]; } + case TypeKind::VARCHAR: + applyVarcharToDecimalCastKernel( + rows, input, context, toType, castResult); + break; default: VELOX_UNSUPPORTED( "Cast from {} to {} is not supported", diff --git a/velox/expression/CastExpr.h b/velox/expression/CastExpr.h index 6da2eca1d72..4aeb5a526bb 100644 --- a/velox/expression/CastExpr.h +++ b/velox/expression/CastExpr.h @@ -198,6 +198,14 @@ class CastExpr : public SpecialForm { const TypePtr& toType, VectorPtr& castResult); + template + void applyVarcharToDecimalCastKernel( + const SelectivityVector& rows, + const BaseVector& input, + exec::EvalCtx& context, + const TypePtr& toType, + VectorPtr& castResult); + template VectorPtr applyDecimalToFloatCast( const SelectivityVector& rows, diff --git a/velox/expression/tests/CastExprTest.cpp b/velox/expression/tests/CastExprTest.cpp index fcb4ce73cc9..1683e8bc710 100644 --- a/velox/expression/tests/CastExprTest.cpp +++ b/velox/expression/tests/CastExprTest.cpp @@ -2059,6 +2059,230 @@ TEST_F(CastExprTest, boolToDecimal) { DECIMAL(20, 10))); } +// The result is obtained by select cast('31.4e-2' as decimal(12, 2)). +TEST_F(CastExprTest, varcharToDecimal) { + auto input = makeFlatVector( + {"9999999999.99", + "15", + "1.5", + "-1.5", + "1.556", + "1.554", + ("1.556" + std::string(32, '1')).data(), + ("1.556" + std::string(32, '9')).data(), + "0000.123", + ".12300000000", + "+09", + "9.", + ".9", + "3E2", + "-3E+2", + "3E+2", + "3E-2", + "3e+2", + "3e-2", + "3.5E-2", + "3.4E-2", + "3.5E+2", + "3.4E+2", + "31.423e+2", + "31.423e-2", + "31.523e-2"}); + testComplexCast( + "c0", + input, + makeFlatVector( + {999'999'999'999, + 1500, + 150, + -150, + 156, + 155, + 156, + 156, + 12, + 12, + 900, + 900, + 90, + 30000, + -30000, + 30000, + 3, + 30000, + 3, + 4, + 3, + 35000, + 34000, + 314230, + 31, + 32}, + DECIMAL(12, 2))); + + // Truncate the fractional digits with exponent. + testComplexCast( + "c0", + makeFlatVector( + {"112345612.23e-6", + "112345662.23e-6", + "1.23e-6", + "1.23e-3", + "1.26e-3", + "1.23456781e3", + "1.23456789e3", + "1.23456789123451789123456789e9", + "1.23456789123456789123456789e9"}), + makeFlatVector( + {1123456, + 1123457, + 0, + 12, + 13, + 12345678, + 12345679, + 12345678912345, + 12345678912346}, + DECIMAL(20, 4))); + + auto minDecimalStr = '-' + std::string(36, '9') + '.' + "99"; + auto maxDecimalStr = std::string(36, '9') + '.' + "99"; + testComplexCast( + "c0", + makeFlatVector( + {StringView(minDecimalStr), + StringView(maxDecimalStr), + "123456789012345678901234.567"}), + makeFlatVector( + { + DecimalUtil::kLongDecimalMin, + DecimalUtil::kLongDecimalMax, + HugeInt::build( + 669260, 10962463713375599297U), // 12345678901234567890123457 + }, + DECIMAL(38, 2))); + + std::string fractionLarge = "1.9" + std::string(67, '9'); + std::string fractionLargeExp = "1.9" + std::string(67, '9') + "e2"; + std::string fractionLargeNegExp = "1000.9" + std::string(67, '9') + "e-2"; + testComplexCast( + "c0", + makeFlatVector( + {StringView(('-' + std::string(38, '9')).data()), + StringView(std::string(38, '9').data()), + StringView(fractionLarge.data()), + StringView(fractionLargeExp.data()), + StringView(fractionLargeNegExp.data())}), + makeFlatVector( + {DecimalUtil::kLongDecimalMin, + DecimalUtil::kLongDecimalMax, + 2, + 200, + 10}, + DECIMAL(38, 0))); + std::string fractionRoundDown = "0." + std::string(38, '9') + "2"; + std::string fractionRoundDownExp = "99." + std::string(36, '9') + "2e-2"; + testComplexCast( + "c0", + makeFlatVector( + {StringView(fractionRoundDown), StringView(fractionRoundDownExp)}), + makeConstant(DecimalUtil::kLongDecimalMax, 2, DECIMAL(38, 38))); + + // WholeDigits shiftAndAdd overflow. + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant(std::string(280, '9').data(), 1), + makeConstant(1, 1, DECIMAL(38, 0))), + fmt::format( + "Cannot cast VARCHAR '{}' to DECIMAL(38, 0). Value too large.", + std::string(280, '9'))) + // Function shiftAndAdd shift fractionalDigits overflow. + std::string shiftFractionOverflow = std::string(36, '9') + '.' + "23456"; + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant(shiftFractionOverflow.data(), 1), + makeConstant(2, 1, DECIMAL(38, 10))), + fmt::format( + "Cannot cast VARCHAR '{}' to DECIMAL(38, 10). Value too large.", + shiftFractionOverflow)) + std::string fractionRoundUp = "0." + std::string(38, '9') + "6"; + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant(fractionRoundUp.data(), 1), + makeConstant(3, 1, DECIMAL(38, 38))), + fmt::format( + "Cannot cast VARCHAR '{}' to DECIMAL(38, 38). Value too large.", + fractionRoundUp)) + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant("0.0444a", 1), + makeConstant(4, 1, DECIMAL(38, 0))), + "Cannot cast VARCHAR '0.0444a' to DECIMAL(38, 0). Value is not a number") + + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant("", 1), + makeConstant(5, 1, DECIMAL(38, 0))), + "Cannot cast VARCHAR '' to DECIMAL(38, 0). Value is not a number") + + // exponent parsedScale > LongDecimalType::kMaxScale. + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant("1.23e67", 1), + makeConstant(6, 1, DECIMAL(38, 0))), + "Cannot cast VARCHAR '1.23e67' to DECIMAL(38, 0). Value too large.") + + // Out * DecimalUtil::kPowersOfTen[-parsedScale] overflow. + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant("20908.23e35", 1), + makeConstant(7, 1, DECIMAL(38, 0))), + "Cannot cast VARCHAR '20908.23e35' to DECIMAL(38, 0). Value too large.") + + // Rescale overflow. + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant("111111111111111111.23", 1), + makeConstant(8, 1, DECIMAL(38, 38))), + "Cannot cast VARCHAR '111111111111111111.23' to DECIMAL(38, 38). Value too large.") + + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant("23e-5d", 1), + makeConstant(9, 1, DECIMAL(38, 0))), + "Cannot cast VARCHAR '23e-5d' to DECIMAL(38, 0). Value is not a number") + + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant("1. 23", 1), + makeConstant(10, 1, DECIMAL(38, 0))), + "Cannot cast VARCHAR '1. 23' to DECIMAL(38, 0). Value is not a number") + + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant("1.23 ", 1), + makeConstant(11, 1, DECIMAL(38, 0))), + "Cannot cast VARCHAR '1.23 ' to DECIMAL(38, 0). Value is not a number") + + VELOX_ASSERT_THROW( + testComplexCast( + "c0", + makeConstant(" 1.23 ", 1), + makeConstant(12, 1, DECIMAL(38, 0))), + "Cannot cast VARCHAR ' 1.23 ' to DECIMAL(38, 0). Value is not a number") +} + TEST_F(CastExprTest, castInTry) { // Test try(cast(array(varchar) as array(bigint))) whose input vector is // wrapped in dictinary encoding. The row of ["2a"] should trigger an error diff --git a/velox/type/DecimalUtil.h b/velox/type/DecimalUtil.h index d0a125b84e5..c34ccdf1d3f 100644 --- a/velox/type/DecimalUtil.h +++ b/velox/type/DecimalUtil.h @@ -151,10 +151,11 @@ class DecimalUtil { const int fromPrecision, const int fromScale, const int toPrecision, - const int toScale) { + const int toScale, + bool& isOverflow, + bool throwIfOverflow = true) { int128_t rescaledValue = inputValue; auto scaleDifference = toScale - fromScale; - bool isOverflow = false; if (scaleDifference >= 0) { isOverflow = __builtin_mul_overflow( rescaledValue, @@ -173,11 +174,15 @@ class DecimalUtil { } // Check overflow. if (!valueInPrecisionRange(rescaledValue, toPrecision) || isOverflow) { - VELOX_USER_FAIL( - "Cannot cast DECIMAL '{}' to DECIMAL({}, {})", - DecimalUtil::toString(inputValue, DECIMAL(fromPrecision, fromScale)), - toPrecision, - toScale); + if (throwIfOverflow) { + VELOX_USER_FAIL( + "Cannot cast DECIMAL '{}' to DECIMAL({}, {})", + DecimalUtil::toString(inputValue, DECIMAL(fromPrecision, fromScale)), + toPrecision, + toScale); + } else { + isOverflow = true; + } } return static_cast(rescaledValue); } diff --git a/velox/type/Type.h b/velox/type/Type.h index f0b60fd94c8..3c7113fc121 100644 --- a/velox/type/Type.h +++ b/velox/type/Type.h @@ -668,6 +668,7 @@ class DecimalType : public ScalarType { static_assert(KIND == TypeKind::BIGINT || KIND == TypeKind::HUGEINT); static constexpr uint8_t kMaxPrecision = KIND == TypeKind::BIGINT ? 18 : 38; static constexpr uint8_t kMinPrecision = KIND == TypeKind::BIGINT ? 0 : 19; + static constexpr uint8_t kMaxScale = kMaxPrecision; inline bool equivalent(const Type& other) const override { if (!Type::hasSameTypeId(other)) { From faccddfcb9c46fe3c2ce00301b899bd6489ec4de Mon Sep 17 00:00:00 2001 From: rui-mo Date: Thu, 7 Sep 2023 10:15:58 +0800 Subject: [PATCH 03/28] Add CAST(double as decimal) (5767) --- velox/expression/CastExpr-inl.h | 39 ++++++++++++++++++++++++++++++ velox/expression/CastExpr.cpp | 4 +++ velox/expression/CastExpr.h | 8 ++++++ velox/type/DecimalUtil.h | 43 +++++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+) diff --git a/velox/expression/CastExpr-inl.h b/velox/expression/CastExpr-inl.h index 4a0de5ea3cf..bddf930272a 100644 --- a/velox/expression/CastExpr-inl.h +++ b/velox/expression/CastExpr-inl.h @@ -570,6 +570,45 @@ void CastExpr::applyVarcharToDecimalCastKernel( }); } +template +void CastExpr::applyDoubleToDecimal( + const SelectivityVector& rows, + const BaseVector& input, + exec::EvalCtx& context, + const TypePtr& toType, + VectorPtr& castResult) { + auto sourceVector = input.as>(); + auto rawResults = + castResult->asUnchecked>()->mutableRawValues(); + const auto toPrecisionScale = getDecimalPrecisionScale(*toType); + applyToSelectedNoThrowLocal( + context, rows, castResult, [&](vector_size_t row) { + if (sourceVector->isNullAt(row)) { + castResult->setNull(row, true); + return; + } + std::string error; + auto rescaledValue = DecimalUtil::rescaleDouble( + sourceVector->valueAt(row), + toPrecisionScale.first, + toPrecisionScale.second, + error); + if (!error.empty()) { + if (setNullInResultAtError()) { + castResult->setNull(row, true); + } else { + context.setVeloxExceptionError( + row, makeBadCastException(toType, input, row, error)); + } + } else if (rescaledValue.has_value()) { + rawResults[row] = rescaledValue.value(); + } else { + castResult->setNull(row, true); + + } + }); +} + template VectorPtr CastExpr::applyDecimalToFloatCast( const SelectivityVector& rows, diff --git a/velox/expression/CastExpr.cpp b/velox/expression/CastExpr.cpp index a17cea6548c..c923f92c90f 100644 --- a/velox/expression/CastExpr.cpp +++ b/velox/expression/CastExpr.cpp @@ -467,6 +467,10 @@ VectorPtr CastExpr::applyDecimal( applyIntToDecimalCastKernel( rows, input, context, toType, castResult); break; + case TypeKind::DOUBLE: + applyDoubleToDecimal( + rows, input, context, toType, castResult); + break; case TypeKind::BIGINT: { if (fromType->isShortDecimal()) { applyDecimalCastKernel( diff --git a/velox/expression/CastExpr.h b/velox/expression/CastExpr.h index 4aeb5a526bb..928f73b1948 100644 --- a/velox/expression/CastExpr.h +++ b/velox/expression/CastExpr.h @@ -206,6 +206,14 @@ class CastExpr : public SpecialForm { const TypePtr& toType, VectorPtr& castResult); + template + void applyDoubleToDecimal( + const SelectivityVector& rows, + const BaseVector& input, + exec::EvalCtx& context, + const TypePtr& toType, + VectorPtr& castResult); + template VectorPtr applyDecimalToFloatCast( const SelectivityVector& rows, diff --git a/velox/type/DecimalUtil.h b/velox/type/DecimalUtil.h index c34ccdf1d3f..c190e6ad27e 100644 --- a/velox/type/DecimalUtil.h +++ b/velox/type/DecimalUtil.h @@ -16,6 +16,7 @@ #pragma once +#include #include #include "velox/common/base/CheckedArithmetic.h" #include "velox/common/base/Exceptions.h" @@ -207,6 +208,48 @@ class DecimalUtil { return static_cast(rescaledValue); } + /// Rescale a double value to decimal value. + /// + /// Use `folly::tryTo` to convert a double value to int128_t or int64_t. It + /// returns an error when overflow occurs so that we could determine whether + /// an overflow occurs through checking the result. + /// + /// Normally, return the rescaled value. Otherwise, if the `toValue` overflows + /// the TOutput's limits or the `toValue` exceeds the precision's limits, it + /// will throw an exception. + template + inline static std::optional rescaleDouble( + double inputValue, + const int toPrecision, + const int toScale, + std::string& error) { + if (!std::isfinite(inputValue)) { + error = "Value is not finite."; + return std::nullopt; + } + + auto toValue = + inputValue * static_cast(DecimalUtil::kPowersOfTen[toScale]); + + TOutput rescaledValue; + bool isOverflow = !std::isfinite(toValue); + if (!isOverflow) { + auto result = folly::tryTo(std::round(toValue)); + if (result.hasError()) { + isOverflow = true; + } else { + rescaledValue = result.value(); + } + } + + if (isOverflow || rescaledValue < -DecimalUtil::kPowersOfTen[toPrecision] || + rescaledValue > DecimalUtil::kPowersOfTen[toPrecision]) { + error = "Rescaled value is overflowed."; + return std::nullopt; + } + return rescaledValue; + } + template inline static R divideWithRoundUp( R& r, From 5a79a2e2c9715dda995136d6a545101ef94c7104 Mon Sep 17 00:00:00 2001 From: rui-mo Date: Wed, 13 Sep 2023 08:53:49 +0800 Subject: [PATCH 04/28] Spark sql avg agg function support decimal (6020) --- .../lib/aggregates/AverageAggregateBase.cpp | 6 +- .../lib/aggregates/DecimalAggregate.h | 11 +- .../sparksql/aggregates/AverageAggregate.cpp | 363 ++++++++++++++++-- .../tests/AverageAggregationTest.cpp | 127 ++++++ .../sparksql/specialforms/DecimalRound.cpp | 5 +- velox/type/DecimalUtil.h | 20 +- 6 files changed, 495 insertions(+), 37 deletions(-) diff --git a/velox/functions/lib/aggregates/AverageAggregateBase.cpp b/velox/functions/lib/aggregates/AverageAggregateBase.cpp index efef798b620..3353caed48b 100644 --- a/velox/functions/lib/aggregates/AverageAggregateBase.cpp +++ b/velox/functions/lib/aggregates/AverageAggregateBase.cpp @@ -21,14 +21,16 @@ namespace facebook::velox::functions::aggregate { void checkAvgIntermediateType(const TypePtr& type) { VELOX_USER_CHECK( type->isRow() || type->isVarbinary(), - "Input type for final average must be row type or varbinary type."); + "Input type for final average must be row type or varbinary type, find {}", + type->toString()); if (type->kind() == TypeKind::VARBINARY) { return; } VELOX_USER_CHECK( type->childAt(0)->kind() == TypeKind::DOUBLE || type->childAt(0)->isLongDecimal(), - "Input type for sum in final average must be double or long decimal type.") + "Input type for sum in final average must be double or long decimal type, find {}", + type->childAt(0)->toString()); VELOX_USER_CHECK_EQ( type->childAt(1)->kind(), TypeKind::BIGINT, diff --git a/velox/functions/lib/aggregates/DecimalAggregate.h b/velox/functions/lib/aggregates/DecimalAggregate.h index 695ec8f5d8a..9d867ab1bb8 100644 --- a/velox/functions/lib/aggregates/DecimalAggregate.h +++ b/velox/functions/lib/aggregates/DecimalAggregate.h @@ -74,11 +74,11 @@ class DecimalAggregate : public exec::Aggregate { explicit DecimalAggregate(TypePtr resultType) : exec::Aggregate(resultType) {} int32_t accumulatorFixedWidthSize() const override { - return sizeof(DecimalAggregate); + return sizeof(LongDecimalWithOverflowState); } int32_t accumulatorAlignmentSize() const override { - return static_cast(sizeof(int128_t)); + return alignof(LongDecimalWithOverflowState); } void initializeNewGroups( @@ -287,7 +287,9 @@ class DecimalAggregate : public exec::Aggregate { } virtual TResultType computeFinalValue( - LongDecimalWithOverflowState* accumulator) = 0; + LongDecimalWithOverflowState* accumulator) { + return 0; + }; void extractValues(char** groups, int32_t numGroups, VectorPtr* result) override { @@ -329,11 +331,12 @@ class DecimalAggregate : public exec::Aggregate { accumulator->count += 1; } - private: + protected: inline LongDecimalWithOverflowState* decimalAccumulator(char* group) { return exec::Aggregate::value(group); } + private: DecodedVector decodedRaw_; DecodedVector decodedPartial_; }; diff --git a/velox/functions/sparksql/aggregates/AverageAggregate.cpp b/velox/functions/sparksql/aggregates/AverageAggregate.cpp index c8637513a76..df401fc5aea 100644 --- a/velox/functions/sparksql/aggregates/AverageAggregate.cpp +++ b/velox/functions/sparksql/aggregates/AverageAggregate.cpp @@ -74,6 +74,290 @@ class AverageAggregate } }; +template +class DecimalAverageAggregate : public DecimalAggregate { + public: + explicit DecimalAverageAggregate(TypePtr resultType, TypePtr sumType) + : DecimalAggregate(resultType), sumType_(sumType) {} + + void addIntermediateResults( + char** groups, + const SelectivityVector& rows, + const std::vector& args, + bool /* mayPushdown */) override { + decodedPartial_.decode(*args[0], rows); + auto baseRowVector = dynamic_cast(decodedPartial_.base()); + auto sumVector = baseRowVector->childAt(0)->as>(); + auto countVector = baseRowVector->childAt(1)->as>(); + + if (decodedPartial_.isConstantMapping()) { + if (!decodedPartial_.isNullAt(0)) { + auto decodedIndex = decodedPartial_.index(0); + auto count = countVector->valueAt(decodedIndex); + if (sumVector->isNullAt(decodedIndex) && + !countVector->isNullAt(decodedIndex) && count > 0) { + // Find overflow, set all groups to null. + rows.applyToSelected( + [&](vector_size_t i) { this->setNull(groups[i]); }); + } else { + auto sum = sumVector->valueAt(decodedIndex); + rows.applyToSelected([&](vector_size_t i) { + this->clearNull(groups[i]); + auto accumulator = this->decimalAccumulator(groups[i]); + mergeSumCount(accumulator, sum, count); + }); + } + } + } else if (decodedPartial_.mayHaveNulls()) { + rows.applyToSelected([&](vector_size_t i) { + if (decodedPartial_.isNullAt(i)) { + return; + } + auto decodedIndex = decodedPartial_.index(i); + auto count = countVector->valueAt(decodedIndex); + if (sumVector->isNullAt(decodedIndex) && + !countVector->isNullAt(decodedIndex) && count > 0) { + this->setNull(groups[i]); + } else { + this->clearNull(groups[i]); + auto sum = sumVector->valueAt(decodedIndex); + auto accumulator = this->decimalAccumulator(groups[i]); + mergeSumCount(accumulator, sum, count); + } + }); + } else { + rows.applyToSelected([&](vector_size_t i) { + auto decodedIndex = decodedPartial_.index(i); + auto count = countVector->valueAt(decodedIndex); + if (sumVector->isNullAt(decodedIndex) && + !countVector->isNullAt(decodedIndex) && count > 0) { + this->setNull(groups[i]); + } else { + this->clearNull(groups[i]); + auto sum = sumVector->valueAt(decodedIndex); + auto accumulator = this->decimalAccumulator(groups[i]); + mergeSumCount(accumulator, sum, count); + } + }); + } + } + + void addSingleGroupIntermediateResults( + char* group, + const SelectivityVector& rows, + const std::vector& args, + bool /* mayPushdown */) override { + decodedPartial_.decode(*args[0], rows); + auto baseRowVector = dynamic_cast(decodedPartial_.base()); + auto sumVector = baseRowVector->childAt(0)->as>(); + auto countVector = baseRowVector->childAt(1)->as>(); + + if (decodedPartial_.isConstantMapping()) { + if (!decodedPartial_.isNullAt(0)) { + auto decodedIndex = decodedPartial_.index(0); + if (isPartialSumOverflow(sumVector, countVector, decodedIndex)) { + // Find overflow, just set group to null and return. + this->setNull(group); + return; + } else { + if (rows.hasSelections()) { + this->clearNull(group); + } + auto sum = sumVector->valueAt(decodedIndex); + auto count = countVector->valueAt(decodedIndex); + rows.applyToSelected( + [&](vector_size_t i) { mergeAccumulators(group, sum, count); }); + } + } + } else if (decodedPartial_.mayHaveNulls()) { + rows.applyToSelected([&](vector_size_t i) { + if (!decodedPartial_.isNullAt(i)) { + this->clearNull(group); + auto decodedIndex = decodedPartial_.index(i); + if (isPartialSumOverflow(sumVector, countVector, decodedIndex)) { + // Find overflow, just set group to null. + this->setNull(group); + } else { + auto sum = sumVector->valueAt(decodedIndex); + auto count = countVector->valueAt(decodedIndex); + mergeAccumulators(group, sum, count); + } + } + }); + } else { + if (rows.hasSelections()) { + this->clearNull(group); + } + rows.applyToSelected([&](vector_size_t i) { + auto decodedIndex = decodedPartial_.index(i); + if (isPartialSumOverflow(sumVector, countVector, decodedIndex)) { + // Find overflow, just set group to null. + this->setNull(group); + } else { + auto sum = sumVector->valueAt(decodedIndex); + auto count = countVector->valueAt(decodedIndex); + mergeAccumulators(group, sum, count); + } + }); + } + } + + void extractAccumulators(char** groups, int32_t numGroups, VectorPtr* result) + override { + auto rowVector = (*result)->as(); + auto sumVector = rowVector->childAt(0)->asFlatVector(); + auto countVector = rowVector->childAt(1)->asFlatVector(); + rowVector->resize(numGroups); + sumVector->resize(numGroups); + countVector->resize(numGroups); + rowVector->clearAllNulls(); + + int64_t* rawCounts = countVector->mutableRawValues(); + int128_t* rawSums = sumVector->mutableRawValues(); + for (auto i = 0; i < numGroups; ++i) { + char* group = groups[i]; + auto* accumulator = this->decimalAccumulator(group); + std::optional validSum = + DecimalUtil::computeValidSum(accumulator->sum, accumulator->overflow); + if (validSum.has_value()) { + rawCounts[i] = accumulator->count; + rawSums[i] = validSum.value(); + } else { + // Find overflow. + sumVector->setNull(i, true); + rawCounts[i] = accumulator->count; + } + } + } + + void extractValues(char** groups, int32_t numGroups, VectorPtr* result) + override { + auto vector = (*result)->as>(); + VELOX_CHECK(vector); + vector->resize(numGroups); + uint64_t* rawNulls = this->getRawNulls(vector); + + TResultType* rawValues = vector->mutableRawValues(); + for (int32_t i = 0; i < numGroups; ++i) { + char* group = groups[i]; + auto accumulator = this->decimalAccumulator(group); + if (accumulator->count == 0) { + // In Spark, if all inputs are null, count will be 0, + // and the result of final avg will be null. + vector->setNull(i, true); + } else { + this->clearNull(rawNulls, i); + std::optional avg = computeAvg(accumulator); + if (avg.has_value()) { + rawValues[i] = avg.value(); + } else { + // Find overflow. + vector->setNull(i, true); + } + } + } + } + + std::optional computeAvg( + LongDecimalWithOverflowState* accumulator) { + std::optional validSum = + DecimalUtil::computeValidSum(accumulator->sum, accumulator->overflow); + if (!validSum.has_value()) { + return std::nullopt; + } + + auto [resultPrecision, resultScale] = + getDecimalPrecisionScale(*this->resultType().get()); + // Spark use DECIMAL(20,0) to represent long value. + const uint8_t countPrecision = 20, countScale = 0; + auto [sumPrecision, sumScale] = + getDecimalPrecisionScale(*this->sumType_.get()); + auto [avgPrecision, avgScale] = computeResultPrecisionScale( + sumPrecision, sumScale, countPrecision, countScale); + auto sumRescale = computeRescaleFactor(sumScale, countScale, avgScale); + auto countDecimal = accumulator->count; + int128_t avg = 0; + + DecimalUtil::divideWithRoundUp( + avg, validSum.value(), countDecimal, false, sumRescale, 0); + bool isOverflow = false; + return DecimalUtil::rescaleWithRoundUp( + avg, + avgPrecision, + avgScale, + resultPrecision, + resultScale, + isOverflow, + false); + } + + private: + template + inline void mergeSumCount( + LongDecimalWithOverflowState* accumulator, + UnscaledType sum, + int64_t count) { + accumulator->count += count; + accumulator->overflow += + DecimalUtil::addWithOverflow(accumulator->sum, sum, accumulator->sum); + } + + template + void mergeAccumulators( + char* group, + const UnscaledType& otherSum, + const int64_t& otherCount) { + if constexpr (tableHasNulls) { + exec::Aggregate::clearNull(group); + } + auto accumulator = this->decimalAccumulator(group); + mergeSumCount(accumulator, otherSum, otherCount); + } + + inline static bool isPartialSumOverflow( + SimpleVector* sumVector, + SimpleVector* countVector, + int32_t index) { + return sumVector->isNullAt(index) && !countVector->isNullAt(index) && + countVector->valueAt(index) > 0; + } + + inline static uint8_t + computeRescaleFactor(uint8_t fromScale, uint8_t toScale, uint8_t rScale) { + return rScale - fromScale + toScale; + } + + inline static std::pair computeResultPrecisionScale( + const uint8_t aPrecision, + const uint8_t aScale, + const uint8_t bPrecision, + const uint8_t bScale) { + uint8_t intDig = aPrecision - aScale + bScale; + uint8_t scale = std::max(6, aScale + bPrecision + 1); + uint8_t precision = intDig + scale; + return adjustPrecisionScale(precision, scale); + } + + inline static std::pair adjustPrecisionScale( + const uint8_t precision, + const uint8_t scale) { + VELOX_CHECK(precision >= scale); + if (precision <= 38) { + return {precision, scale}; + } else { + uint8_t intDigits = precision - scale; + uint8_t minScaleValue = std::min(scale, (uint8_t)6); + uint8_t adjustedScale = + std::max((uint8_t)(38 - intDigits), minScaleValue); + return {38, adjustedScale}; + } + } + + DecodedVector decodedRaw_; + DecodedVector decodedPartial_; + TypePtr sumType_; +}; + } // namespace /// Count is BIGINT() while sum and the final aggregates type depends on @@ -98,13 +382,16 @@ exec::AggregateRegistrationResult registerAverage( .build()); } - signatures.push_back(exec::AggregateFunctionSignatureBuilder() - .integerVariable("a_precision") - .integerVariable("a_scale") - .argumentType("DECIMAL(a_precision, a_scale)") - .intermediateType("varbinary") - .returnType("DECIMAL(a_precision, a_scale)") - .build()); + signatures.push_back( + exec::AggregateFunctionSignatureBuilder() + .integerVariable("a_precision") + .integerVariable("a_scale") + .integerVariable("r_precision", "min(38, a_precision + 4)") + .integerVariable("r_scale", "min(38, a_scale + 4)") + .argumentType("DECIMAL(a_precision, a_scale)") + .intermediateType("ROW(DECIMAL(38 , a_scale), BIGINT)") + .returnType("DECIMAL(r_precision, r_scale)") + .build()); return exec::registerAggregateFunction( name, @@ -117,7 +404,7 @@ exec::AggregateRegistrationResult registerAverage( -> std::unique_ptr { VELOX_CHECK_LE( argTypes.size(), 1, "{} takes at most one argument", name); - auto inputType = argTypes[0]; + const auto& inputType = argTypes[0]; if (exec::isRawInput(step)) { switch (inputType->kind()) { case TypeKind::SMALLINT: @@ -128,16 +415,40 @@ exec::AggregateRegistrationResult registerAverage( AverageAggregate>(resultType); case TypeKind::BIGINT: { if (inputType->isShortDecimal()) { - return std::make_unique>( - resultType); + auto inputPrecision = inputType->asShortDecimal().precision(); + auto inputScale = inputType->asShortDecimal().scale(); + auto sumType = + DECIMAL(std::min(38, inputPrecision + 10), inputScale); + if (exec::isPartialOutput(step)) { + return std::make_unique< + DecimalAverageAggregate>( + resultType, sumType); + } else { + if (resultType->isShortDecimal()) { + return std::make_unique< + DecimalAverageAggregate>( + resultType, sumType); + } else if (resultType->isLongDecimal()) { + return std::make_unique< + DecimalAverageAggregate>( + resultType, sumType); + } else { + VELOX_FAIL("Result type must be decimal"); + } + } } return std::make_unique< AverageAggregate>(resultType); } case TypeKind::HUGEINT: { if (inputType->isLongDecimal()) { - return std::make_unique>( - resultType); + auto inputPrecision = inputType->asLongDecimal().precision(); + auto inputScale = inputType->asLongDecimal().scale(); + auto sumType = + DECIMAL(std::min(38, inputPrecision + 10), inputScale); + return std::make_unique< + DecimalAverageAggregate>( + resultType, sumType); } VELOX_NYI(); } @@ -161,27 +472,21 @@ exec::AggregateRegistrationResult registerAverage( resultType); case TypeKind::DOUBLE: case TypeKind::ROW: + if (inputType->childAt(0)->isLongDecimal()) { + return std::make_unique< + DecimalAverageAggregate>( + resultType, inputType->childAt(0)); + } return std::make_unique< AverageAggregate>(resultType); case TypeKind::BIGINT: - return std::make_unique>( - resultType); + return std::make_unique< + DecimalAverageAggregate>( + resultType, inputType->childAt(0)); case TypeKind::HUGEINT: - return std::make_unique>( - resultType); - case TypeKind::VARBINARY: - if (inputType->isLongDecimal()) { - return std::make_unique>( - resultType); - } else if ( - inputType->isShortDecimal() || - inputType->kind() == TypeKind::VARBINARY) { - // If the input and out type are VARBINARY, then the - // LongDecimalWithOverflowState is used and the template type - // does not matter. - return std::make_unique>( - resultType); - } + return std::make_unique< + DecimalAverageAggregate>( + resultType, inputType->childAt(0)); default: VELOX_FAIL( "Unsupported result type for final aggregation: {}", diff --git a/velox/functions/sparksql/aggregates/tests/AverageAggregationTest.cpp b/velox/functions/sparksql/aggregates/tests/AverageAggregationTest.cpp index 3dfbb97e5bd..f20b4bfde3a 100644 --- a/velox/functions/sparksql/aggregates/tests/AverageAggregationTest.cpp +++ b/velox/functions/sparksql/aggregates/tests/AverageAggregationTest.cpp @@ -111,5 +111,132 @@ TEST_F(AverageAggregationTest, avgAllNulls) { assertQuery(plan, expected); } +TEST_F(AverageAggregationTest, avgDecimal) { + int64_t kRescale = DecimalUtil::kPowersOfTen[4]; + // Short decimal aggregation + auto shortDecimal = makeNullableFlatVector( + {1'000, 2'000, 3'000, 4'000, 5'000, std::nullopt}, DECIMAL(10, 1)); + testAggregations( + {makeRowVector({shortDecimal})}, + {}, + {"spark_avg(c0)"}, + {}, + {makeRowVector({makeNullableFlatVector( + {3'000 * kRescale}, DECIMAL(14, 5))})}); + + // Long decimal aggregation + testAggregations( + {makeRowVector({makeNullableFlatVector( + {HugeInt::build(10, 100), + HugeInt::build(10, 200), + HugeInt::build(10, 300), + HugeInt::build(10, 400), + HugeInt::build(10, 500), + std::nullopt}, + DECIMAL(23, 4))})}, + {}, + {"spark_avg(c0)"}, + {}, + {makeRowVector({makeFlatVector( + std::vector{HugeInt::build(10, 300) * kRescale}, + DECIMAL(27, 8))})}); + + // The total sum overflows the max int128_t limit. + std::vector rawVector; + for (int i = 0; i < 10; ++i) { + rawVector.push_back(DecimalUtil::kLongDecimalMax); + } + testAggregations( + {makeRowVector({makeFlatVector(rawVector, DECIMAL(38, 0))})}, + {}, + {"spark_avg(c0)"}, + {}, + {makeRowVector({makeNullableFlatVector( + std::vector>{std::nullopt}, + DECIMAL(38, 4))})}); + + // The total sum underflows the min int128_t limit. + rawVector.clear(); + auto underFlowTestResult = makeNullableFlatVector( + std::vector>{std::nullopt}, DECIMAL(38, 4)); + for (int i = 0; i < 10; ++i) { + rawVector.push_back(DecimalUtil::kLongDecimalMin); + } + testAggregations( + {makeRowVector({makeFlatVector(rawVector, DECIMAL(38, 0))})}, + {}, + {"spark_avg(c0)"}, + {}, + {makeRowVector({underFlowTestResult})}); + + // Test constant vector. + testAggregations( + {makeRowVector({makeConstant(100, 10, DECIMAL(10, 2))})}, + {}, + {"spark_avg(c0)"}, + {}, + {makeRowVector({makeFlatVector( + std::vector{100 * kRescale}, DECIMAL(14, 6))})}); + + auto newSize = shortDecimal->size() * 2; + auto indices = makeIndices(newSize, [&](int row) { return row / 2; }); + auto dictVector = + VectorTestBase::wrapInDictionary(indices, newSize, shortDecimal); + + testAggregations( + {makeRowVector({dictVector})}, + {}, + {"spark_avg(c0)"}, + {}, + {makeRowVector({makeFlatVector( + std::vector{3'000 * kRescale}, DECIMAL(14, 5))})}); + + // Decimal average aggregation with multiple groups. + auto inputRows = { + makeRowVector( + {makeNullableFlatVector({1, 1}), + makeFlatVector({37220, 53450}, DECIMAL(15, 2))}), + makeRowVector( + {makeNullableFlatVector({2, 2}), + makeFlatVector({10410, 9250}, DECIMAL(15, 2))}), + makeRowVector( + {makeNullableFlatVector({3, 3}), + makeFlatVector({-12783, 0}, DECIMAL(15, 2))}), + makeRowVector( + {makeNullableFlatVector({1, 2}), + makeFlatVector({23178, 41093}, DECIMAL(15, 2))}), + makeRowVector( + {makeNullableFlatVector({2, 3}), + makeFlatVector({-10023, 5290}, DECIMAL(15, 2))}), + }; + + auto expectedResult = { + makeRowVector( + {makeNullableFlatVector({1}), + makeFlatVector(std::vector{379493333}, DECIMAL(19, 6))}), + makeRowVector( + {makeNullableFlatVector({2}), + makeFlatVector(std::vector{126825000}, DECIMAL(19, 6))}), + makeRowVector( + {makeNullableFlatVector({3}), + makeFlatVector(std::vector{-24976667}, DECIMAL(19, 6))})}; + + testAggregations(inputRows, {"c0"}, {"spark_avg(c1)"}, expectedResult); +} + +TEST_F(AverageAggregationTest, avgDecimalWithMultipleRowVectors) { + int64_t kRescale = DecimalUtil::kPowersOfTen[4]; + auto inputRows = { + makeRowVector({makeFlatVector({100, 200}, DECIMAL(15, 2))}), + makeRowVector({makeFlatVector({300, 400}, DECIMAL(15, 2))}), + makeRowVector({makeFlatVector({500, 600}, DECIMAL(15, 2))}), + }; + + auto expectedResult = {makeRowVector( + {makeFlatVector(std::vector{350 * kRescale}, DECIMAL(19, 6))})}; + + testAggregations(inputRows, {}, {"spark_avg(c0)"}, expectedResult); +} + } // namespace } // namespace facebook::velox::functions::aggregate::sparksql::test diff --git a/velox/functions/sparksql/specialforms/DecimalRound.cpp b/velox/functions/sparksql/specialforms/DecimalRound.cpp index b96132688f9..d8091145263 100644 --- a/velox/functions/sparksql/specialforms/DecimalRound.cpp +++ b/velox/functions/sparksql/specialforms/DecimalRound.cpp @@ -91,13 +91,16 @@ class DecimalRoundFunction : public exec::VectorFunction { private: inline TResult applyRound(const TInput& input) const { if (scale_ >= 0) { + bool overflow = false; const auto rescaledValue = DecimalUtil::rescaleWithRoundUp( input, inputPrecision_, inputScale_, resultPrecision_, - resultScale_); + resultScale_, + overflow, + false); VELOX_DCHECK(rescaledValue.has_value()); return rescaledValue.value(); } else { diff --git a/velox/type/DecimalUtil.h b/velox/type/DecimalUtil.h index c190e6ad27e..bd749d9967f 100644 --- a/velox/type/DecimalUtil.h +++ b/velox/type/DecimalUtil.h @@ -178,11 +178,13 @@ class DecimalUtil { if (throwIfOverflow) { VELOX_USER_FAIL( "Cannot cast DECIMAL '{}' to DECIMAL({}, {})", - DecimalUtil::toString(inputValue, DECIMAL(fromPrecision, fromScale)), + DecimalUtil::toString( + inputValue, DECIMAL(fromPrecision, fromScale)), toPrecision, toScale); } else { isOverflow = true; + return std::nullopt; } } return static_cast(rescaledValue); @@ -366,6 +368,22 @@ class DecimalUtil { int64_t count, int64_t overflow); + inline static std::optional computeValidSum( + int128_t sum, + int64_t overflow) { + // Value is valid if the conditions below are true. + int128_t validSum = sum; + if ((overflow == 1 && sum < 0) || (overflow == -1 && sum > 0)) { + validSum = static_cast( + DecimalUtil::kOverflowMultiplier * overflow + sum); + } else { + if (overflow != 0) { + return std::nullopt; + } + } + return validSum; + } + /// Origins from java side BigInteger#bitLength. /// /// Returns the number of bits in the minimal two's-complement From 27453e1035f7d74f9e6779fb7784c2880aa48ef9 Mon Sep 17 00:00:00 2001 From: rui-mo Date: Mon, 14 Aug 2023 00:48:17 +0000 Subject: [PATCH 05/28] Spark sql sum agg function support decimal (5372) --- .../sparksql/aggregates/DecimalSumAggregate.h | 450 ++++++++++++++++++ .../sparksql/aggregates/Register.cpp | 2 + .../sparksql/aggregates/tests/CMakeLists.txt | 1 + .../tests/DecimalSumAggregateTest.cpp | 332 +++++++++++++ 4 files changed, 785 insertions(+) create mode 100644 velox/functions/sparksql/aggregates/DecimalSumAggregate.h create mode 100644 velox/functions/sparksql/aggregates/tests/DecimalSumAggregateTest.cpp diff --git a/velox/functions/sparksql/aggregates/DecimalSumAggregate.h b/velox/functions/sparksql/aggregates/DecimalSumAggregate.h new file mode 100644 index 00000000000..3ea6e06caf1 --- /dev/null +++ b/velox/functions/sparksql/aggregates/DecimalSumAggregate.h @@ -0,0 +1,450 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once +#include "velox/exec/Aggregate.h" +#include "velox/expression/FunctionSignature.h" +#include "velox/vector/FlatVector.h" + +namespace facebook::velox::functions::aggregate::sparksql { + +struct DecimalSum { + int128_t sum{0}; + int64_t overflow{0}; + bool isEmpty{true}; + + void mergeWith(const DecimalSum& other) { + this->overflow += other.overflow; + this->overflow += + DecimalUtil::addWithOverflow(this->sum, other.sum, this->sum); + this->isEmpty &= other.isEmpty; + } +}; + +template +class DecimalSumAggregate : public exec::Aggregate { + public: + explicit DecimalSumAggregate(TypePtr resultType, TypePtr sumType) + : exec::Aggregate(resultType), sumType_(sumType) {} + + int32_t accumulatorFixedWidthSize() const override { + return sizeof(DecimalSum); + } + + int32_t accumulatorAlignmentSize() const override { + return alignof(DecimalSum); + } + + void initializeNewGroups( + char** groups, + folly::Range indices) override { + setAllNulls(groups, indices); + for (auto i : indices) { + new (groups[i] + offset_) DecimalSum(); + } + } + + int128_t computeFinalValue(DecimalSum* decimalSum, bool& overflow) { + int128_t sum = decimalSum->sum; + if ((decimalSum->overflow == 1 && decimalSum->sum < 0) || + (decimalSum->overflow == -1 && decimalSum->sum > 0)) { + sum = static_cast( + DecimalUtil::kOverflowMultiplier * decimalSum->overflow + + decimalSum->sum); + } else { + if (decimalSum->overflow != 0) { + overflow = true; + return 0; + } + } + + auto [resultPrecision, resultScale] = + getDecimalPrecisionScale(*sumType_.get()); + overflow = !DecimalUtil::valueInPrecisionRange(sum, resultPrecision); + return sum; + } + + void extractValues(char** groups, int32_t numGroups, VectorPtr* result) + override { + VELOX_CHECK_EQ((*result)->encoding(), VectorEncoding::Simple::FLAT); + auto vector = (*result)->as>(); + VELOX_CHECK(vector); + vector->resize(numGroups); + uint64_t* rawNulls = getRawNulls(vector); + + TResultType* rawValues = vector->mutableRawValues(); + for (auto i = 0; i < numGroups; ++i) { + char* group = groups[i]; + if (isNull(group)) { + vector->setNull(i, true); + } else { + clearNull(rawNulls, i); + auto* decimalSum = accumulator(group); + if (decimalSum->isEmpty) { + // If isEmpty is true, we should set null. + vector->setNull(i, true); + } else { + bool overflow = false; + auto result = (TResultType)computeFinalValue(decimalSum, overflow); + if (overflow) { + // Sum should be set to null on overflow. + vector->setNull(i, true); + } else { + rawValues[i] = result; + } + } + } + } + } + + void extractAccumulators( + char** groups, + int32_t numGroups, + facebook::velox::VectorPtr* result) override { + VELOX_CHECK_EQ((*result)->encoding(), VectorEncoding::Simple::ROW); + auto rowVector = (*result)->as(); + auto sumVector = rowVector->childAt(0)->asFlatVector(); + auto isEmptyVector = rowVector->childAt(1)->asFlatVector(); + + rowVector->resize(numGroups); + sumVector->resize(numGroups); + isEmptyVector->resize(numGroups); + + TResultType* rawSums = sumVector->mutableRawValues(); + // Bool uses compact representation, use mutableRawValues + // and bits::setBit instead. + auto* rawIsEmpty = isEmptyVector->mutableRawValues(); + uint64_t* rawNulls = getRawNulls(rowVector); + + for (auto i = 0; i < numGroups; ++i) { + char* group = groups[i]; + clearNull(rawNulls, i); + if (isNull(group)) { + bits::setBit(rawIsEmpty, i, true); + rawSums[i] = 0; + } else { + auto* decimalSum = accumulator(group); + bool overflow = false; + auto result = (TResultType)computeFinalValue(decimalSum, overflow); + if (overflow) { + // Sum should be set to null on overflow, and + // isEmpty should be set to false. + sumVector->setNull(i, true); + bits::setBit(rawIsEmpty, i, false); + } else { + rawSums[i] = result; + bits::setBit(rawIsEmpty, i, decimalSum->isEmpty); + } + } + } + } + + void addRawInput( + char** groups, + const facebook::velox::SelectivityVector& rows, + const std::vector& args, + bool /* mayPushdown */) override { + decodedRaw_.decode(*args[0], rows); + if (decodedRaw_.isConstantMapping()) { + if (!decodedRaw_.isNullAt(0)) { + auto value = decodedRaw_.valueAt(0); + rows.applyToSelected([&](vector_size_t i) { + updateNonNullValue(groups[i], value, false); + }); + } + } else if (decodedRaw_.mayHaveNulls()) { + rows.applyToSelected([&](vector_size_t i) { + if (decodedRaw_.isNullAt(i)) { + return; + } + updateNonNullValue( + groups[i], decodedRaw_.valueAt(i), false); + }); + } else if (!exec::Aggregate::numNulls_ && decodedRaw_.isIdentityMapping()) { + auto data = decodedRaw_.data(); + rows.applyToSelected([&](vector_size_t i) { + updateNonNullValue(groups[i], data[i], false); + }); + } else { + rows.applyToSelected([&](vector_size_t i) { + updateNonNullValue( + groups[i], decodedRaw_.valueAt(i), false); + }); + } + } + + void addSingleGroupRawInput( + char* group, + const SelectivityVector& rows, + const std::vector& args, + bool /* mayPushdown */) override { + decodedRaw_.decode(*args[0], rows); + if (decodedRaw_.isConstantMapping()) { + if (!decodedRaw_.isNullAt(0)) { + auto value = decodedRaw_.valueAt(0); + rows.template applyToSelected( + [&](vector_size_t i) { updateNonNullValue(group, value, false); }); + } + } else if (decodedRaw_.mayHaveNulls()) { + rows.applyToSelected([&](vector_size_t i) { + if (!decodedRaw_.isNullAt(i)) { + updateNonNullValue(group, decodedRaw_.valueAt(i), false); + } + }); + } else if (!exec::Aggregate::numNulls_ && decodedRaw_.isIdentityMapping()) { + auto data = decodedRaw_.data(); + DecimalSum decimalSum; + rows.applyToSelected([&](vector_size_t i) { + decimalSum.overflow += DecimalUtil::addWithOverflow( + decimalSum.sum, data[i], decimalSum.sum); + decimalSum.isEmpty = false; + }); + mergeAccumulators(group, decimalSum); + } else { + DecimalSum decimalSum; + rows.applyToSelected([&](vector_size_t i) { + decimalSum.overflow += DecimalUtil::addWithOverflow( + decimalSum.sum, decodedRaw_.valueAt(i), decimalSum.sum); + decimalSum.isEmpty = false; + }); + mergeAccumulators(group, decimalSum); + } + } + + void addIntermediateResults( + char** groups, + const SelectivityVector& rows, + const std::vector& args, + bool /* mayPushdown */) override { + decodedPartial_.decode(*args[0], rows); + VELOX_CHECK_EQ( + decodedPartial_.base()->encoding(), VectorEncoding::Simple::ROW); + auto baseRowVector = dynamic_cast(decodedPartial_.base()); + auto sumVector = baseRowVector->childAt(0)->as>(); + auto isEmptyVector = baseRowVector->childAt(1)->as>(); + + if (decodedPartial_.isConstantMapping()) { + if (!decodedPartial_.isNullAt(0)) { + auto decodedIndex = decodedPartial_.index(0); + if (!isEmptyVector->valueAt(decodedIndex) && + sumVector->isNullAt(decodedIndex)) { + // If isEmpty is false and sum is null, it means this intermediate + // result has an overflow. The final accumulator of this group will + // be null. + rows.applyToSelected([&](vector_size_t i) { setNull(groups[i]); }); + } else { + auto sum = sumVector->valueAt(decodedIndex); + auto isEmpty = isEmptyVector->valueAt(decodedIndex); + rows.applyToSelected([&](vector_size_t i) { + clearNull(groups[i]); + updateNonNullValue(groups[i], sum, isEmpty); + }); + } + } + } else if (decodedPartial_.mayHaveNulls()) { + rows.applyToSelected([&](vector_size_t i) { + if (decodedPartial_.isNullAt(i)) { + return; + } + auto decodedIndex = decodedPartial_.index(i); + if (!isEmptyVector->valueAt(decodedIndex) && + sumVector->isNullAt(decodedIndex)) { + setNull(groups[i]); + } else { + auto sum = sumVector->valueAt(decodedIndex); + auto isEmpty = isEmptyVector->valueAt(decodedIndex); + updateNonNullValue(groups[i], sum, isEmpty); + } + }); + } else { + rows.applyToSelected([&](vector_size_t i) { + clearNull(groups[i]); + auto decodedIndex = decodedPartial_.index(i); + if (!isEmptyVector->valueAt(decodedIndex) && + sumVector->isNullAt(decodedIndex)) { + setNull(groups[i]); + } else { + auto sum = sumVector->valueAt(decodedIndex); + auto isEmpty = isEmptyVector->valueAt(decodedIndex); + updateNonNullValue(groups[i], sum, isEmpty); + } + }); + } + } + + void addSingleGroupIntermediateResults( + char* group, + const SelectivityVector& rows, + const std::vector& args, + bool /* mayPushdown */) override { + decodedPartial_.decode(*args[0], rows); + VELOX_CHECK_EQ( + decodedPartial_.base()->encoding(), VectorEncoding::Simple::ROW); + auto baseRowVector = dynamic_cast(decodedPartial_.base()); + auto sumVector = baseRowVector->childAt(0)->as>(); + auto isEmptyVector = baseRowVector->childAt(1)->as>(); + if (decodedPartial_.isConstantMapping()) { + if (!decodedPartial_.isNullAt(0)) { + auto decodedIndex = decodedPartial_.index(0); + if (!isEmptyVector->valueAt(decodedIndex) && + sumVector->isNullAt(decodedIndex)) { + setNull(group); + } else { + auto sum = sumVector->valueAt(decodedIndex); + auto isEmpty = isEmptyVector->valueAt(decodedIndex); + if (rows.hasSelections()) { + clearNull(group); + } + rows.applyToSelected([&](vector_size_t i) { + updateNonNullValue(group, sum, isEmpty); + }); + } + } + } else if (decodedPartial_.mayHaveNulls()) { + rows.applyToSelected([&](vector_size_t i) { + if (decodedPartial_.isNullAt(i)) { + return; + } + auto decodedIndex = decodedPartial_.index(i); + if (!isEmptyVector->valueAt(decodedIndex) && + sumVector->isNullAt(decodedIndex)) { + setNull(group); + return; + } else { + clearNull(group); + auto sum = sumVector->valueAt(decodedIndex); + auto isEmpty = isEmptyVector->valueAt(decodedIndex); + updateNonNullValue(group, sum, isEmpty); + } + }); + } else { + if (rows.hasSelections()) { + clearNull(group); + } + rows.applyToSelected([&](vector_size_t i) { + auto decodedIndex = decodedPartial_.index(i); + if (!isEmptyVector->valueAt(decodedIndex) && + sumVector->isNullAt(decodedIndex)) { + setNull(group); + return; + } else { + auto sum = sumVector->valueAt(decodedIndex); + auto isEmpty = isEmptyVector->valueAt(decodedIndex); + updateNonNullValue(group, sum, isEmpty); + } + }); + } + } + + private: + template + inline void updateNonNullValue(char* group, TResultType value, bool isEmpty) { + if constexpr (tableHasNulls) { + exec::Aggregate::clearNull(group); + } + auto decimalSum = accumulator(group); + decimalSum->overflow += + DecimalUtil::addWithOverflow(decimalSum->sum, value, decimalSum->sum); + decimalSum->isEmpty &= isEmpty; + } + + template + inline void mergeAccumulators(char* group, DecimalSum other) { + if constexpr (tableHasNulls) { + exec::Aggregate::clearNull(group); + } + auto decimalSum = accumulator(group); + decimalSum->mergeWith(other); + } + + inline DecimalSum* accumulator(char* group) { + return exec::Aggregate::value(group); + } + + DecodedVector decodedRaw_; + DecodedVector decodedPartial_; + TypePtr sumType_; +}; + +exec::AggregateRegistrationResult registerDecimalSumAggregate( + const std::string& name) { + std::vector> signatures{ + exec::AggregateFunctionSignatureBuilder() + .integerVariable("a_precision") + .integerVariable("a_scale") + .integerVariable("r_precision", "min(38, a_precision + 10)") + .integerVariable("r_scale", "min(38, a_scale)") + .argumentType("DECIMAL(a_precision, a_scale)") + .intermediateType("ROW(DECIMAL(r_precision, r_scale), boolean)") + .returnType("DECIMAL(r_precision, r_scale)") + .build()}; + + return exec::registerAggregateFunction( + name, + std::move(signatures), + [name]( + core::AggregationNode::Step step, + const std::vector& argTypes, + const TypePtr& resultType, + const core::QueryConfig& /*config*/) + -> std::unique_ptr { + VELOX_CHECK_EQ(argTypes.size(), 1, "{} takes only one argument", name); + auto& inputType = argTypes[0]; + auto sumType = + exec::isPartialOutput(step) ? resultType->childAt(0) : resultType; + switch (inputType->kind()) { + case TypeKind::BIGINT: { + DCHECK(exec::isRawInput(step)); + if (inputType->isShortDecimal()) { + if (sumType->isShortDecimal()) { + return std::make_unique>( + resultType, sumType); + } else if (sumType->isLongDecimal()) { + return std::make_unique>( + resultType, sumType); + } + } + } + case TypeKind::HUGEINT: + if (inputType->isLongDecimal()) { + // If inputType is long decimal, + // its output type always be long decimal. + return std::make_unique>( + resultType, sumType); + } + case TypeKind::ROW: { + DCHECK(!exec::isRawInput(step)); + // For intermediate input agg, input intermediate sum type + // is equal to final result sum type. + if (inputType->childAt(0)->isShortDecimal()) { + return std::make_unique>( + resultType, sumType); + } else if (inputType->childAt(0)->isLongDecimal()) { + return std::make_unique>( + resultType, sumType); + } + } + default: + VELOX_CHECK( + false, + "Unknown input type for {} aggregation {}", + name, + inputType->kindName()); + } + }, + true); +} + +} // namespace facebook::velox::functions::aggregate::sparksql diff --git a/velox/functions/sparksql/aggregates/Register.cpp b/velox/functions/sparksql/aggregates/Register.cpp index b44d3c4397d..0eeba920668 100644 --- a/velox/functions/sparksql/aggregates/Register.cpp +++ b/velox/functions/sparksql/aggregates/Register.cpp @@ -19,6 +19,7 @@ #include "velox/functions/sparksql/aggregates/AverageAggregate.h" #include "velox/functions/sparksql/aggregates/BitwiseXorAggregate.h" #include "velox/functions/sparksql/aggregates/BloomFilterAggAggregate.h" +#include "velox/functions/sparksql/aggregates/DecimalSumAggregate.h" #include "velox/functions/sparksql/aggregates/SumAggregate.h" namespace facebook::velox::functions::aggregate::sparksql { @@ -35,5 +36,6 @@ void registerAggregateFunctions( registerBloomFilterAggAggregate(prefix + "bloom_filter_agg"); registerAverage(prefix + "avg", withCompanionFunctions); registerSum(prefix + "sum"); + registerDecimalSumAggregate(prefix + "sum"); } } // namespace facebook::velox::functions::aggregate::sparksql diff --git a/velox/functions/sparksql/aggregates/tests/CMakeLists.txt b/velox/functions/sparksql/aggregates/tests/CMakeLists.txt index 22730f9d7e5..9d06aa73236 100644 --- a/velox/functions/sparksql/aggregates/tests/CMakeLists.txt +++ b/velox/functions/sparksql/aggregates/tests/CMakeLists.txt @@ -16,6 +16,7 @@ add_executable( velox_functions_spark_aggregates_test BitwiseXorAggregationTest.cpp BloomFilterAggAggregateTest.cpp + DecimalSumAggregateTest.cpp FirstAggregateTest.cpp LastAggregateTest.cpp AverageAggregationTest.cpp diff --git a/velox/functions/sparksql/aggregates/tests/DecimalSumAggregateTest.cpp b/velox/functions/sparksql/aggregates/tests/DecimalSumAggregateTest.cpp new file mode 100644 index 00000000000..e2c2887248c --- /dev/null +++ b/velox/functions/sparksql/aggregates/tests/DecimalSumAggregateTest.cpp @@ -0,0 +1,332 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "velox/common/base/tests/GTestUtils.h" +#include "velox/exec/tests/utils/AssertQueryBuilder.h" +#include "velox/exec/tests/utils/PlanBuilder.h" +#include "velox/functions/lib/aggregates/tests/AggregationTestBase.h" +#include "velox/functions/sparksql/aggregates/Register.h" + +using facebook::velox::exec::test::PlanBuilder; +using namespace facebook::velox::exec::test; +using namespace facebook::velox::functions::aggregate::test; + +namespace facebook::velox::functions::aggregate::sparksql::test { +namespace { +class DecimalSumAggregateTest : public AggregationTestBase { + protected: + void SetUp() override { + AggregationTestBase::SetUp(); + registerAggregateFunctions("spark_"); + allowInputShuffle(); + } + + protected: + // check global partial agg overflow, and final agg output null + void decimalGlobalSumOverflow( + const std::vector>& input, + const std::vector>& output) { + const TypePtr type = DECIMAL(38, 0); + auto in = makeRowVector({makeNullableFlatVector({input}, type)}); + auto expected = + makeRowVector({makeNullableFlatVector({output}, type)}); + PlanBuilder builder(pool()); + builder.values({in}); + builder.partialAggregation({}, {"spark_sum(c0)"}).finalAggregation(); + AssertQueryBuilder queryBuilder( + builder.planNode(), this->duckDbQueryRunner_); + queryBuilder.assertResults({expected}); + } + + // check group by partial agg overflow, and final agg output null + void decimalGroupBySumOverflow( + const std::vector>& input) { + const TypePtr type = DECIMAL(38, 0); + auto in = makeRowVector( + {makeFlatVector(20, [](auto row) { return row % 10; }), + makeNullableFlatVector(input, type)}); + auto expected = makeRowVector( + {makeFlatVector(10, [](auto row) { return row; }), + makeNullableFlatVector( + std::vector>(10, std::nullopt), type)}); + PlanBuilder builder(pool()); + builder.values({in}); + builder.partialAggregation({"c0"}, {"spark_sum(c1)"}).finalAggregation(); + AssertQueryBuilder queryBuilder( + builder.planNode(), this->duckDbQueryRunner_); + queryBuilder.assertResults({expected}); + } + + template + void decimalSumAllNulls( + const std::vector>& input, + const TypePtr& inputType, + const std::vector>& output, + const TypePtr& outputType) { + std::vector vectors; + FlatVectorPtr inputDecimalVector; + if constexpr (std::is_same_v) { + inputDecimalVector = makeNullableFlatVector(input, inputType); + } else { + inputDecimalVector = makeNullableFlatVector(input, inputType); + } + for (int i = 0; i < 5; ++i) { + vectors.emplace_back(makeRowVector( + {makeFlatVector(20, [](auto row) { return row % 4; }), + inputDecimalVector})); + } + + FlatVectorPtr outputDecimalVector; + if constexpr (std::is_same_v) { + outputDecimalVector = makeNullableFlatVector(output, outputType); + } else { + outputDecimalVector = + makeNullableFlatVector(output, outputType); + } + auto expected = makeRowVector( + {makeFlatVector(std::vector{0, 1, 2, 3}), + outputDecimalVector}); + PlanBuilder builder(pool()); + builder.values({vectors}); + builder.singleAggregation({"c0"}, {"spark_sum(c1)"}); + AssertQueryBuilder queryBuilder( + builder.planNode(), this->duckDbQueryRunner_); + queryBuilder.assertResults({expected}); + } +}; + +TEST_F(DecimalSumAggregateTest, sumDecimal) { + std::vector> shortDecimalRawVector; + std::vector> longDecimalRawVector; + for (int i = 0; i < 1000; ++i) { + shortDecimalRawVector.emplace_back(i * 1000); + longDecimalRawVector.emplace_back(HugeInt::build(i * 10, i * 100)); + } + shortDecimalRawVector.emplace_back(std::nullopt); + longDecimalRawVector.emplace_back(std::nullopt); + auto input = makeRowVector( + {makeNullableFlatVector(shortDecimalRawVector, DECIMAL(10, 1)), + makeNullableFlatVector(longDecimalRawVector, DECIMAL(23, 4))}); + createDuckDbTable({input}); + testAggregations( + {input}, + {}, + {"spark_sum(c0)", "spark_sum(c1)"}, + "SELECT sum(c0), sum(c1) FROM tmp"); + + // Short decimal sum aggregation with multiple groups. + auto inputShortDecimalRows = { + makeRowVector( + {makeNullableFlatVector({1, 1}), + makeFlatVector( + std::vector{37220, 53450}, DECIMAL(5, 2))}), + makeRowVector( + {makeNullableFlatVector({2, 2}), + makeFlatVector( + std::vector{10410, 9250}, DECIMAL(5, 2))}), + makeRowVector( + {makeNullableFlatVector({3, 3}), + makeFlatVector( + std::vector{-12783, 0}, DECIMAL(5, 2))}), + makeRowVector( + {makeNullableFlatVector({1, 2}), + makeFlatVector( + std::vector{23178, 41093}, DECIMAL(5, 2))}), + makeRowVector( + {makeNullableFlatVector({2, 3}), + makeFlatVector( + std::vector{-10023, 5290}, DECIMAL(5, 2))}), + }; + + auto expectedShortDecimalResult = { + makeRowVector( + {makeNullableFlatVector({1}), + makeFlatVector( + std::vector{113848}, DECIMAL(15, 2))}), + makeRowVector( + {makeNullableFlatVector({2}), + makeFlatVector( + std::vector{50730}, DECIMAL(15, 2))}), + makeRowVector( + {makeNullableFlatVector({3}), + makeFlatVector( + std::vector{-7493}, DECIMAL(15, 2))})}; + + testAggregations( + inputShortDecimalRows, + {"c0"}, + {"spark_sum(c1)"}, + expectedShortDecimalResult); + + // Long decimal sum aggregation with multiple groups. + auto inputLongDecimalRows = { + makeRowVector( + {makeNullableFlatVector({1, 1}), + makeFlatVector( + {HugeInt::build(13, 113848), HugeInt::build(12, 53450)}, + DECIMAL(20, 2))}), + makeRowVector( + {makeNullableFlatVector({2, 2}), + makeFlatVector( + {HugeInt::build(21, 10410), HugeInt::build(17, 9250)}, + DECIMAL(20, 2))}), + makeRowVector( + {makeNullableFlatVector({3, 3}), + makeFlatVector( + {HugeInt::build(25, 12783), HugeInt::build(19, 0)}, + DECIMAL(20, 2))}), + makeRowVector( + {makeNullableFlatVector({1, 2}), + makeFlatVector( + {HugeInt::build(31, 23178), HugeInt::build(82, 41093)}, + DECIMAL(20, 2))}), + makeRowVector( + {makeNullableFlatVector({2, 3}), + makeFlatVector( + {HugeInt::build(25, 10023), HugeInt::build(43, 5290)}, + DECIMAL(20, 2))}), + }; + + auto expectedLongDecimalResult = { + makeRowVector( + {makeNullableFlatVector({1}), + makeFlatVector( + std::vector{HugeInt::build(56, 190476)}, + DECIMAL(38, 2))}), + makeRowVector( + {makeNullableFlatVector({2}), + makeFlatVector( + std::vector{HugeInt::build(145, 70776)}, + DECIMAL(38, 2))}), + makeRowVector( + {makeNullableFlatVector({3}), + makeFlatVector( + std::vector{HugeInt::build(87, 18073)}, + DECIMAL(38, 2))})}; + + testAggregations( + inputLongDecimalRows, + {"c0"}, + {"spark_sum(c1)"}, + expectedLongDecimalResult); +} + +TEST_F(DecimalSumAggregateTest, globalSumDecimalOverflow) { + // Test Positive Overflow. + std::vector> longDecimalInput; + std::vector> longDecimalOutput; + // Create input with 2 kLongDecimalMax. + longDecimalInput.emplace_back(DecimalUtil::kLongDecimalMax); + longDecimalInput.emplace_back(DecimalUtil::kLongDecimalMax); + // The sum must overflow, and will return null + decimalGlobalSumOverflow(longDecimalInput, {std::nullopt}); + + // Now add kLongDecimalMin. + // The sum now must not overflow. + longDecimalInput.emplace_back(DecimalUtil::kLongDecimalMin); + longDecimalOutput.emplace_back(DecimalUtil::kLongDecimalMax); + decimalGlobalSumOverflow(longDecimalInput, longDecimalOutput); + + // Test Negative Overflow. + longDecimalInput.clear(); + longDecimalOutput.clear(); + + // Create input with 2 kLongDecimalMin. + longDecimalInput.emplace_back(DecimalUtil::kLongDecimalMin); + longDecimalInput.emplace_back(DecimalUtil::kLongDecimalMin); + + // The sum must overflow, and will return null + decimalGlobalSumOverflow(longDecimalInput, {std::nullopt}); + + // Now add kLongDecimalMax. + // The sum now must not overflow. + longDecimalInput.emplace_back(DecimalUtil::kLongDecimalMax); + longDecimalOutput.emplace_back(DecimalUtil::kLongDecimalMin); + decimalGlobalSumOverflow(longDecimalInput, longDecimalOutput); + + // Check value in range. + longDecimalInput.clear(); + longDecimalInput.emplace_back(DecimalUtil::kLongDecimalMax); + longDecimalInput.emplace_back(1); + decimalGlobalSumOverflow(longDecimalInput, {std::nullopt}); + + longDecimalInput.clear(); + longDecimalInput.emplace_back(DecimalUtil::kLongDecimalMin); + longDecimalInput.emplace_back(-1); + decimalGlobalSumOverflow(longDecimalInput, {std::nullopt}); +} + +TEST_F(DecimalSumAggregateTest, groupBySumDecimalOverflow) { + // Test Positive Overflow. + decimalGroupBySumOverflow( + std::vector>(20, DecimalUtil::kLongDecimalMax)); + + // Test Negative Overflow. + decimalGroupBySumOverflow( + std::vector>(20, DecimalUtil::kLongDecimalMin)); + + // Check value in range. + auto decimalVector = + std::vector>(10, DecimalUtil::kLongDecimalMax); + auto oneValueVector = std::vector>(10, 1); + decimalVector.insert( + decimalVector.end(), oneValueVector.begin(), oneValueVector.end()); + decimalGroupBySumOverflow(decimalVector); + + decimalVector = + std::vector>(10, DecimalUtil::kLongDecimalMin); + oneValueVector = std::vector>(10, -1); + decimalVector.insert( + decimalVector.end(), oneValueVector.begin(), oneValueVector.end()); + decimalGroupBySumOverflow(decimalVector); +} + +/// Test if all values in some groups are null, +/// the final sum of this group should be null. +TEST_F(DecimalSumAggregateTest, someGroupsAllnullValues) { + std::vector> shortDecimalNulls(20); + std::vector> longDecimalNulls(20); + for (int i = 0; i < 20; i++) { + if (i % 4 == 1 || i % 4 == 3) { + // not all groups are null + shortDecimalNulls[i] = 1; + longDecimalNulls[i] = 1; + } + } + + // Test short decimal inputs and the output sum is short decimal. + decimalSumAllNulls( + shortDecimalNulls, + DECIMAL(7, 2), + std::vector>{std::nullopt, 25, std::nullopt, 25}, + DECIMAL(17, 2)); + + // Test short decimal inputs and the output sum is long decimal. + decimalSumAllNulls( + shortDecimalNulls, + DECIMAL(17, 2), + std::vector>{std::nullopt, 25, std::nullopt, 25}, + DECIMAL(27, 2)); + + // Test long decimal inputs and the output sum is long decimal. + decimalSumAllNulls( + longDecimalNulls, + DECIMAL(25, 2), + std::vector>{std::nullopt, 25, std::nullopt, 25}, + DECIMAL(35, 2)); +} +} // namespace +} // namespace facebook::velox::functions::aggregate::sparksql::test From 05dfe33739836ec22ef6826cc06ba38eee006d02 Mon Sep 17 00:00:00 2001 From: yan ma Date: Wed, 29 Nov 2023 20:02:13 +0800 Subject: [PATCH 06/28] Disable check for an compatibility issue with Spark3.2 --- .../sparksql/specialforms/DecimalRound.cpp | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/velox/functions/sparksql/specialforms/DecimalRound.cpp b/velox/functions/sparksql/specialforms/DecimalRound.cpp index d8091145263..6635d5b54e6 100644 --- a/velox/functions/sparksql/specialforms/DecimalRound.cpp +++ b/velox/functions/sparksql/specialforms/DecimalRound.cpp @@ -37,16 +37,19 @@ class DecimalRoundFunction : public exec::VectorFunction { inputScale_(inputScale), resultPrecision_(resultPrecision), resultScale_(resultScale) { - const auto [p, s] = DecimalRoundCallToSpecialForm::getResultPrecisionScale( - inputPrecision, inputScale, scale); - VELOX_USER_CHECK_EQ( - p, - resultPrecision, - "The result precision of decimal_round is inconsistent with Spark expected."); - VELOX_USER_CHECK_EQ( - s, - resultScale, - "The result scale of decimal_round is inconsistent with Spark expected."); + // const auto [p, s] = + // DecimalRoundCallToSpecialForm::getResultPrecisionScale( + // inputPrecision, inputScale, scale); + // VELOX_USER_CHECK_EQ( + // p, + // resultPrecision, + // "The result precision of decimal_round is inconsistent with Spark + // expected."); + // VELOX_USER_CHECK_EQ( + // s, + // resultScale, + // "The result scale of decimal_round is inconsistent with Spark + // expected."); // Decide the rescale factor of divide and multiply when rounding to a // negative scale. From 255a98d7bf0582f2f3957260157c75027b2b9eb0 Mon Sep 17 00:00:00 2001 From: rui-mo Date: Fri, 1 Sep 2023 13:37:51 +0800 Subject: [PATCH 07/28] Fix replace SparkSQL function (4922) --- velox/functions/lib/string/StringCore.h | 8 +++++++ velox/functions/lib/string/StringImpl.h | 14 +++++++---- velox/functions/prestosql/StringFunctions.cpp | 23 ++++++++++++++++--- velox/functions/sparksql/Register.cpp | 3 ++- velox/functions/sparksql/tests/StringTest.cpp | 22 ++++++++++++++++++ 5 files changed, 62 insertions(+), 8 deletions(-) diff --git a/velox/functions/lib/string/StringCore.h b/velox/functions/lib/string/StringCore.h index 42550584e55..76f6be3e698 100644 --- a/velox/functions/lib/string/StringCore.h +++ b/velox/functions/lib/string/StringCore.h @@ -297,6 +297,7 @@ inline int64_t findNthInstanceByteIndexFromEnd( /// each charecter. When inputString is empty results is empty. /// replace("", "", "x") = "" /// replace("aa", "", "x") = "xaxax" +template inline static size_t replace( char* outputString, const std::string_view& inputString, @@ -307,6 +308,13 @@ inline static size_t replace( return 0; } + if (ignoreEmptyReplaced && replaced.size() == 0) { + if (!inPlace) { + std::memcpy(outputString, inputString.data(), inputString.size()); + } + return inputString.size(); + } + size_t readPosition = 0; size_t writePosition = 0; // Copy needed in out of place replace, and when replaced and replacement are diff --git a/velox/functions/lib/string/StringImpl.h b/velox/functions/lib/string/StringImpl.h index 681cee524c7..9649c5ce128 100644 --- a/velox/functions/lib/string/StringImpl.h +++ b/velox/functions/lib/string/StringImpl.h @@ -209,7 +209,10 @@ stringPosition(const T& string, const T& subString, int64_t instance = 0) { /// Replace replaced with replacement in inputString and write results to /// outputString. -template +template < + typename TOutString, + typename TInString, + bool ignoreEmptyReplaced = false> FOLLY_ALWAYS_INLINE void replace( TOutString& outputString, const TInString& inputString, @@ -226,7 +229,7 @@ FOLLY_ALWAYS_INLINE void replace( (inputString.size() / replaced.size()) * replacement.size()); } - auto outputSize = stringCore::replace( + auto outputSize = stringCore::replace( outputString.data(), std::string_view(inputString.data(), inputString.size()), std::string_view(replaced.data(), replaced.size()), @@ -237,14 +240,17 @@ FOLLY_ALWAYS_INLINE void replace( } /// Replace replaced with replacement in place in string. -template +template < + typename TInOutString, + typename TInString, + bool ignoreEmptyReplaced = false> FOLLY_ALWAYS_INLINE void replaceInPlace( TInOutString& string, const TInString& replaced, const TInString& replacement) { assert(replacement.size() <= replaced.size() && "invalid inplace replace"); - auto outputSize = stringCore::replace( + auto outputSize = stringCore::replace( string.data(), std::string_view(string.data(), string.size()), std::string_view(replaced.data(), replaced.size()), diff --git a/velox/functions/prestosql/StringFunctions.cpp b/velox/functions/prestosql/StringFunctions.cpp index b6387c2f463..2c9e71909c4 100644 --- a/velox/functions/prestosql/StringFunctions.cpp +++ b/velox/functions/prestosql/StringFunctions.cpp @@ -284,7 +284,8 @@ class ConcatFunction : public exec::VectorFunction { * If search is an empty string, inserts replace in front of every character *and at the end of the string. **/ -class Replace : public exec::VectorFunction { +template +class ReplaceBase : public exec::VectorFunction { private: template < typename StringReader, @@ -298,7 +299,10 @@ class Replace : public exec::VectorFunction { FlatVector* results) const { rows.applyToSelected([&](int row) { auto proxy = exec::StringWriter<>(results, row); - stringImpl::replace( + stringImpl::replace< + decltype(proxy), + decltype(searchReader(row)), + ignoreEmptyReplaced>( proxy, stringReader(row), searchReader(row), replaceReader(row)); proxy.finalize(); }); @@ -317,7 +321,10 @@ class Replace : public exec::VectorFunction { rows.applyToSelected([&](int row) { auto proxy = exec::StringWriter( results, row, stringReader(row) /*reusedInput*/, true /*inPlace*/); - stringImpl::replaceInPlace(proxy, searchReader(row), replaceReader(row)); + stringImpl::replaceInPlace< + decltype(proxy), + decltype(searchReader(row)), + ignoreEmptyReplaced>(proxy, searchReader(row), replaceReader(row)); proxy.finalize(); }); } @@ -429,6 +436,11 @@ class Replace : public exec::VectorFunction { return {{0, 2}}; } }; + +class Replace : public ReplaceBase {}; + +class ReplaceIgnoreEmptyReplaced + : public ReplaceBase {}; } // namespace VELOX_DECLARE_VECTOR_FUNCTION( @@ -456,4 +468,9 @@ VELOX_DECLARE_VECTOR_FUNCTION( Replace::signatures(), std::make_unique()); +VELOX_DECLARE_VECTOR_FUNCTION( + udf_replace_ignore_empty_replaced, + ReplaceIgnoreEmptyReplaced::signatures(), + std::make_unique()); + } // namespace facebook::velox::functions diff --git a/velox/functions/sparksql/Register.cpp b/velox/functions/sparksql/Register.cpp index 26976c492d1..0c8905514ba 100644 --- a/velox/functions/sparksql/Register.cpp +++ b/velox/functions/sparksql/Register.cpp @@ -72,7 +72,8 @@ static void workAroundRegistrationMacro(const std::string& prefix) { // String functions. VELOX_REGISTER_VECTOR_FUNCTION(udf_concat, prefix + "concat"); VELOX_REGISTER_VECTOR_FUNCTION(udf_lower, prefix + "lower"); - VELOX_REGISTER_VECTOR_FUNCTION(udf_replace, prefix + "replace"); + VELOX_REGISTER_VECTOR_FUNCTION( + udf_replace_ignore_empty_replaced, prefix + "replace"); VELOX_REGISTER_VECTOR_FUNCTION(udf_upper, prefix + "upper"); // Logical. VELOX_REGISTER_VECTOR_FUNCTION(udf_not, prefix + "not"); diff --git a/velox/functions/sparksql/tests/StringTest.cpp b/velox/functions/sparksql/tests/StringTest.cpp index adfdca35482..cc3da3c918b 100644 --- a/velox/functions/sparksql/tests/StringTest.cpp +++ b/velox/functions/sparksql/tests/StringTest.cpp @@ -205,6 +205,14 @@ class StringTest : public SparkFunctionBaseTest { std::optional toBase) { return evaluateOnce("conv(c0, c1, c2)", str, fromBase, toBase); } + + std::optional replace( + std::optional str, + std::optional replaced, + std::optional replacement) { + return evaluateOnce( + "replace(c0, c1, c2)", str, replaced, replacement); + } }; TEST_F(StringTest, Ascii) { @@ -772,5 +780,19 @@ TEST_F(StringTest, conv) { EXPECT_EQ(conv("", std::nullopt, 16), std::nullopt); EXPECT_EQ(conv("", 10, std::nullopt), std::nullopt); } + +TEST_F(StringTest, replace) { + EXPECT_EQ(replace("aaabaac", "a", "z"), "zzzbzzc"); + EXPECT_EQ(replace("aaabaac", "", "z"), "aaabaac"); + EXPECT_EQ(replace("aaabaac", "a", ""), "bc"); + EXPECT_EQ(replace("aaabaac", "x", "z"), "aaabaac"); + EXPECT_EQ(replace("aaabaac", "ab", "z"), "aazaac"); + EXPECT_EQ(replace("aaabaac", "aa", "z"), "zabzc"); + EXPECT_EQ(replace("aaabaac", "aa", "xyz"), "xyzabxyzc"); + EXPECT_EQ(replace("aaabaac", "aaabaac", "z"), "z"); + EXPECT_EQ( + replace("123\u6570\u6570\u636E", "\u6570\u636E", "data"), + "123\u6570data"); +} } // namespace } // namespace facebook::velox::functions::sparksql::test From 24483789db9ea4988dba4b2da1add4566bb0ba0c Mon Sep 17 00:00:00 2001 From: rui-mo Date: Wed, 18 Oct 2023 10:06:52 +0800 Subject: [PATCH 08/28] Fix decimal agg signatures --- .../sparksql/aggregates/AverageAggregate.cpp | 9 +++ .../sparksql/aggregates/DecimalSumAggregate.h | 69 ------------------- .../sparksql/aggregates/SumAggregate.cpp | 45 +++++++++++- .../tests/DecimalSumAggregateTest.cpp | 2 +- 4 files changed, 54 insertions(+), 71 deletions(-) diff --git a/velox/functions/sparksql/aggregates/AverageAggregate.cpp b/velox/functions/sparksql/aggregates/AverageAggregate.cpp index df401fc5aea..e5a689776bc 100644 --- a/velox/functions/sparksql/aggregates/AverageAggregate.cpp +++ b/velox/functions/sparksql/aggregates/AverageAggregate.cpp @@ -393,6 +393,15 @@ exec::AggregateRegistrationResult registerAverage( .returnType("DECIMAL(r_precision, r_scale)") .build()); + signatures.push_back( + exec::AggregateFunctionSignatureBuilder() + .integerVariable("a_precision") + .integerVariable("a_scale") + .argumentType("DECIMAL(a_precision, a_scale)") + .intermediateType("ROW(DECIMAL(a_precision, a_scale), BIGINT)") + .returnType("DECIMAL(a_precision, a_scale)") + .build()); + return exec::registerAggregateFunction( name, std::move(signatures), diff --git a/velox/functions/sparksql/aggregates/DecimalSumAggregate.h b/velox/functions/sparksql/aggregates/DecimalSumAggregate.h index 3ea6e06caf1..c69aaae6543 100644 --- a/velox/functions/sparksql/aggregates/DecimalSumAggregate.h +++ b/velox/functions/sparksql/aggregates/DecimalSumAggregate.h @@ -378,73 +378,4 @@ class DecimalSumAggregate : public exec::Aggregate { TypePtr sumType_; }; -exec::AggregateRegistrationResult registerDecimalSumAggregate( - const std::string& name) { - std::vector> signatures{ - exec::AggregateFunctionSignatureBuilder() - .integerVariable("a_precision") - .integerVariable("a_scale") - .integerVariable("r_precision", "min(38, a_precision + 10)") - .integerVariable("r_scale", "min(38, a_scale)") - .argumentType("DECIMAL(a_precision, a_scale)") - .intermediateType("ROW(DECIMAL(r_precision, r_scale), boolean)") - .returnType("DECIMAL(r_precision, r_scale)") - .build()}; - - return exec::registerAggregateFunction( - name, - std::move(signatures), - [name]( - core::AggregationNode::Step step, - const std::vector& argTypes, - const TypePtr& resultType, - const core::QueryConfig& /*config*/) - -> std::unique_ptr { - VELOX_CHECK_EQ(argTypes.size(), 1, "{} takes only one argument", name); - auto& inputType = argTypes[0]; - auto sumType = - exec::isPartialOutput(step) ? resultType->childAt(0) : resultType; - switch (inputType->kind()) { - case TypeKind::BIGINT: { - DCHECK(exec::isRawInput(step)); - if (inputType->isShortDecimal()) { - if (sumType->isShortDecimal()) { - return std::make_unique>( - resultType, sumType); - } else if (sumType->isLongDecimal()) { - return std::make_unique>( - resultType, sumType); - } - } - } - case TypeKind::HUGEINT: - if (inputType->isLongDecimal()) { - // If inputType is long decimal, - // its output type always be long decimal. - return std::make_unique>( - resultType, sumType); - } - case TypeKind::ROW: { - DCHECK(!exec::isRawInput(step)); - // For intermediate input agg, input intermediate sum type - // is equal to final result sum type. - if (inputType->childAt(0)->isShortDecimal()) { - return std::make_unique>( - resultType, sumType); - } else if (inputType->childAt(0)->isLongDecimal()) { - return std::make_unique>( - resultType, sumType); - } - } - default: - VELOX_CHECK( - false, - "Unknown input type for {} aggregation {}", - name, - inputType->kindName()); - } - }, - true); -} - } // namespace facebook::velox::functions::aggregate::sparksql diff --git a/velox/functions/sparksql/aggregates/SumAggregate.cpp b/velox/functions/sparksql/aggregates/SumAggregate.cpp index 486331631ec..bd9a2c5b7c8 100644 --- a/velox/functions/sparksql/aggregates/SumAggregate.cpp +++ b/velox/functions/sparksql/aggregates/SumAggregate.cpp @@ -16,6 +16,7 @@ #include "velox/functions/sparksql/aggregates/SumAggregate.h" #include "velox/functions/lib/aggregates/SumAggregateBase.h" +#include "velox/functions/sparksql/aggregates/DecimalSumAggregate.h" using namespace facebook::velox::functions::aggregate; @@ -38,6 +39,15 @@ exec::AggregateRegistrationResult registerSum(const std::string& name) { .intermediateType("double") .argumentType("double") .build(), + exec::AggregateFunctionSignatureBuilder() + .integerVariable("a_precision") + .integerVariable("a_scale") + .integerVariable("r_precision", "min(38, a_precision + 10)") + .integerVariable("r_scale", "min(38, a_scale)") + .argumentType("DECIMAL(a_precision, a_scale)") + .intermediateType("ROW(DECIMAL(r_precision, r_scale), boolean)") + .returnType("DECIMAL(r_precision, r_scale)") + .build(), }; for (const auto& inputType : {"tinyint", "smallint", "integer", "bigint"}) { @@ -71,12 +81,31 @@ exec::AggregateRegistrationResult registerSum(const std::string& name) { BIGINT()); case TypeKind::BIGINT: { if (inputType->isShortDecimal()) { - VELOX_NYI(); + auto sumType = exec::isPartialOutput(step) + ? resultType->childAt(0) + : resultType; + if (sumType->isShortDecimal()) { + return std::make_unique>( + resultType, sumType); + } else if (sumType->isLongDecimal()) { + return std::make_unique>( + resultType, sumType); + } + VELOX_UNREACHABLE(); } return std::make_unique>( BIGINT()); } case TypeKind::HUGEINT: { + if (inputType->isLongDecimal()) { + auto sumType = exec::isPartialOutput(step) + ? resultType->childAt(0) + : resultType; + // If inputType is long decimal, + // its output type always be long decimal. + return std::make_unique>( + resultType, sumType); + } VELOX_NYI(); } case TypeKind::REAL: @@ -93,6 +122,20 @@ exec::AggregateRegistrationResult registerSum(const std::string& name) { } return std::make_unique>( DOUBLE()); + case TypeKind::ROW: { + DCHECK(!exec::isRawInput(step)); + // For intermediate input agg, input intermediate sum type + // is equal to final result sum type. + auto sumType = exec::isPartialOutput(step) ? resultType->childAt(0) + : resultType; + if (inputType->childAt(0)->isShortDecimal()) { + return std::make_unique>( + resultType, sumType); + } else if (inputType->childAt(0)->isLongDecimal()) { + return std::make_unique>( + resultType, sumType); + } + } default: VELOX_CHECK( false, diff --git a/velox/functions/sparksql/aggregates/tests/DecimalSumAggregateTest.cpp b/velox/functions/sparksql/aggregates/tests/DecimalSumAggregateTest.cpp index e2c2887248c..549cec70df1 100644 --- a/velox/functions/sparksql/aggregates/tests/DecimalSumAggregateTest.cpp +++ b/velox/functions/sparksql/aggregates/tests/DecimalSumAggregateTest.cpp @@ -17,7 +17,7 @@ #include "velox/common/base/tests/GTestUtils.h" #include "velox/exec/tests/utils/AssertQueryBuilder.h" #include "velox/exec/tests/utils/PlanBuilder.h" -#include "velox/functions/lib/aggregates/tests/AggregationTestBase.h" +#include "velox/functions/lib/aggregates/tests/utils/AggregationTestBase.h" #include "velox/functions/sparksql/aggregates/Register.h" using facebook::velox::exec::test::PlanBuilder; From faba84e782267a9035caca146712d93a7f965181 Mon Sep 17 00:00:00 2001 From: PHILO-HE Date: Fri, 3 Nov 2023 09:04:17 +0800 Subject: [PATCH 09/28] Issues calling reclaimer / arbitrator APIs in single-thread execution (5790) --- velox/common/memory/Memory.cpp | 27 +++++++++++++++------------ velox/core/QueryCtx.h | 20 +++++++++++++++----- velox/exec/Task.cpp | 21 ++++++++------------- velox/exec/tests/TaskTest.cpp | 2 +- 4 files changed, 39 insertions(+), 31 deletions(-) diff --git a/velox/common/memory/Memory.cpp b/velox/common/memory/Memory.cpp index 1316dee5081..d2adb1cf65d 100644 --- a/velox/common/memory/Memory.cpp +++ b/velox/common/memory/Memory.cpp @@ -122,19 +122,22 @@ std::shared_ptr MemoryManager::addRootPool( options.debugEnabled = debugEnabled_; options.coreOnAllocationFailureEnabled = coreOnAllocationFailureEnabled_; - folly::SharedMutex::WriteHolder guard{mutex_}; - if (pools_.find(poolName) != pools_.end()) { - VELOX_FAIL("Duplicate root pool name found: {}", poolName); + std::shared_ptr pool; + { + folly::SharedMutex::WriteHolder guard{mutex_}; + if (pools_.find(poolName) != pools_.end()) { + VELOX_FAIL("Duplicate root pool name found: {}", poolName); + } + pool = std::make_shared( + this, + poolName, + MemoryPool::Kind::kAggregate, + nullptr, + std::move(reclaimer), + poolDestructionCb_, + options); + pools_.emplace(poolName, pool); } - auto pool = std::make_shared( - this, - poolName, - MemoryPool::Kind::kAggregate, - nullptr, - std::move(reclaimer), - poolDestructionCb_, - options); - pools_.emplace(poolName, pool); VELOX_CHECK_EQ(pool->capacity(), 0); arbitrator_->reserveMemory(pool.get(), capacity); return pool; diff --git a/velox/core/QueryCtx.h b/velox/core/QueryCtx.h index 5d02f9e6ab7..cbae8ceb554 100644 --- a/velox/core/QueryCtx.h +++ b/velox/core/QueryCtx.h @@ -76,11 +76,13 @@ class QueryCtx { return cache_; } - folly::Executor* executor() const { - if (executor_ != nullptr) { - return executor_; - } - auto executor = executorKeepalive_.get(); + bool isExecutorSupplied() const { + auto executor = executor0(); + return executor != nullptr; + } + + folly::Executor* FOLLY_NONNULL executor() const { + auto executor = executor0(); VELOX_CHECK(executor, "Executor was not supplied."); return executor; } @@ -139,6 +141,14 @@ class QueryCtx { } } + folly::Executor* executor0() const { + if (executor_ != nullptr) { + return executor_; + } + auto executor = executorKeepalive_.get(); + return executor; + } + const std::string queryId_; folly::Executor* const executor_{nullptr}; folly::Executor* const spillExecutor_{nullptr}; diff --git a/velox/exec/Task.cpp b/velox/exec/Task.cpp index 9ec1f5ec157..32666e2221c 100644 --- a/velox/exec/Task.cpp +++ b/velox/exec/Task.cpp @@ -535,12 +535,6 @@ RowVectorPtr Task::next(ContinueFuture* future) { createSplitGroupStateLocked(kUngroupedGroupId); std::vector> drivers = createDriversLocked(kUngroupedGroupId); - if (pool_->stats().currentBytes != 0) { - VELOX_FAIL( - "Unexpected memory pool allocations during task[{}] driver initialization: {}", - taskId_, - pool_->treeMemoryUsage()); - } drivers_ = std::move(drivers); } @@ -704,12 +698,6 @@ void Task::createAndStartDrivers(uint32_t concurrentSplitGroups) { // Create drivers. std::vector> drivers = createDriversLocked(kUngroupedGroupId); - if (pool_->stats().currentBytes != 0) { - VELOX_FAIL( - "Unexpected memory pool allocations during task[{}] driver initialization: {}", - taskId_, - pool_->treeMemoryUsage()); - } // Prevent the connecting structures from being cleaned up before all // split groups are finished during the grouped execution mode. @@ -839,9 +827,16 @@ void Task::resume(std::shared_ptr self) { continue; } VELOX_CHECK(!driver->isOnThread() && !driver->isTerminated()); - if (!driver->state().hasBlockingFuture) { + if (!driver->state().hasBlockingFuture && + driver->task()->queryCtx()->isExecutorSupplied()) { // Do not continue a Driver that is blocked on external // event. The Driver gets enqueued by the promise realization. + // + // Do not continue the driver if no executor is supplied, + // Since it's likely that we are in single-thread execution. + // + // 2023/07.13 Hongze: Is there a way to hide the execution model + // (single or async) from here? Driver::enqueue(driver); } } diff --git a/velox/exec/tests/TaskTest.cpp b/velox/exec/tests/TaskTest.cpp index ee78430f361..7df6db66669 100644 --- a/velox/exec/tests/TaskTest.cpp +++ b/velox/exec/tests/TaskTest.cpp @@ -1289,7 +1289,7 @@ DEBUG_ONLY_TEST_F(TaskTest, raceBetweenTaskPauseAndTerminate) { taskThread.join(); } -TEST_F(TaskTest, driverCreationMemoryAllocationCheck) { +TEST_F(TaskTest, DISABLED_driverCreationMemoryAllocationCheck) { exec::Operator::registerOperator(std::make_unique()); auto data = makeRowVector({ makeFlatVector(1'000, [](auto row) { return row; }), From 2de8116926a11c0629c7a6d3b99e40b3b4ef9fe4 Mon Sep 17 00:00:00 2001 From: rui-mo Date: Tue, 17 Oct 2023 10:16:41 +0800 Subject: [PATCH 10/28] Support date as partition value type (7084) --- velox/connectors/hive/SplitReader.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/velox/connectors/hive/SplitReader.cpp b/velox/connectors/hive/SplitReader.cpp index a42b64e3714..7790343f04b 100644 --- a/velox/connectors/hive/SplitReader.cpp +++ b/velox/connectors/hive/SplitReader.cpp @@ -305,8 +305,19 @@ void SplitReader::setPartitionValue( it != partitionKeys_.end(), "ColumnHandle is missing for partition key {}", partitionKey); - auto constValue = VELOX_DYNAMIC_SCALAR_TYPE_DISPATCH( - convertFromString, it->second->dataType()->kind(), value); + velox::variant constValue; + if (it->second->dataType()->isDate()) { + // TODO: need to align with query config for isIso8601. + if (value.has_value()) { + constValue = velox::variant( + velox::util::castFromDateString(StringView(value.value()), false)); + } else { + constValue = velox::variant(TypeKind::INTEGER); + } + } else { + constValue = VELOX_DYNAMIC_SCALAR_TYPE_DISPATCH( + convertFromString, it->second->dataType()->kind(), value); + } setConstantValue(spec, it->second->dataType(), constValue); } From 67e57b3ca4dd24f041ab7efcefe613de2870f1ff Mon Sep 17 00:00:00 2001 From: rui-mo Date: Tue, 17 Oct 2023 10:20:36 +0800 Subject: [PATCH 11/28] Support cast (from float to decimal) --- velox/expression/CastExpr-inl.h | 5 ++--- velox/expression/CastExpr.cpp | 6 +++++- velox/expression/CastExpr.h | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/velox/expression/CastExpr-inl.h b/velox/expression/CastExpr-inl.h index bddf930272a..b4598136e1e 100644 --- a/velox/expression/CastExpr-inl.h +++ b/velox/expression/CastExpr-inl.h @@ -570,14 +570,14 @@ void CastExpr::applyVarcharToDecimalCastKernel( }); } -template +template void CastExpr::applyDoubleToDecimal( const SelectivityVector& rows, const BaseVector& input, exec::EvalCtx& context, const TypePtr& toType, VectorPtr& castResult) { - auto sourceVector = input.as>(); + auto sourceVector = input.as>(); auto rawResults = castResult->asUnchecked>()->mutableRawValues(); const auto toPrecisionScale = getDecimalPrecisionScale(*toType); @@ -604,7 +604,6 @@ void CastExpr::applyDoubleToDecimal( rawResults[row] = rescaledValue.value(); } else { castResult->setNull(row, true); - } }); } diff --git a/velox/expression/CastExpr.cpp b/velox/expression/CastExpr.cpp index c923f92c90f..e5a5785458b 100644 --- a/velox/expression/CastExpr.cpp +++ b/velox/expression/CastExpr.cpp @@ -467,8 +467,12 @@ VectorPtr CastExpr::applyDecimal( applyIntToDecimalCastKernel( rows, input, context, toType, castResult); break; + case TypeKind::REAL: + applyDoubleToDecimal( + rows, input, context, toType, castResult); + break; case TypeKind::DOUBLE: - applyDoubleToDecimal( + applyDoubleToDecimal( rows, input, context, toType, castResult); break; case TypeKind::BIGINT: { diff --git a/velox/expression/CastExpr.h b/velox/expression/CastExpr.h index 928f73b1948..1f6a5168d5e 100644 --- a/velox/expression/CastExpr.h +++ b/velox/expression/CastExpr.h @@ -206,7 +206,7 @@ class CastExpr : public SpecialForm { const TypePtr& toType, VectorPtr& castResult); - template + template void applyDoubleToDecimal( const SelectivityVector& rows, const BaseVector& input, From 10a4ab9d28dd7a99166c5d9485539ab4f1de7d8c Mon Sep 17 00:00:00 2001 From: Jia Ke Date: Thu, 9 Nov 2023 06:34:06 +0000 Subject: [PATCH 12/28] Fix array_union on NaN (7086) --- velox/docs/functions/spark/array.rst | 9 + velox/functions/sparksql/ArrayUnionFunction.h | 89 ++++++++ velox/functions/sparksql/Register.cpp | 21 ++ .../sparksql/tests/ArrayUnionTest.cpp | 208 ++++++++++++++++++ velox/functions/sparksql/tests/CMakeLists.txt | 1 + 5 files changed, 328 insertions(+) create mode 100644 velox/functions/sparksql/ArrayUnionFunction.h create mode 100644 velox/functions/sparksql/tests/ArrayUnionTest.cpp diff --git a/velox/docs/functions/spark/array.rst b/velox/docs/functions/spark/array.rst index 2183f4f301c..f31eb10008f 100644 --- a/velox/docs/functions/spark/array.rst +++ b/velox/docs/functions/spark/array.rst @@ -62,6 +62,15 @@ Array Functions SELECT array_sort(ARRAY [NULL, 1, NULL]); -- [1, NULL, NULL] SELECT array_sort(ARRAY [NULL, 2, 1]); -- [1, 2, NULL] +.. spark:function:: array_union(array(E), array(E1)) -> array(E2) + + Returns an array of the elements in the union of array1 and array2, without duplicates. :: + + SELECT array_union(array(1, 2, 3), array(1, 3, 5)); -- [1, 2, 3, 5] + SELECT array_union(array(1, 3, 5), array(1, 2, 3)); -- [1, 3, 5, 2] + SELECT array_union(array(1, 2, 3), array(1, 3, 5, null)); -- [1, 2, 3, 5, null] + SELECT array_union(array(1, 2, NaN), array(1, 3, NaN)); -- [1, 2, NaN, 3] + .. spark:function:: concat(array(E), array(E1), ..., array(En)) -> array(E, E1, ..., En) Returns the concatenation of array(E), array(E1), ..., array(En). :: diff --git a/velox/functions/sparksql/ArrayUnionFunction.h b/velox/functions/sparksql/ArrayUnionFunction.h new file mode 100644 index 00000000000..c8a4f21f5af --- /dev/null +++ b/velox/functions/sparksql/ArrayUnionFunction.h @@ -0,0 +1,89 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +namespace facebook::velox::functions::sparksql { + +/// This class implements the array union function. +/// +/// DEFINITION: +/// array_union(x, y) → array +/// Returns an array of the elements in the union of x and y, without +/// duplicates. +template +struct ArrayUnionFunction { + VELOX_DEFINE_FUNCTION_TYPES(T) + + // Fast path for primitives. + template + void call(Out& out, const In& inputArray1, const In& inputArray2) { + folly::F14FastSet elementSet; + bool nullAdded = false; + bool nanAdded = false; + auto addItems = [&](auto& inputArray) { + for (const auto& item : inputArray) { + if (item.has_value()) { + if constexpr ( + std::is_same_v>> || + std::is_same_v>>) { + bool isNaN = std::isnan(item.value()); + if ((isNaN && !nanAdded) || + (!isNaN && elementSet.insert(item.value()).second)) { + auto& newItem = out.add_item(); + newItem = item.value(); + } + if (!nanAdded && isNaN) { + nanAdded = true; + } + } else if (elementSet.insert(item.value()).second) { + auto& newItem = out.add_item(); + newItem = item.value(); + } + } else if (!nullAdded) { + nullAdded = true; + out.add_null(); + } + } + }; + addItems(inputArray1); + addItems(inputArray2); + } + + void call( + out_type>>& out, + const arg_type>>& inputArray1, + const arg_type>>& inputArray2) { + folly::F14FastSet elementSet; + bool nullAdded = false; + auto addItems = [&](auto& inputArray) { + for (const auto& item : inputArray) { + if (item.has_value()) { + if (elementSet.insert(item.value()).second) { + auto& newItem = out.add_item(); + newItem.copy_from(item.value()); + } + } else if (!nullAdded) { + nullAdded = true; + out.add_null(); + } + } + }; + addItems(inputArray1); + addItems(inputArray2); + } +}; +} // namespace facebook::velox::functions::sparksql diff --git a/velox/functions/sparksql/Register.cpp b/velox/functions/sparksql/Register.cpp index 0c8905514ba..d0a0ee57956 100644 --- a/velox/functions/sparksql/Register.cpp +++ b/velox/functions/sparksql/Register.cpp @@ -26,6 +26,7 @@ #include "velox/functions/prestosql/StringFunctions.h" #include "velox/functions/sparksql/ArrayMinMaxFunction.h" #include "velox/functions/sparksql/ArraySort.h" +#include "velox/functions/sparksql/ArrayUnionFunction.h" #include "velox/functions/sparksql/Bitwise.h" #include "velox/functions/sparksql/DateTimeFunctions.h" #include "velox/functions/sparksql/Hash.h" @@ -115,6 +116,12 @@ inline void registerArrayMinMaxFunctions(const std::string& prefix) { } } // namespace +template +inline void registerArrayUnionFunctions(const std::string& prefix) { + registerFunction, Array, Array>( + {prefix + "array_union"}); +} + void registerFunctions(const std::string& prefix) { registerAllSpecialFormGeneralFunctions(); @@ -298,6 +305,20 @@ void registerFunctions(const std::string& prefix) { prefix + "unscaled_value", unscaledValueSignatures(), makeUnscaledValue()); + + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions(prefix); + registerArrayUnionFunctions>(prefix); } } // namespace sparksql diff --git a/velox/functions/sparksql/tests/ArrayUnionTest.cpp b/velox/functions/sparksql/tests/ArrayUnionTest.cpp new file mode 100644 index 00000000000..e75719bc5a1 --- /dev/null +++ b/velox/functions/sparksql/tests/ArrayUnionTest.cpp @@ -0,0 +1,208 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "velox/common/base/tests/GTestUtils.h" +#include "velox/functions/sparksql/tests/SparkFunctionBaseTest.h" + +using namespace facebook::velox; +using namespace facebook::velox::test; + +namespace facebook::velox::functions::sparksql::test { +namespace { + +class ArrayUnionTest : public SparkFunctionBaseTest { + protected: + void testExpression( + const std::string& expression, + const std::vector& input, + const VectorPtr& expected) { + auto result = evaluate(expression, makeRowVector(input)); + assertEqualVectors(expected, result); + } + + template + void testFloatArray() { + const auto array1 = makeArrayVector( + {{1.99, 2.78, 3.98, 4.01}, + {3.89, 4.99, 5.13}, + {7.13, 8.91, std::numeric_limits::quiet_NaN()}, + {10.02, 20.01, std::numeric_limits::quiet_NaN()}}); + const auto array2 = makeArrayVector( + {{2.78, 4.01, 5.99}, + {3.89, 4.99, 5.13}, + {7.13, 8.91, std::numeric_limits::quiet_NaN()}, + {40.99, 50.12}}); + + VectorPtr expected; + expected = makeArrayVector({ + {1.99, 2.78, 3.98, 4.01, 5.99}, + {3.89, 4.99, 5.13}, + {7.13, 8.91, std::numeric_limits::quiet_NaN()}, + {10.02, 20.01, std::numeric_limits::quiet_NaN(), 40.99, 50.12}, + }); + testExpression("array_union(c0, c1)", {array1, array2}, expected); + + expected = makeArrayVector({ + {2.78, 4.01, 5.99, 1.99, 3.98}, + {3.89, 4.99, 5.13}, + {7.13, 8.91, std::numeric_limits::quiet_NaN()}, + {40.99, 50.12, 10.02, 20.01, std::numeric_limits::quiet_NaN()}, + }); + testExpression("array_union(c0, c1)", {array2, array1}, expected); + } +}; + +// Union two integer arrays. +TEST_F(ArrayUnionTest, intArray) { + const auto array1 = makeArrayVector( + {{1, 2, 3, 4}, {3, 4, 5}, {7, 8, 9}, {10, 20, 30}}); + const auto array2 = + makeArrayVector({{2, 4, 5}, {3, 4, 5}, {}, {40, 50}}); + VectorPtr expected; + + expected = makeArrayVector({ + {1, 2, 3, 4, 5}, + {3, 4, 5}, + {7, 8, 9}, + {10, 20, 30, 40, 50}, + }); + testExpression("array_union(c0, c1)", {array1, array2}, expected); + + expected = makeArrayVector({ + {2, 4, 5, 1, 3}, + {3, 4, 5}, + {7, 8, 9}, + {40, 50, 10, 20, 30}, + }); + testExpression("array_union(c0, c1)", {array2, array1}, expected); +} + +// Union two float or double arrays. +TEST_F(ArrayUnionTest, floatArray) { + testFloatArray(); + testFloatArray(); +} + +// Union two string arrays. +TEST_F(ArrayUnionTest, stringArray) { + const auto array1 = + makeArrayVector({{"foo", "bar"}, {"foo", "baz"}}); + const auto array2 = + makeArrayVector({{"foo", "bar"}, {"bar", "baz"}}); + VectorPtr expected; + + expected = makeArrayVector({ + {"foo", "bar"}, + {"foo", "baz", "bar"}, + }); + testExpression("array_union(c0, c1)", {array1, array2}, expected); +} + +// Union two integer arrays with null. +TEST_F(ArrayUnionTest, nullArray) { + const auto array1 = makeNullableArrayVector({ + {{1, std::nullopt, 3, 4}}, + {7, 8, 9}, + {{10, std::nullopt, std::nullopt}}, + }); + const auto array2 = makeNullableArrayVector({ + {{std::nullopt, std::nullopt, 3, 5}}, + std::nullopt, + {{1, 10}}, + }); + VectorPtr expected; + + expected = makeNullableArrayVector({ + {{1, std::nullopt, 3, 4, 5}}, + std::nullopt, + {{10, std::nullopt, 1}}, + }); + testExpression("array_union(c0, c1)", {array1, array2}, expected); + + expected = makeNullableArrayVector({ + {{std::nullopt, 3, 5, 1, 4}}, + std::nullopt, + {{1, 10, std::nullopt}}, + }); + testExpression("array_union(c0, c1)", {array2, array1}, expected); +} + +// Union array vectors. +TEST_F(ArrayUnionTest, complexTypes) { + auto baseVector = makeArrayVector( + {{1, 1}, {2, 2}, {3, 3}, {4, 4}, {5, 5}, {6, 6}}); + + // Create arrays of array vector using above base vector. + // [[1, 1], [2, 2]] + // [[3, 3], [4, 4]] + // [[5, 5], [6, 6]] + auto arrayOfArrays1 = makeArrayVector({0, 2, 4}, baseVector); + // [[1, 1], [2, 2], [3, 3]] + // [[4, 4]] + // [[5, 5], [6, 6]] + auto arrayOfArrays2 = makeArrayVector({0, 3, 4}, baseVector); + + // [[1, 1], [2, 2], [3, 3]] + // [[3, 3], [4, 4]] + // [[5, 5], [6, 6]] + auto expected = makeArrayVector( + {0, 3, 5}, + makeArrayVector( + {{1, 1}, {2, 2}, {3, 3}, {3, 3}, {4, 4}, {5, 5}, {6, 6}})); + + testExpression( + "array_union(c0, c1)", {arrayOfArrays1, arrayOfArrays2}, expected); +} + +// Union double array vectors. +TEST_F(ArrayUnionTest, complexDoubleType) { + auto baseVector = makeArrayVector( + {{1.0, 1.0}, + {2.0, 2.0}, + {3.0, 3.0}, + {4.0, 4.0}, + {5.0, std::numeric_limits::quiet_NaN()}, + {6.0, 6.0}}); + + // Create arrays of array vector using above base vector. + // [[1.0, 1.0], [2.0, 2.0]] + // [[3.0, 3.0], [4.0, 4.0]] + // [[5.0, NaN], [6.0, 6.0]] + auto arrayOfArrays1 = makeArrayVector({0, 2, 4}, baseVector); + // [[1.0, 1.0], [2.0, 2.0], [3.0, 3.0]] + // [[4.0, 4.0]] + // [[5.0, NaN], [6.0, 6.0]] + auto arrayOfArrays2 = makeArrayVector({0, 3, 4}, baseVector); + + // [[1.0, 1.0], [2.0, 2.0], [3.0, 3.0]] + // [[3.0, 3.0], [4.0, 4.0]] + // [[5.0, NaN], [6.0, 6.0]] + auto expected = makeArrayVector( + {0, 3, 5}, + makeArrayVector( + {{1.0, 1.0}, + {2.0, 2.0}, + {3.0, 3.0}, + {3.0, 3.0}, + {4.0, 4.0}, + {5.0, std::numeric_limits::quiet_NaN()}, + {6.0, 6.0}})); + + testExpression( + "array_union(c0, c1)", {arrayOfArrays1, arrayOfArrays2}, expected); +} +} // namespace +} // namespace facebook::velox::functions::sparksql::test diff --git a/velox/functions/sparksql/tests/CMakeLists.txt b/velox/functions/sparksql/tests/CMakeLists.txt index 49b63c10d81..ac04521c52a 100644 --- a/velox/functions/sparksql/tests/CMakeLists.txt +++ b/velox/functions/sparksql/tests/CMakeLists.txt @@ -18,6 +18,7 @@ add_executable( ArrayMaxTest.cpp ArrayMinTest.cpp ArraySortTest.cpp + ArrayUnionTest.cpp BitwiseTest.cpp ComparisonsTest.cpp DateTimeFunctionsTest.cpp From d383628851c730111d76bf5ea226474cb98d9b38 Mon Sep 17 00:00:00 2001 From: Yuan Date: Mon, 25 Sep 2023 09:40:40 +0800 Subject: [PATCH 13/28] Add setup script for cent7 (oap 408) Signed-off-by: Yuan Zhou --- scripts/setup-centos7.sh | 272 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 272 insertions(+) create mode 100755 scripts/setup-centos7.sh diff --git a/scripts/setup-centos7.sh b/scripts/setup-centos7.sh new file mode 100755 index 00000000000..2f1ca1c5000 --- /dev/null +++ b/scripts/setup-centos7.sh @@ -0,0 +1,272 @@ +#!/bin/bash +# Copyright (c) Facebook, Inc. and its affiliates. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -efx -o pipefail +# Some of the packages must be build with the same compiler flags +# so that some low level types are the same size. Also, disable warnings. +SCRIPTDIR=$(dirname "${BASH_SOURCE[0]}") +source $SCRIPTDIR/setup-helper-functions.sh +DEPENDENCY_DIR=${DEPENDENCY_DIR:-/tmp/velox-deps} +CPU_TARGET="${CPU_TARGET:-avx}" +NPROC=$(getconf _NPROCESSORS_ONLN) +export CFLAGS=$(get_cxx_flags $CPU_TARGET) # Used by LZO. +export CXXFLAGS=$CFLAGS # Used by boost. +export CPPFLAGS=$CFLAGS # Used by LZO. +export PKG_CONFIG_PATH=/usr/local/lib64/pkgconfig:/usr/local/lib/pkgconfig:/usr/lib64/pkgconfig:/usr/lib/pkgconfig:$PKG_CONFIG_PATH +FB_OS_VERSION=v2022.11.14.00 + +# shellcheck disable=SC2037 +SUDO="sudo -E" + +function run_and_time { + time "$@" + { echo "+ Finished running $*"; } 2> /dev/null +} + +function dnf_install { + $SUDO dnf install -y -q --setopt=install_weak_deps=False "$@" +} + +function yum_install { + $SUDO yum install -y "$@" +} + +function cmake_install_deps { + cmake -B"$1-build" -GNinja -DCMAKE_CXX_STANDARD=17 \ + -DCMAKE_CXX_FLAGS="${CFLAGS}" -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DCMAKE_BUILD_TYPE=Release -Wno-dev "$@" + ninja -C "$1-build" + $SUDO ninja -C "$1-build" install +} + +function wget_and_untar { + local URL=$1 + local DIR=$2 + mkdir -p "${DIR}" + wget -q --max-redirect 3 -O - "${URL}" | tar -xz -C "${DIR}" --strip-components=1 +} + +function install_cmake { + cd "${DEPENDENCY_DIR}" + wget_and_untar https://cmake.org/files/v3.25/cmake-3.25.1.tar.gz cmake-3 + cd cmake-3 + ./bootstrap --prefix=/usr/local + make -j$(nproc) + $SUDO make install + cmake --version +} + +function install_ninja { + cd "${DEPENDENCY_DIR}" + github_checkout ninja-build/ninja v1.11.1 + ./configure.py --bootstrap + cmake -Bbuild-cmake + cmake --build build-cmake + $SUDO cp ninja /usr/local/bin/ +} + +function install_fmt { + cd "${DEPENDENCY_DIR}" + github_checkout fmtlib/fmt 8.0.0 + cmake_install -DFMT_TEST=OFF +} + +function install_folly { + cd "${DEPENDENCY_DIR}" + github_checkout facebook/folly "${FB_OS_VERSION}" + cmake_install -DBUILD_TESTS=OFF -DFOLLY_HAVE_INT128_T=ON +} + +function install_conda { + cd "${DEPENDENCY_DIR}" + mkdir -p conda && cd conda + wget https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh + MINICONDA_PATH=/opt/miniconda-for-velox + bash Miniconda3-latest-Linux-x86_64.sh -b -u $MINICONDA_PATH +} + +function install_openssl { + cd "${DEPENDENCY_DIR}" + wget_and_untar https://github.com/openssl/openssl/archive/refs/tags/OpenSSL_1_1_1s.tar.gz openssl + cd openssl + ./config no-shared + make depend + make + $SUDO make install +} + +function install_gflags { + cd "${DEPENDENCY_DIR}" + wget_and_untar https://github.com/gflags/gflags/archive/v2.2.2.tar.gz gflags + cd gflags + cmake_install -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=ON -DBUILD_gflags_LIB=ON -DLIB_SUFFIX=64 -DCMAKE_INSTALL_PREFIX:PATH=/usr/local +} + +function install_glog { + cd "${DEPENDENCY_DIR}" + wget_and_untar https://github.com/google/glog/archive/v0.5.0.tar.gz glog + cd glog + cmake_install -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr/local +} + +function install_snappy { + cd "${DEPENDENCY_DIR}" + wget_and_untar https://github.com/google/snappy/archive/1.1.8.tar.gz snappy + cd snappy + cmake_install -DSNAPPY_BUILD_TESTS=OFF +} + +function install_dwarf { + cd "${DEPENDENCY_DIR}" + wget_and_untar https://github.com/davea42/libdwarf-code/archive/refs/tags/20210528.tar.gz dwarf + cd dwarf + #local URL=https://github.com/davea42/libdwarf-code/releases/download/v0.5.0/libdwarf-0.5.0.tar.xz + #local DIR=dwarf + #mkdir -p "${DIR}" + #wget -q --max-redirect 3 "${URL}" + #tar -xf libdwarf-0.5.0.tar.xz -C "${DIR}" + #cd dwarf/libdwarf-0.5.0 + ./configure --enable-shared=no + make + make check + $SUDO make install +} + +function install_re2 { + cd "${DEPENDENCY_DIR}" + wget_and_untar https://github.com/google/re2/archive/refs/tags/2023-03-01.tar.gz re2 + cd re2 + $SUDO make install +} + +function install_flex { + cd "${DEPENDENCY_DIR}" + wget_and_untar https://github.com/westes/flex/releases/download/v2.6.4/flex-2.6.4.tar.gz flex + cd flex + ./autogen.sh + ./configure + $SUDO make install +} + +function install_lzo { + cd "${DEPENDENCY_DIR}" + wget_and_untar http://www.oberhumer.com/opensource/lzo/download/lzo-2.10.tar.gz lzo + cd lzo + ./configure --prefix=/usr/local --enable-shared --disable-static --docdir=/usr/local/share/doc/lzo-2.10 + make "-j$(nproc)" + $SUDO make install +} + +function install_boost { + cd "${DEPENDENCY_DIR}" + wget_and_untar https://boostorg.jfrog.io/artifactory/main/release/1.72.0/source/boost_1_72_0.tar.gz boost + cd boost + ./bootstrap.sh --prefix=/usr/local --with-python=/usr/bin/python3 --with-python-root=/usr/lib/python3.6 --without-libraries=python + $SUDO ./b2 "-j$(nproc)" -d0 install threading=multi +} + +function install_libhdfs3 { + cd "${DEPENDENCY_DIR}" + github_checkout apache/hawq master + cd depends/libhdfs3 + sed -i "/FIND_PACKAGE(GoogleTest REQUIRED)/d" ./CMakeLists.txt + sed -i "s/dumpversion/dumpfullversion/" ./CMake/Platform.cmake + sed -i "s/dfs.domain.socket.path\", \"\"/dfs.domain.socket.path\", \"\/var\/lib\/hadoop-hdfs\/dn_socket\"/g" src/common/SessionConfig.cpp + sed -i "s/pos < endOfCurBlock/pos \< endOfCurBlock \&\& pos \- cursor \<\= 128 \* 1024/g" src/client/InputStreamImpl.cpp + cmake_install +} + +function install_protobuf { + cd "${DEPENDENCY_DIR}" + wget https://github.com/protocolbuffers/protobuf/releases/download/v21.4/protobuf-all-21.4.tar.gz + tar -xzf protobuf-all-21.4.tar.gz + cd protobuf-21.4 + ./configure CXXFLAGS="-fPIC" --prefix=/usr/local + make "-j$(nproc)" + $SUDO make install +} + +function install_awssdk { + cd "${DEPENDENCY_DIR}" + github_checkout aws/aws-sdk-cpp 1.9.379 --depth 1 --recurse-submodules + cmake_install -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS:BOOL=OFF -DMINIMIZE_SIZE:BOOL=ON -DENABLE_TESTING:BOOL=OFF -DBUILD_ONLY:STRING="s3;identity-management" +} + +function install_gtest { + cd "${DEPENDENCY_DIR}" + wget https://github.com/google/googletest/archive/refs/tags/release-1.12.1.tar.gz + tar -xzf release-1.12.1.tar.gz + cd googletest-release-1.12.1 + mkdir -p build && cd build && cmake -DBUILD_GTEST=ON -DBUILD_GMOCK=ON -DINSTALL_GTEST=ON -DINSTALL_GMOCK=ON -DBUILD_SHARED_LIBS=ON .. + make "-j$(nproc)" + $SUDO make install +} + +function install_prerequisites { + run_and_time install_lzo + run_and_time install_boost + run_and_time install_re2 + run_and_time install_flex + run_and_time install_openssl + run_and_time install_gflags + run_and_time install_glog + run_and_time install_snappy + run_and_time install_dwarf +} + +function install_velox_deps { + run_and_time install_fmt + run_and_time install_folly + run_and_time install_conda +} + +$SUDO dnf makecache + +# dnf install dependency libraries +dnf_install epel-release dnf-plugins-core # For ccache, ninja +# PowerTools only works on CentOS8 +# dnf config-manager --set-enabled powertools +dnf_install ccache git wget which libevent-devel \ + openssl-devel libzstd-devel lz4-devel double-conversion-devel \ + curl-devel cmake libxml2-devel libgsasl-devel libuuid-devel patch + +$SUDO dnf remove -y gflags + +# Required for Thrift +dnf_install autoconf automake libtool bison python3 python3-devel + +# Required for build flex +dnf_install gettext-devel texinfo help2man + +# dnf_install conda + +# Activate gcc9; enable errors on unset variables afterwards. +# GCC9 install via yum and devtoolset +# dnf install gcc-toolset-9 only works on CentOS8 + +$SUDO yum makecache +yum_install centos-release-scl +yum_install devtoolset-9 +source /opt/rh/devtoolset-9/enable || exit 1 +gcc --version +set -u + +# Build from source +[ -d "$DEPENDENCY_DIR" ] || mkdir -p "$DEPENDENCY_DIR" + +run_and_time install_cmake +run_and_time install_ninja + +install_prerequisites +install_velox_deps From 2a0635db0b1c1c47c0c60c8b0dac9de9db19588a Mon Sep 17 00:00:00 2001 From: Jia Date: Thu, 23 Nov 2023 00:55:28 +0000 Subject: [PATCH 14/28] Support struct column reading with different schemas (5962) --- velox/connectors/hive/SplitReader.cpp | 15 +++- .../common/SelectiveStructColumnReader.cpp | 4 +- velox/dwio/common/TypeWithId.h | 5 ++ .../parquet/reader/ParquetColumnReader.cpp | 9 ++- .../dwio/parquet/reader/ParquetColumnReader.h | 3 +- velox/dwio/parquet/reader/ParquetReader.cpp | 46 ++++++++++- .../parquet/reader/RepeatedColumnReader.cpp | 23 ++++-- .../parquet/reader/RepeatedColumnReader.h | 6 +- .../parquet/reader/StructColumnReader.cpp | 40 +++++++-- .../dwio/parquet/reader/StructColumnReader.h | 3 +- .../parquet/tests/examples/contacts.parquet | Bin 0 -> 1355 bytes .../tests/reader/ParquetTableScanTest.cpp | 76 ++++++++++++++++++ 12 files changed, 201 insertions(+), 29 deletions(-) create mode 100644 velox/dwio/parquet/tests/examples/contacts.parquet diff --git a/velox/connectors/hive/SplitReader.cpp b/velox/connectors/hive/SplitReader.cpp index 7790343f04b..ab2adf3a673 100644 --- a/velox/connectors/hive/SplitReader.cpp +++ b/velox/connectors/hive/SplitReader.cpp @@ -220,9 +220,18 @@ std::vector SplitReader::adaptColumns( } else { auto fileTypeIdx = fileType->getChildIdxIfExists(fieldName); if (!fileTypeIdx.has_value()) { - // Column is missing. Most likely due to schema evolution. - VELOX_CHECK(tableSchema); - setNullConstantValue(childSpec, tableSchema->findChild(fieldName)); + // If field name exists in the user-specified output type, + // set the column as null constant. + // Related PR: https://github.com/facebookincubator/velox/pull/6427. + auto outputTypeIdx = readerOutputType_->getChildIdxIfExists(fieldName); + if (outputTypeIdx.has_value()) { + setNullConstantValue( + childSpec, readerOutputType_->childAt(outputTypeIdx.value())); + } else { + // Column is missing. Most likely due to schema evolution. + VELOX_CHECK(tableSchema); + setNullConstantValue(childSpec, tableSchema->findChild(fieldName)); + } } else { // Column no longer missing, reset constant value set on the spec. childSpec->setConstantValue(nullptr); diff --git a/velox/dwio/common/SelectiveStructColumnReader.cpp b/velox/dwio/common/SelectiveStructColumnReader.cpp index 30e6e748fc1..face75910c8 100644 --- a/velox/dwio/common/SelectiveStructColumnReader.cpp +++ b/velox/dwio/common/SelectiveStructColumnReader.cpp @@ -133,7 +133,6 @@ void SelectiveStructColumnReaderBase::read( } auto& childSpecs = scanSpec_->children(); - VELOX_CHECK(!childSpecs.empty()); for (size_t i = 0; i < childSpecs.size(); ++i) { auto& childSpec = childSpecs[i]; if (isChildConstant(*childSpec)) { @@ -218,7 +217,7 @@ bool SelectiveStructColumnReaderBase::isChildConstant( fileType_->type()->kind() != TypeKind::MAP && // If this is the case it means this is a flat map, // so it can't have "missing" fields. - childSpec.channel() >= fileType_->size()); + !fileType_->containsChild(childSpec.fieldName())); } namespace { @@ -302,7 +301,6 @@ void setNullField( void SelectiveStructColumnReaderBase::getValues( RowSet rows, VectorPtr* result) { - VELOX_CHECK(!scanSpec_->children().empty()); VELOX_CHECK_NOT_NULL( *result, "SelectiveStructColumnReaderBase expects a non-null result"); VELOX_CHECK( diff --git a/velox/dwio/common/TypeWithId.h b/velox/dwio/common/TypeWithId.h index 953ac87b2b8..96c6cd38fc4 100644 --- a/velox/dwio/common/TypeWithId.h +++ b/velox/dwio/common/TypeWithId.h @@ -59,6 +59,11 @@ class TypeWithId : public velox::Tree> { const std::shared_ptr& childAt(uint32_t idx) const override; + bool containsChild(const std::string& name) const { + VELOX_CHECK_EQ(type_->kind(), velox::TypeKind::ROW); + return type_->as().containsChild(name); + } + const std::shared_ptr& childByName( const std::string& name) const { VELOX_CHECK_EQ(type_->kind(), velox::TypeKind::ROW); diff --git a/velox/dwio/parquet/reader/ParquetColumnReader.cpp b/velox/dwio/parquet/reader/ParquetColumnReader.cpp index ea3169ae727..d63d16bca33 100644 --- a/velox/dwio/parquet/reader/ParquetColumnReader.cpp +++ b/velox/dwio/parquet/reader/ParquetColumnReader.cpp @@ -37,7 +37,8 @@ std::unique_ptr ParquetColumnReader::build( const std::shared_ptr& requestedType, const std::shared_ptr& fileType, ParquetParams& params, - common::ScanSpec& scanSpec) { + common::ScanSpec& scanSpec, + memory::MemoryPool& pool) { auto colName = scanSpec.fieldName(); switch (fileType->type()->kind()) { @@ -58,7 +59,7 @@ std::unique_ptr ParquetColumnReader::build( case TypeKind::ROW: return std::make_unique( - requestedType, fileType, params, scanSpec); + requestedType, fileType, params, scanSpec, pool); case TypeKind::VARBINARY: case TypeKind::VARCHAR: @@ -66,11 +67,11 @@ std::unique_ptr ParquetColumnReader::build( case TypeKind::ARRAY: return std::make_unique( - requestedType, fileType, params, scanSpec); + requestedType, fileType, params, scanSpec, pool); case TypeKind::MAP: return std::make_unique( - requestedType, fileType, params, scanSpec); + requestedType, fileType, params, scanSpec, pool); case TypeKind::BOOLEAN: return std::make_unique( diff --git a/velox/dwio/parquet/reader/ParquetColumnReader.h b/velox/dwio/parquet/reader/ParquetColumnReader.h index 516a500cd22..34a5b258273 100644 --- a/velox/dwio/parquet/reader/ParquetColumnReader.h +++ b/velox/dwio/parquet/reader/ParquetColumnReader.h @@ -45,6 +45,7 @@ class ParquetColumnReader { const std::shared_ptr& requestedType, const std::shared_ptr& fileType, ParquetParams& params, - common::ScanSpec& scanSpec); + common::ScanSpec& scanSpec, + memory::MemoryPool& pool); }; } // namespace facebook::velox::parquet diff --git a/velox/dwio/parquet/reader/ParquetReader.cpp b/velox/dwio/parquet/reader/ParquetReader.cpp index 07471ba15e2..ab5a9998215 100644 --- a/velox/dwio/parquet/reader/ParquetReader.cpp +++ b/velox/dwio/parquet/reader/ParquetReader.cpp @@ -84,6 +84,11 @@ class ReaderBase { /// the data still exists in the buffered inputs. bool isRowGroupBuffered(int32_t rowGroupIndex) const; + static std::shared_ptr createTypeWithId( + const std::shared_ptr& inputType, + const RowTypePtr& rowTypePtr, + bool fileColumnNamesReadAsLowerCase); + private: // Reads and parses file footer. void loadFileMetaData(); @@ -564,6 +569,33 @@ std::shared_ptr ReaderBase::createRowType( std::move(childNames), std::move(childTypes)); } +std::shared_ptr ReaderBase::createTypeWithId( + const std::shared_ptr& inputType, + const RowTypePtr& rowTypePtr, + bool fileColumnNamesReadAsLowerCase) { + if (!fileColumnNamesReadAsLowerCase) { + return inputType; + } + std::vector names; + names.reserve(rowTypePtr->names().size()); + std::vector types = rowTypePtr->children(); + for (const auto& name : rowTypePtr->names()) { + std::string childName = name; + folly::toLowerAscii(childName); + names.emplace_back(childName); + } + auto convertedType = + TypeFactory::create(std::move(names), std::move(types)); + + auto children = inputType->getChildren(); + return std::make_shared( + convertedType, + std::move(children), + inputType->id(), + inputType->maxId(), + inputType->column()); +} + void ReaderBase::scheduleRowGroups( const std::vector& rowGroupIds, int32_t currentGroup, @@ -630,13 +662,19 @@ ParquetRowReader::ParquetRowReader( return; // TODO } ParquetParams params(pool_, columnReaderStats_, readerBase_->fileMetaData()); - auto columnSelector = std::make_shared( - ColumnSelector::apply(options_.getSelector(), readerBase_->schema())); + // ColumnSelector::apply does not work for schema pruning case. + auto columnSelector = options_.getSelector() == nullptr + ? std::make_shared(ColumnSelector(readerBase_->schema())) + : options_.getSelector(); columnReader_ = ParquetColumnReader::build( - columnSelector->getSchemaWithId(), + ReaderBase::createTypeWithId( + columnSelector->getSchemaWithId(), + asRowType(columnSelector->getSchemaWithId()->type()), + readerBase_->isFileColumnNamesReadAsLowerCase()), readerBase_->schemaWithId(), // Id is schema id params, - *options_.getScanSpec()); + *options_.getScanSpec(), + pool_); filterRowGroups(); if (!rowGroupIds_.empty()) { diff --git a/velox/dwio/parquet/reader/RepeatedColumnReader.cpp b/velox/dwio/parquet/reader/RepeatedColumnReader.cpp index 250bd204e08..743bfd1be94 100644 --- a/velox/dwio/parquet/reader/RepeatedColumnReader.cpp +++ b/velox/dwio/parquet/reader/RepeatedColumnReader.cpp @@ -33,6 +33,9 @@ PageReader* FOLLY_NULLABLE readLeafRepDefs( return nullptr; } auto pageReader = reader->formatData().as().reader(); + if (pageReader == nullptr) { + return nullptr; + } pageReader->decodeRepDefs(numTop); return pageReader; } @@ -113,7 +116,8 @@ MapColumnReader::MapColumnReader( const std::shared_ptr& requestedType, const std::shared_ptr& fileType, ParquetParams& params, - common::ScanSpec& scanSpec) + common::ScanSpec& scanSpec, + memory::MemoryPool& pool) : dwio::common::SelectiveMapColumnReader( requestedType, fileType, @@ -123,9 +127,17 @@ MapColumnReader::MapColumnReader( auto& keyChildType = requestedType->childAt(0); auto& elementChildType = requestedType->childAt(1); keyReader_ = ParquetColumnReader::build( - keyChildType, fileType_->childAt(0), params, *scanSpec.children()[0]); + keyChildType, + fileType_->childAt(0), + params, + *scanSpec.children()[0], + pool); elementReader_ = ParquetColumnReader::build( - elementChildType, fileType_->childAt(1), params, *scanSpec.children()[1]); + elementChildType, + fileType_->childAt(1), + params, + *scanSpec.children()[1], + pool); reinterpret_cast(fileType.get()) ->makeLevelInfo(levelInfo_); children_ = {keyReader_.get(), elementReader_.get()}; @@ -223,7 +235,8 @@ ListColumnReader::ListColumnReader( const std::shared_ptr& requestedType, const std::shared_ptr& fileType, ParquetParams& params, - common::ScanSpec& scanSpec) + common::ScanSpec& scanSpec, + memory::MemoryPool& pool) : dwio::common::SelectiveListColumnReader( requestedType, fileType, @@ -231,7 +244,7 @@ ListColumnReader::ListColumnReader( scanSpec) { auto& childType = requestedType->childAt(0); child_ = ParquetColumnReader::build( - childType, fileType_->childAt(0), params, *scanSpec.children()[0]); + childType, fileType_->childAt(0), params, *scanSpec.children()[0], pool); reinterpret_cast(fileType.get()) ->makeLevelInfo(levelInfo_); children_ = {child_.get()}; diff --git a/velox/dwio/parquet/reader/RepeatedColumnReader.h b/velox/dwio/parquet/reader/RepeatedColumnReader.h index 3155e8d6647..d6c68d2239a 100644 --- a/velox/dwio/parquet/reader/RepeatedColumnReader.h +++ b/velox/dwio/parquet/reader/RepeatedColumnReader.h @@ -59,7 +59,8 @@ class MapColumnReader : public dwio::common::SelectiveMapColumnReader { const std::shared_ptr& requestedType, const std::shared_ptr& fileType, ParquetParams& params, - common::ScanSpec& scanSpec); + common::ScanSpec& scanSpec, + memory::MemoryPool& pool); void prepareRead( vector_size_t offset, @@ -115,7 +116,8 @@ class ListColumnReader : public dwio::common::SelectiveListColumnReader { const std::shared_ptr& requestedType, const std::shared_ptr& fileType, ParquetParams& params, - common::ScanSpec& scanSpec); + common::ScanSpec& scanSpec, + memory::MemoryPool& pool); void prepareRead( vector_size_t offset, diff --git a/velox/dwio/parquet/reader/StructColumnReader.cpp b/velox/dwio/parquet/reader/StructColumnReader.cpp index eca887eab15..af8000046e7 100644 --- a/velox/dwio/parquet/reader/StructColumnReader.cpp +++ b/velox/dwio/parquet/reader/StructColumnReader.cpp @@ -30,21 +30,46 @@ StructColumnReader::StructColumnReader( const std::shared_ptr& requestedType, const std::shared_ptr& fileType, ParquetParams& params, - common::ScanSpec& scanSpec) + common::ScanSpec& scanSpec, + memory::MemoryPool& pool) : SelectiveStructColumnReader(requestedType, fileType, params, scanSpec) { auto& childSpecs = scanSpec_->stableChildren(); + std::vector missingFields; for (auto i = 0; i < childSpecs.size(); ++i) { auto childSpec = childSpecs[i]; if (childSpecs[i]->isConstant()) { continue; } - auto childFileType = fileType_->childByName(childSpec->fieldName()); - auto childRequestedType = - requestedType_->childByName(childSpec->fieldName()); + const auto& fieldName = childSpec->fieldName(); + if (!fileType_->containsChild(fieldName)) { + missingFields.emplace_back(i); + continue; + } + auto childFileType = fileType_->childByName(fieldName); + auto childRequestedType = requestedType_->childByName(fieldName); addChild(ParquetColumnReader::build( - childRequestedType, childFileType, params, *childSpec)); + childRequestedType, childFileType, params, *childSpec, pool)); childSpecs[i]->setSubscript(children_.size() - 1); } + + if (missingFields.size() > 0) { + // Set the struct as null if all the children fields in the output type are + // missing and the number of child fields is more than one. + if (childSpecs.size() > 1 && missingFields.size() == childSpecs.size()) { + scanSpec_->setConstantValue( + BaseVector::createNullConstant(requestedType_->type(), 1, &pool)); + } else { + // Set null constant for the missing child field of output type. + for (int channel : missingFields) { + childSpecs[channel]->setConstantValue(BaseVector::createNullConstant( + requestedType_->childByName(childSpecs[channel]->fieldName()) + ->type(), + 1, + &pool)); + } + } + } + auto type = reinterpret_cast(fileType_.get()); if (type->parent()) { levelMode_ = reinterpret_cast(fileType_.get()) @@ -54,7 +79,10 @@ StructColumnReader::StructColumnReader( // this and the child. auto child = childForRepDefs_; for (;;) { - assert(child); + if (child == nullptr) { + levelMode_ = LevelMode::kNulls; + break; + } if (child->fileType().type()->kind() == TypeKind::ARRAY || child->fileType().type()->kind() == TypeKind::MAP) { levelMode_ = LevelMode::kStructOverLists; diff --git a/velox/dwio/parquet/reader/StructColumnReader.h b/velox/dwio/parquet/reader/StructColumnReader.h index f38c9e849c7..f03d5549387 100644 --- a/velox/dwio/parquet/reader/StructColumnReader.h +++ b/velox/dwio/parquet/reader/StructColumnReader.h @@ -35,7 +35,8 @@ class StructColumnReader : public dwio::common::SelectiveStructColumnReader { const std::shared_ptr& requestedType, const std::shared_ptr& fileType, ParquetParams& params, - common::ScanSpec& scanSpec); + common::ScanSpec& scanSpec, + memory::MemoryPool& pool); void read(vector_size_t offset, RowSet rows, const uint64_t* incomingNulls) override; diff --git a/velox/dwio/parquet/tests/examples/contacts.parquet b/velox/dwio/parquet/tests/examples/contacts.parquet new file mode 100644 index 0000000000000000000000000000000000000000..fa3751f8dc46282db3ad2faabe5e0b69dfdba1f1 GIT binary patch literal 1355 zcmb7^-)qxQ6vuCG(;A^Lh22X6eF(vp4Qg1EwcScz96rdHGWMv5yGw3sFipE8oygdK zF~q;YCx3u{fRB4sWQzE>2OoSA+0*dc^tS6-CJsVw&X4=~o}6=&%{yCFf{02Az5|>LLNh+ zP3q*&o0p%ZTuRipg
  • 2lraNVcci^fE#xse~;@ov92j1EEj@x3Wjj!Qt{DxAt1y= zAQe5|F*i-urah8Uq$+8G4$_Nsjcm!M!_4^cBttymh5X=NA?~y2*^}6P%Dar@!C68Y z_B~X?ni8@uCki5k64r8UKZ=tQ+9-;x!lMN2qT?F#yokrKOrlsX>KkRzExn{d697vv z#k5YFWwkYI&G1gqLfWXI1(id$p%jGZ2ZhYjBTOt*UMrGAB$D`Qq zg<8(BY|BR4;!H-o8(45Mb_YDMqTX|>%M<3YglRwVzL~t}bF*p2Ni=X1vtqV=9(b|Y ze6(+hBLgAd1Gh}`!-RKu1debJ1Z*dOgfR?ku@0 tORJ8vQs-{Xb{)6P>aMfA)8K8#v8#@4yQ|BKy7+R(7dnGK;U@e>{{rEFM9lyI literal 0 HcmV?d00001 diff --git a/velox/dwio/parquet/tests/reader/ParquetTableScanTest.cpp b/velox/dwio/parquet/tests/reader/ParquetTableScanTest.cpp index 5cb7b759259..9b40f2b41c5 100644 --- a/velox/dwio/parquet/tests/reader/ParquetTableScanTest.cpp +++ b/velox/dwio/parquet/tests/reader/ParquetTableScanTest.cpp @@ -443,6 +443,82 @@ TEST_F(ParquetTableScanTest, readAsLowerCase) { result.second, {makeRowVector({"a"}, {makeFlatVector({0, 1})})}); } +TEST_F(ParquetTableScanTest, structSelection) { + auto vector = makeArrayVector({{}}); + loadData( + getExampleFilePath("contacts.parquet"), + ROW({"name"}, {ROW({"first", "last"}, {VARCHAR(), VARCHAR()})}), + makeRowVector( + {"t"}, + { + vector, + })); + assertSelectWithFilter({"name"}, {}, "", "SELECT ('Janet', 'Jones')"); + + loadData( + getExampleFilePath("contacts.parquet"), + ROW({"name"}, + {ROW( + {"first", "middle", "last"}, {VARCHAR(), VARCHAR(), VARCHAR()})}), + makeRowVector( + {"t"}, + { + vector, + })); + assertSelectWithFilter({"name"}, {}, "", "SELECT ('Janet', null, 'Jones')"); + + loadData( + getExampleFilePath("contacts.parquet"), + ROW({"name"}, {ROW({"first", "middle"}, {VARCHAR(), VARCHAR()})}), + makeRowVector( + {"t"}, + { + vector, + })); + assertSelectWithFilter({"name"}, {}, "", "SELECT ('Janet', null)"); + + loadData( + getExampleFilePath("contacts.parquet"), + ROW({"name"}, {ROW({"middle", "last"}, {VARCHAR(), VARCHAR()})}), + makeRowVector( + {"t"}, + { + vector, + })); + assertSelectWithFilter({"name"}, {}, "", "SELECT (null, 'Jones')"); + + loadData( + getExampleFilePath("contacts.parquet"), + ROW({"name"}, {ROW({"middle"}, {VARCHAR()})}), + makeRowVector( + {"t"}, + { + vector, + })); + assertSelectWithFilter({"name"}, {}, "", "SELECT row(null)"); + + loadData( + getExampleFilePath("contacts.parquet"), + ROW({"name"}, {ROW({"middle", "info"}, {VARCHAR(), VARCHAR()})}), + makeRowVector( + {"t"}, + { + vector, + })); + assertSelectWithFilter({"name"}, {}, "", "SELECT NULL"); + + loadData( + getExampleFilePath("contacts.parquet"), + ROW({"name"}, {ROW({}, {})}), + makeRowVector( + {"t"}, + { + vector, + })); + + assertSelectWithFilter({"name"}, {}, "", "SELECT t from tmp"); +} + int main(int argc, char** argv) { testing::InitGoogleTest(&argc, argv); folly::init(&argc, &argv, false); From d5b5e827610347bb49f7d0fcb1f5d5e6145a7088 Mon Sep 17 00:00:00 2001 From: rui-mo Date: Wed, 18 Oct 2023 14:13:47 +0800 Subject: [PATCH 15/28] Add Spark atan2 function (7113) --- velox/docs/functions/spark/math.rst | 4 ++++ velox/functions/sparksql/Arithmetic.h | 9 +++++++++ velox/functions/sparksql/RegisterArithmetic.cpp | 1 + velox/functions/sparksql/tests/ArithmeticTest.cpp | 9 ++++++++- 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/velox/docs/functions/spark/math.rst b/velox/docs/functions/spark/math.rst index 46be649696c..fc7f0386035 100644 --- a/velox/docs/functions/spark/math.rst +++ b/velox/docs/functions/spark/math.rst @@ -18,6 +18,10 @@ Mathematical Functions Returns inverse hyperbolic sine of ``x``. +.. spark:function:: atan2(x, y) -> double + + Returns the angle in radians between the positive x-axis of a plane and the point given by the coordinates(x, y). + .. spark:function:: atanh(x) -> double Returns inverse hyperbolic tangent of ``x``. diff --git a/velox/functions/sparksql/Arithmetic.h b/velox/functions/sparksql/Arithmetic.h index fa9e101913d..4523a89c22c 100644 --- a/velox/functions/sparksql/Arithmetic.h +++ b/velox/functions/sparksql/Arithmetic.h @@ -280,4 +280,13 @@ struct Log10Function { return true; } }; + +template +struct Atan2Function { + template + FOLLY_ALWAYS_INLINE void call(TInput& result, TInput y, TInput x) { + result = std::atan2(y + 0.0, x + 0.0); + } +}; + } // namespace facebook::velox::functions::sparksql diff --git a/velox/functions/sparksql/RegisterArithmetic.cpp b/velox/functions/sparksql/RegisterArithmetic.cpp index 08851f9e0d9..cdddf87bd86 100644 --- a/velox/functions/sparksql/RegisterArithmetic.cpp +++ b/velox/functions/sparksql/RegisterArithmetic.cpp @@ -95,6 +95,7 @@ void registerArithmeticFunctions(const std::string& prefix) { VELOX_REGISTER_VECTOR_FUNCTION(udf_decimal_sub, prefix + "subtract"); VELOX_REGISTER_VECTOR_FUNCTION(udf_decimal_mul, prefix + "multiply"); VELOX_REGISTER_VECTOR_FUNCTION(udf_decimal_div, prefix + "divide"); + registerFunction({prefix + "atan2"}); } } // namespace facebook::velox::functions::sparksql diff --git a/velox/functions/sparksql/tests/ArithmeticTest.cpp b/velox/functions/sparksql/tests/ArithmeticTest.cpp index 9aa9a4d2faa..602dcb8e7fa 100644 --- a/velox/functions/sparksql/tests/ArithmeticTest.cpp +++ b/velox/functions/sparksql/tests/ArithmeticTest.cpp @@ -379,6 +379,14 @@ TEST_F(ArithmeticTest, cot) { EXPECT_EQ(cot(0), 1 / std::tan(0)); } +TEST_F(ArithmeticTest, atan2) { + const auto atan2 = [&](std::optional y, std::optional x) { + return evaluateOnce("atan2(c0, c1)", y, x); + }; + + EXPECT_EQ(atan2(0, 0), 0.0); +} + class LogNTest : public SparkFunctionBaseTest { protected: static constexpr float kInf = std::numeric_limits::infinity(); @@ -403,6 +411,5 @@ TEST_F(LogNTest, log10) { EXPECT_EQ(log10(-1.0), std::nullopt); EXPECT_EQ(log10(kInf), kInf); } - } // namespace } // namespace facebook::velox::functions::sparksql::test From 911bebbdd115ba3c1a116e95bf1861d1361c4ac6 Mon Sep 17 00:00:00 2001 From: Jia Date: Thu, 23 Nov 2023 00:58:13 +0000 Subject: [PATCH 16/28] Support timestamp reader for Parquet file format (4680) --- velox/connectors/hive/HiveDataSource.cpp | 13 ++- velox/dwio/common/SelectiveColumnReader.cpp | 3 + velox/dwio/parquet/reader/PageReader.cpp | 48 +++++++- .../parquet/reader/ParquetColumnReader.cpp | 5 + velox/dwio/parquet/reader/ParquetReader.cpp | 2 +- .../parquet/reader/TimestampColumnReader.h | 49 ++++++++ .../tests/examples/timestamp_int96.parquet | Bin 0 -> 560 bytes .../tests/reader/ParquetTableScanTest.cpp | 106 ++++++++++++++++++ velox/exec/tests/utils/PlanBuilder.cpp | 6 +- velox/exec/tests/utils/PlanBuilder.h | 9 +- velox/type/Type.h | 5 + 11 files changed, 236 insertions(+), 10 deletions(-) create mode 100644 velox/dwio/parquet/reader/TimestampColumnReader.h create mode 100644 velox/dwio/parquet/tests/examples/timestamp_int96.parquet diff --git a/velox/connectors/hive/HiveDataSource.cpp b/velox/connectors/hive/HiveDataSource.cpp index 448f673ac65..2260795a7fc 100644 --- a/velox/connectors/hive/HiveDataSource.cpp +++ b/velox/connectors/hive/HiveDataSource.cpp @@ -420,11 +420,14 @@ HiveDataSource::HiveDataSource( for (auto& [k, v] : hiveTableHandle_->subfieldFilters()) { filters.emplace(k.clone(), v->clone()); } - auto remainingFilter = extractFiltersFromRemainingFilter( - hiveTableHandle_->remainingFilter(), - expressionEvaluator_, - false, - filters); + auto remainingFilter = hiveTableHandle_->remainingFilter(); + if (hiveTableHandle_->isFilterPushdownEnabled()) { + remainingFilter = extractFiltersFromRemainingFilter( + hiveTableHandle_->remainingFilter(), + expressionEvaluator_, + false, + filters); + } std::vector remainingFilterSubfields; if (remainingFilter) { diff --git a/velox/dwio/common/SelectiveColumnReader.cpp b/velox/dwio/common/SelectiveColumnReader.cpp index f2c157ff9c7..35d7078e4bb 100644 --- a/velox/dwio/common/SelectiveColumnReader.cpp +++ b/velox/dwio/common/SelectiveColumnReader.cpp @@ -214,6 +214,9 @@ void SelectiveColumnReader::getIntValues( VELOX_FAIL("Unsupported value size: {}", valueSize_); } break; + case TypeKind::TIMESTAMP: + getFlatValues(rows, result, requestedType); + break; default: VELOX_FAIL( "Not a valid type for integer reader: {}", requestedType->toString()); diff --git a/velox/dwio/parquet/reader/PageReader.cpp b/velox/dwio/parquet/reader/PageReader.cpp index 042150f37a0..1ab243ec1ae 100644 --- a/velox/dwio/parquet/reader/PageReader.cpp +++ b/velox/dwio/parquet/reader/PageReader.cpp @@ -396,6 +396,51 @@ void PageReader::prepareDictionary(const PageHeader& pageHeader) { } break; } + case thrift::Type::INT96: { + auto numVeloxBytes = dictionary_.numValues * sizeof(Timestamp); + dictionary_.values = AlignedBuffer::allocate(numVeloxBytes, &pool_); + auto numBytes = dictionary_.numValues * sizeof(Int96Timestamp); + if (pageData_) { + memcpy(dictionary_.values->asMutable(), pageData_, numBytes); + } else { + dwio::common::readBytes( + numBytes, + inputStream_.get(), + dictionary_.values->asMutable(), + bufferStart_, + bufferEnd_); + } + // Expand the Parquet type length values to Velox type length. + // We start from the end to allow in-place expansion. + auto values = dictionary_.values->asMutable(); + auto parquetValues = dictionary_.values->asMutable(); + static constexpr int64_t kJulianToUnixEpochDays = 2440588LL; + static constexpr int64_t kSecondsPerDay = 86400LL; + static constexpr int64_t kNanosPerSecond = + Timestamp::kNanosecondsInMillisecond * + Timestamp::kMillisecondsInSecond; + for (auto i = dictionary_.numValues - 1; i >= 0; --i) { + // Convert the timestamp into seconds and nanos since the Unix epoch, + // 00:00:00.000000 on 1 January 1970. + uint64_t nanos; + memcpy( + &nanos, + parquetValues + i * sizeof(Int96Timestamp), + sizeof(uint64_t)); + int32_t days; + memcpy( + &days, + parquetValues + i * sizeof(Int96Timestamp) + sizeof(uint64_t), + sizeof(int32_t)); + int64_t seconds = (days - kJulianToUnixEpochDays) * kSecondsPerDay; + if (nanos > Timestamp::kMaxNanos) { + seconds += nanos / kNanosPerSecond; + nanos -= (nanos / kNanosPerSecond) * kNanosPerSecond; + } + values[i] = Timestamp(seconds, nanos); + } + break; + } case thrift::Type::BYTE_ARRAY: { dictionary_.values = AlignedBuffer::allocate(dictionary_.numValues, &pool_); @@ -486,7 +531,6 @@ void PageReader::prepareDictionary(const PageHeader& pageHeader) { VELOX_UNSUPPORTED( "Parquet type {} not supported for dictionary", parquetType); } - case thrift::Type::INT96: default: VELOX_UNSUPPORTED( "Parquet type {} not supported for dictionary", parquetType); @@ -513,6 +557,8 @@ int32_t parquetTypeBytes(thrift::Type::type type) { case thrift::Type::INT64: case thrift::Type::DOUBLE: return 8; + case thrift::Type::INT96: + return 12; default: VELOX_FAIL("Type does not have a byte width {}", type); } diff --git a/velox/dwio/parquet/reader/ParquetColumnReader.cpp b/velox/dwio/parquet/reader/ParquetColumnReader.cpp index d63d16bca33..8f5df722873 100644 --- a/velox/dwio/parquet/reader/ParquetColumnReader.cpp +++ b/velox/dwio/parquet/reader/ParquetColumnReader.cpp @@ -28,6 +28,7 @@ #include "velox/dwio/parquet/reader/Statistics.h" #include "velox/dwio/parquet/reader/StringColumnReader.h" #include "velox/dwio/parquet/reader/StructColumnReader.h" +#include "velox/dwio/parquet/reader/TimestampColumnReader.h" #include "velox/dwio/parquet/thrift/ParquetThriftTypes.h" namespace facebook::velox::parquet { @@ -77,6 +78,10 @@ std::unique_ptr ParquetColumnReader::build( return std::make_unique( requestedType, fileType, params, scanSpec); + case TypeKind::TIMESTAMP: + return std::make_unique( + requestedType, fileType, params, scanSpec); + default: VELOX_FAIL( "buildReader unhandled type: " + diff --git a/velox/dwio/parquet/reader/ParquetReader.cpp b/velox/dwio/parquet/reader/ParquetReader.cpp index ab5a9998215..44b884366f5 100644 --- a/velox/dwio/parquet/reader/ParquetReader.cpp +++ b/velox/dwio/parquet/reader/ParquetReader.cpp @@ -532,7 +532,7 @@ TypePtr ReaderBase::convertType( case thrift::Type::type::INT64: return BIGINT(); case thrift::Type::type::INT96: - return DOUBLE(); // TODO: Lose precision + return TIMESTAMP(); case thrift::Type::type::FLOAT: return REAL(); case thrift::Type::type::DOUBLE: diff --git a/velox/dwio/parquet/reader/TimestampColumnReader.h b/velox/dwio/parquet/reader/TimestampColumnReader.h new file mode 100644 index 00000000000..4c534b4bfce --- /dev/null +++ b/velox/dwio/parquet/reader/TimestampColumnReader.h @@ -0,0 +1,49 @@ +/* + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include "velox/dwio/parquet/reader/IntegerColumnReader.h" +#include "velox/dwio/parquet/reader/ParquetColumnReader.h" + +namespace facebook::velox::parquet { + +class TimestampColumnReader : public IntegerColumnReader { + public: + TimestampColumnReader( + const std::shared_ptr& requestedType, + std::shared_ptr fileType, + ParquetParams& params, + common::ScanSpec& scanSpec) + : IntegerColumnReader(requestedType, fileType, params, scanSpec) {} + + bool hasBulkPath() const override { + return false; + } + + void read( + vector_size_t offset, + RowSet rows, + const uint64_t* /*incomingNulls*/) override { + auto& data = formatData_->as(); + // Use int128_t as a workaroud. Timestamp in Velox is of 16-byte length. + prepareRead(offset, rows, nullptr); + readCommon(rows); + readOffset_ += rows.back() + 1; + } +}; + +} // namespace facebook::velox::parquet diff --git a/velox/dwio/parquet/tests/examples/timestamp_int96.parquet b/velox/dwio/parquet/tests/examples/timestamp_int96.parquet new file mode 100644 index 0000000000000000000000000000000000000000..ea3a125aab6062da3e978a20eae207b0755b169c GIT binary patch literal 560 zcmZWnL1@!Z8286zK zGzJ6lC}Rgf@Zd!~3Z6WB^yp0#EF!p_^q}aU(VaZKkMIBf@B4V~tt_sVsYs6nIuz*N zlPB+md8$wm;DHVA&OIwE0Nma!!UY-Xf_!}K=F-WlF@RxE1W;anUbqPGmLvYQUic&f zTslD9e*WXC0Pyt&V)^HtH#WfHCgMkr*Zvd$4uy$2>3qyB1JpB{UrWEAo(FiH6oCRY z=>*+>vG+kTvKmyPd4TbG0vIQVxpDW__90^3P>%>*RS{C6+t=l!%i{a%iNDDsMhQ@< zNHie@RnZOIC>;@lO8ShH7ja!E8L%Pea~;PitkM~s=VcSZrO8P Te{OnGHw?Vy4SdlR{44(eD@c)t literal 0 HcmV?d00001 diff --git a/velox/dwio/parquet/tests/reader/ParquetTableScanTest.cpp b/velox/dwio/parquet/tests/reader/ParquetTableScanTest.cpp index 9b40f2b41c5..9ccccff4d88 100644 --- a/velox/dwio/parquet/tests/reader/ParquetTableScanTest.cpp +++ b/velox/dwio/parquet/tests/reader/ParquetTableScanTest.cpp @@ -73,6 +73,34 @@ class ParquetTableScanTest : public HiveConnectorTestBase { assertQuery(plan, splits_, sql); } + void assertSelectWithFilter( + std::vector&& outputColumnNames, + const std::vector& subfieldFilters, + const std::string& remainingFilter, + const std::string& sql, + bool isFilterPushdownEnabled) { + auto rowType = getRowType(std::move(outputColumnNames)); + parse::ParseOptions options; + options.parseDecimalAsDouble = false; + + auto plan = PlanBuilder(pool_.get()) + .setParseOptions(options) + // Function extractFiltersFromRemainingFilter will extract + // filters to subfield filters, but for some types, filter + // pushdown is not supported. + .tableScan( + "hive_table", + rowType, + {}, + subfieldFilters, + remainingFilter, + nullptr, + isFilterPushdownEnabled) + .planNode(); + + assertQuery(plan, splits_, sql); + } + void assertSelectWithAgg( std::vector&& outputColumnNames, const std::vector& aggregates, @@ -519,6 +547,84 @@ TEST_F(ParquetTableScanTest, structSelection) { assertSelectWithFilter({"name"}, {}, "", "SELECT t from tmp"); } +TEST_F(ParquetTableScanTest, timestampFilter) { + // Timestamp-int96.parquet holds one column (t: TIMESTAMP) and + // 10 rows in one row group. Data is in SNAPPY compressed format. + // The values are: + // |t | + // +-------------------+ + // |2015-06-01 19:34:56| + // |2015-06-02 19:34:56| + // |2001-02-03 03:34:06| + // |1998-03-01 08:01:06| + // |2022-12-23 03:56:01| + // |1980-01-24 00:23:07| + // |1999-12-08 13:39:26| + // |2023-04-21 09:09:34| + // |2000-09-12 22:36:29| + // |2007-12-12 04:27:56| + // +-------------------+ + auto vector = makeFlatVector( + {Timestamp(1433116800, 70496000000000), + Timestamp(1433203200, 70496000000000), + Timestamp(981158400, 12846000000000), + Timestamp(888710400, 28866000000000), + Timestamp(1671753600, 14161000000000), + Timestamp(317520000, 1387000000000), + Timestamp(944611200, 49166000000000), + Timestamp(1682035200, 32974000000000), + Timestamp(968716800, 81389000000000), + Timestamp(1197417600, 16076000000000)}); + + loadData( + getExampleFilePath("timestamp_int96.parquet"), + ROW({"t"}, {TIMESTAMP()}), + makeRowVector( + {"t"}, + { + vector, + })); + + assertSelectWithFilter({"t"}, {}, "", "SELECT t from tmp", false); + assertSelectWithFilter( + {"t"}, + {}, + "t < TIMESTAMP '2000-09-12 22:36:29'", + "SELECT t from tmp where t < TIMESTAMP '2000-09-12 22:36:29'", + false); + assertSelectWithFilter( + {"t"}, + {}, + "t <= TIMESTAMP '2000-09-12 22:36:29'", + "SELECT t from tmp where t <= TIMESTAMP '2000-09-12 22:36:29'", + false); + assertSelectWithFilter( + {"t"}, + {}, + "t > TIMESTAMP '1980-01-24 00:23:07'", + "SELECT t from tmp where t > TIMESTAMP '1980-01-24 00:23:07'", + false); + assertSelectWithFilter( + {"t"}, + {}, + "t >= TIMESTAMP '1980-01-24 00:23:07'", + "SELECT t from tmp where t >= TIMESTAMP '1980-01-24 00:23:07'", + false); + assertSelectWithFilter( + {"t"}, + {}, + "t == TIMESTAMP '2022-12-23 03:56:01'", + "SELECT t from tmp where t == TIMESTAMP '2022-12-23 03:56:01'", + false); + VELOX_ASSERT_THROW( + assertSelectWithFilter( + {"t"}, + {"t < TIMESTAMP '2000-09-12 22:36:29'"}, + "", + "SELECT t from tmp where t < TIMESTAMP '2000-09-12 22:36:29'"), + "testInt128() is not supported"); +} + int main(int argc, char** argv) { testing::InitGoogleTest(&argc, argv); folly::init(&argc, &argv, false); diff --git a/velox/exec/tests/utils/PlanBuilder.cpp b/velox/exec/tests/utils/PlanBuilder.cpp index f086991debd..ab7f72a4437 100644 --- a/velox/exec/tests/utils/PlanBuilder.cpp +++ b/velox/exec/tests/utils/PlanBuilder.cpp @@ -97,12 +97,14 @@ PlanBuilder& PlanBuilder::tableScan( const std::unordered_map& columnAliases, const std::vector& subfieldFilters, const std::string& remainingFilter, - const RowTypePtr& dataColumns) { + const RowTypePtr& dataColumns, + const bool isFilterPushdownEnabled) { return TableScanBuilder(*this) .tableName(tableName) .outputType(outputType) .columnAliases(columnAliases) .subfieldFilters(subfieldFilters) + .isFilterPushdownEnabled(isFilterPushdownEnabled) .remainingFilter(remainingFilter) .dataColumns(dataColumns) .endTableScan(); @@ -200,7 +202,7 @@ core::PlanNodePtr PlanBuilder::TableScanBuilder::build(core::PlanNodeId id) { tableHandle_ = std::make_shared( connectorId_, tableName_, - true, + isFilterPushdownEnabled_, std::move(filters), remainingFilterExpr, dataColumns_); diff --git a/velox/exec/tests/utils/PlanBuilder.h b/velox/exec/tests/utils/PlanBuilder.h index e311c0d441d..29917eac876 100644 --- a/velox/exec/tests/utils/PlanBuilder.h +++ b/velox/exec/tests/utils/PlanBuilder.h @@ -144,7 +144,8 @@ class PlanBuilder { const std::unordered_map& columnAliases = {}, const std::vector& subfieldFilters = {}, const std::string& remainingFilter = "", - const RowTypePtr& dataColumns = nullptr); + const RowTypePtr& dataColumns = nullptr, + bool isFilterPushdownEnabled = true); /// Add a TableScanNode to scan a TPC-H table. /// @@ -209,6 +210,11 @@ class PlanBuilder { return *this; } + TableScanBuilder& isFilterPushdownEnabled(bool isFilterPushdownEnabled) { + isFilterPushdownEnabled_ = std::move(isFilterPushdownEnabled); + return *this; + } + /// @param dataColumns can be different from 'outputType' for the purposes /// of testing queries using missing columns. It is used, if specified, for /// parseExpr call and as 'dataColumns' for the TableHandle. You supply more @@ -269,6 +275,7 @@ class PlanBuilder { std::shared_ptr tableHandle_; std::unordered_map> assignments_; + bool isFilterPushdownEnabled_; }; /// Start a TableScanBuilder. diff --git a/velox/type/Type.h b/velox/type/Type.h index 3c7113fc121..02bd70891bc 100644 --- a/velox/type/Type.h +++ b/velox/type/Type.h @@ -45,6 +45,11 @@ namespace facebook::velox { using int128_t = __int128_t; +struct __attribute__((__packed__)) Int96Timestamp { + int32_t days; + uint64_t nanos; +}; + /// Velox type system supports a small set of SQL-compatible composeable types: /// BOOLEAN, TINYINT, SMALLINT, INTEGER, BIGINT, HUGEINT, REAL, DOUBLE, VARCHAR, /// VARBINARY, TIMESTAMP, ARRAY, MAP, ROW From e1d5593eec41da6084fb40aa88815178cea46656 Mon Sep 17 00:00:00 2001 From: Jia Date: Wed, 22 Nov 2023 02:39:40 +0000 Subject: [PATCH 17/28] Add config for registration (7110) --- .../lib/aggregates/BitwiseAggregateBase.h | 9 ++++- .../aggregates/BitwiseAggregates.cpp | 11 ++++-- .../prestosql/aggregates/CountAggregate.cpp | 9 ++++- .../aggregates/CovarianceAggregates.cpp | 28 ++++++++++---- .../prestosql/aggregates/MinMaxAggregates.cpp | 18 ++++++--- .../aggregates/RegisterAggregateFunctions.cpp | 38 +++++++++++++------ .../aggregates/RegisterAggregateFunctions.h | 3 +- .../aggregates/VarianceAggregates.cpp | 32 +++++++++++----- .../sparksql/aggregates/AverageAggregate.cpp | 6 ++- .../sparksql/aggregates/AverageAggregate.h | 3 +- .../aggregates/BitwiseXorAggregate.cpp | 6 ++- .../sparksql/aggregates/BitwiseXorAggregate.h | 4 +- .../aggregates/FirstLastAggregate.cpp | 26 +++++++++---- .../sparksql/aggregates/MinMaxByAggregate.cpp | 20 +++++++--- .../sparksql/aggregates/Register.cpp | 24 +++++++----- .../functions/sparksql/aggregates/Register.h | 3 +- .../sparksql/aggregates/SumAggregate.cpp | 9 ++++- .../sparksql/aggregates/SumAggregate.h | 5 ++- 18 files changed, 182 insertions(+), 72 deletions(-) diff --git a/velox/functions/lib/aggregates/BitwiseAggregateBase.h b/velox/functions/lib/aggregates/BitwiseAggregateBase.h index 428b905ea83..02f21c02ac8 100644 --- a/velox/functions/lib/aggregates/BitwiseAggregateBase.h +++ b/velox/functions/lib/aggregates/BitwiseAggregateBase.h @@ -70,7 +70,10 @@ class BitwiseAggregateBase : public SimpleNumericAggregate { }; template