Skip to content

Commit

Permalink
fix comments
Browse files Browse the repository at this point in the history
  • Loading branch information
liujiayi771 committed Feb 19, 2024
1 parent 0b40e54 commit 7a01252
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 19 deletions.
36 changes: 18 additions & 18 deletions velox/functions/sparksql/aggregates/DecimalSumAggregate.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ class DecimalSumAggregate {

using OutputType = TSumType;

// Spark's decimal sum doesn't have the concept of a null group, each group is
// initialized with an initial value, where sum = 0 and isEmpty = true. The
// final agg may fallback to being executed in Spark, so the meaning of the
// intermediate data should be consistent with Spark. Therefore, we need to
// use the parameter nonNullGroup in writeIntermediateResult to output a null
// group as sum = 0, isEmpty = true. nonNullGroup is only available when
// default-null behavior is disabled.
/// Spark's decimal sum doesn't have the concept of a null group, each group
/// is initialized with an initial value, where sum = 0 and isEmpty = true.
/// The final agg may fallback to being executed in Spark, so the meaning of
/// the intermediate data should be consistent with Spark. Therefore, we need
/// to use the parameter nonNullGroup in writeIntermediateResult to output a
/// null group as sum = 0, isEmpty = true. nonNullGroup is only available when
/// default-null behavior is disabled.
static constexpr bool default_null_behavior_ = false;

static bool toIntermediate(
Expand All @@ -54,13 +54,13 @@ class DecimalSumAggregate {
return true;
}

// This struct stores the sum of input values, overflow during accumulation,
// and a bool value isEmpty used to indicate whether all inputs are null. The
// initial value of sum is 0. We need to keep sum unchanged if the input is
// null, as sum function ignores null input. If the isEmpty is true, then it
// means there were no values to begin with or all the values were null, so
// the result will be null. If the isEmpty is false, then if sum is nullopt
// that means an overflow has happened, it returns null.
/// This struct stores the sum of input values, overflow during accumulation,
/// and a bool value isEmpty used to indicate whether all inputs are null. The
/// initial value of sum is 0. We need to keep sum unchanged if the input is
/// null, as sum function ignores null input. If the isEmpty is true, then it
/// means there were no values to begin with or all the values were null, so
/// the result will be null. If the isEmpty is false, then if sum is nullopt
/// that means an overflow has happened, it returns null.
struct AccumulatorType {
std::optional<int128_t> sum{0};
int64_t overflow{0};
Expand All @@ -74,7 +74,7 @@ class DecimalSumAggregate {
if (!sum.has_value()) {
return std::nullopt;
}
auto adjustedSum =
auto const adjustedSum =
DecimalUtil::adjustSumForOverflow(sum.value(), overflow);
constexpr uint8_t maxPrecision = std::is_same_v<TSumType, int128_t>
? LongDecimalType::kMaxPrecision
Expand Down Expand Up @@ -108,8 +108,8 @@ class DecimalSumAggregate {
if (!other.has_value()) {
return false;
}
auto otherSum = other.value().template at<0>();
auto otherIsEmpty = other.value().template at<1>();
auto const otherSum = other.value().template at<0>();
auto const otherIsEmpty = other.value().template at<1>();

// isEmpty is never null.
VELOX_CHECK(otherIsEmpty.has_value());
Expand Down Expand Up @@ -143,7 +143,7 @@ class DecimalSumAggregate {
static_cast<TSumType>(finalResult.value()), isEmpty);
} else {
// Sum should be set to null on overflow,
// and isEmptyshould be set to false.
// and isEmpty should be set to false.
out.template set_null_at<0>();
out.template get_writer_at<1>() = false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class SumAggregationTest : public SumTestBase {
/*testWithTableScan*/ false);
}

// check group by partial agg overflow, and final agg output null
// Check group by partial agg overflow, and final agg output null.
void decimalGroupBySumOverflow(
const std::vector<std::optional<int128_t>>& input) {
const TypePtr type = DECIMAL(38, 0);
Expand Down

0 comments on commit 7a01252

Please sign in to comment.