Skip to content

Commit

Permalink
linting and doc fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@khwilson
khwilson committed Oct 13, 2024
1 parent 01e53b3 commit 9901d1f
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 39 deletions.
11 changes: 6 additions & 5 deletions cpp/src/arrow/compute/kernels/aggregate_basic.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1053,10 +1053,10 @@ void RegisterScalarAggregateBasic(FunctionRegistry* registry) {
func = std::make_shared<ScalarAggregateFunction>("sum", Arity::Unary(), sum_doc,
&default_scalar_aggregate_options);
AddArrayScalarAggKernels(SumInit, {boolean()}, uint64(), func.get());
AddAggKernel(KernelSignature::Make({Type::DECIMAL32}, MaxPrecisionDecimalType),
SumInit, func.get(), SimdLevel::NONE);
AddAggKernel(KernelSignature::Make({Type::DECIMAL64}, MaxPrecisionDecimalType),
SumInit, func.get(), SimdLevel::NONE);
AddAggKernel(KernelSignature::Make({Type::DECIMAL32}, MaxPrecisionDecimalType), SumInit,
func.get(), SimdLevel::NONE);
AddAggKernel(KernelSignature::Make({Type::DECIMAL64}, MaxPrecisionDecimalType), SumInit,
func.get(), SimdLevel::NONE);
AddAggKernel(KernelSignature::Make({Type::DECIMAL128}, MaxPrecisionDecimalType),
SumInit, func.get(), SimdLevel::NONE);
AddAggKernel(KernelSignature::Make({Type::DECIMAL256}, MaxPrecisionDecimalType),
Expand Down Expand Up @@ -1204,7 +1204,8 @@ void RegisterScalarAggregateBasic(FunctionRegistry* registry) {
AddBasicAggKernels(IndexInit::Init, PrimitiveTypes(), int64(), func.get());
AddBasicAggKernels(IndexInit::Init, TemporalTypes(), int64(), func.get());
AddBasicAggKernels(IndexInit::Init,
{fixed_size_binary(1), decimal32(1, 0), decimal64(1, 0), decimal128(1, 0), decimal256(1, 0), null()},
{fixed_size_binary(1), decimal32(1, 0), decimal64(1, 0),
decimal128(1, 0), decimal256(1, 0), null()},
int64(), func.get());
DCHECK_OK(registry->AddFunction(std::move(func)));
}
Expand Down
50 changes: 32 additions & 18 deletions cpp/src/arrow/compute/kernels/aggregate_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,10 @@ TEST_F(TestSumKernelRoundOff, Basics) {
}

TEST(TestDecimalSumKernel, SimpleSum) {
std::vector<std::shared_ptr<DataType>> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};
std::vector<std::shared_ptr<DataType>> init_types = {
decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {
decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};

for (size_t i = 0; i < init_types.size(); ++i) {
auto& ty = init_types[i];
Expand Down Expand Up @@ -541,12 +543,14 @@ TEST(TestDecimalSumKernel, SimpleSum) {
}

TEST(TestDecimalSumKernel, ScalarAggregateOptions) {
std::vector<std::shared_ptr<DataType>> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};
std::vector<std::shared_ptr<DataType>> init_types = {
decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {
decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};

for (size_t i = 0; i < init_types.size(); ++i) {
auto& ty = init_types[i];
auto& out_ty = out_types[i ];
auto& out_ty = out_types[i];

Datum null = ScalarFromJSON(out_ty, R"(null)");
Datum zero = ScalarFromJSON(out_ty, R"("0.00")");
Expand Down Expand Up @@ -723,12 +727,14 @@ TYPED_TEST(TestNumericProductKernel, ScalarAggregateOptions) {
}

TEST(TestDecimalProductKernel, SimpleProduct) {
std::vector<std::shared_ptr<DataType>> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};
std::vector<std::shared_ptr<DataType>> init_types = {
decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {
decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};

for (size_t i = 0; i < init_types.size(); ++i) {
auto& ty = init_types[i];
auto& out_ty = out_types[i ];
auto& out_ty = out_types[i];

Datum null = ScalarFromJSON(out_ty, R"(null)");

Expand All @@ -754,7 +760,8 @@ TEST(TestDecimalProductKernel, SimpleProduct) {
EXPECT_THAT(Product(ArrayFromJSON(ty, R"([null])"), options),
ResultWith(ScalarFromJSON(out_ty, R"("1.00")")));
chunks = ChunkedArrayFromJSON(ty, {});
EXPECT_THAT(Product(chunks, options), ResultWith(ScalarFromJSON(out_ty, R"("1.00")")));
EXPECT_THAT(Product(chunks, options),
ResultWith(ScalarFromJSON(out_ty, R"("1.00")")));

EXPECT_THAT(Product(ArrayFromJSON(
ty, R"(["1.00", null, "-3.00", null, "3.00", null, "7.00"])"),
Expand All @@ -768,12 +775,14 @@ TEST(TestDecimalProductKernel, SimpleProduct) {
}

TEST(TestDecimalProductKernel, ScalarAggregateOptions) {
std::vector<std::shared_ptr<DataType>> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};
std::vector<std::shared_ptr<DataType>> init_types = {
decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {
decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};

for (size_t i = 0; i < init_types.size(); ++i) {
auto& ty = init_types[i];
auto& out_ty = out_types[i ];
auto& out_ty = out_types[i];

Datum null = ScalarFromJSON(out_ty, R"(null)");
Datum one = ScalarFromJSON(out_ty, R"("1.00")");
Expand Down Expand Up @@ -1360,8 +1369,10 @@ TYPED_TEST(TestRandomNumericMeanKernel, RandomArrayMeanOverflow) {
TEST(TestDecimalMeanKernel, SimpleMean) {
ScalarAggregateOptions options(/*skip_nulls=*/true, /*min_count=*/0);

std::vector<std::shared_ptr<DataType>> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};
std::vector<std::shared_ptr<DataType>> init_types = {
decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {
decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};

for (size_t i = 0; i < init_types.size(); ++i) {
auto& ty = init_types[i];
Expand Down Expand Up @@ -1421,8 +1432,9 @@ TEST(TestDecimalMeanKernel, SimpleMean) {
}

// TODO: Currently, casts are not implemented for decimal32/64 so we ignore that for now
// init_types = {decimal32(3, -2), decimal64(3, -2), decimal128(3, -2), decimal256(3, -2)};
// out_types = {decimal32(9, -2), decimal64(18, -2), decimal128(38, -2), decimal256(76, -2)};
// init_types = {decimal32(3, -2), decimal64(3, -2), decimal128(3, -2), decimal256(3,
// -2)}; out_types = {decimal32(9, -2), decimal64(18, -2), decimal128(38, -2),
// decimal256(76, -2)};

init_types = {decimal128(3, -2), decimal256(3, -2)};
out_types = {decimal128(38, -2), decimal256(76, -2)};
Expand Down Expand Up @@ -1463,8 +1475,10 @@ TEST(TestDecimalMeanKernel, SimpleMean) {
}

TEST(TestDecimalMeanKernel, ScalarAggregateOptions) {
std::vector<std::shared_ptr<DataType>> init_types = {decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};
std::vector<std::shared_ptr<DataType>> init_types = {
decimal32(3, 2), decimal64(3, 2), decimal128(3, 2), decimal256(3, 2)};
std::vector<std::shared_ptr<DataType>> out_types = {
decimal32(9, 2), decimal64(18, 2), decimal128(38, 2), decimal256(76, 2)};

for (size_t i = 0; i < init_types.size(); ++i) {
auto& ty = init_types[i];
Expand Down
13 changes: 9 additions & 4 deletions cpp/src/arrow/compute/kernels/codegen_internal.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ Result<TypeHolder> ListValuesType(KernelContext* ctx,

Result<TypeHolder> MaxPrecisionDecimalType(KernelContext*,
const std::vector<TypeHolder>& args) {
ARROW_ASSIGN_OR_RAISE(auto out_type_, WidenDecimalToMaxPrecision(args[0].GetSharedPtr()));
ARROW_ASSIGN_OR_RAISE(auto out_type_,
WidenDecimalToMaxPrecision(args[0].GetSharedPtr()));
return out_type_;
}

Expand Down Expand Up @@ -533,9 +534,11 @@ Status CastDecimalArgs(TypeHolder* begin, size_t count) {
return Status::OK();
}

Result<std::shared_ptr<DataType>> WidenDecimalToMaxPrecision(std::shared_ptr<DataType> type) {
Result<std::shared_ptr<DataType>> WidenDecimalToMaxPrecision(
std::shared_ptr<DataType> type) {
if (!is_decimal(type->id())) {
return Status::TypeError("Non-DecimalType passed to WidenDecimalToMaxPrecision: " + type->ToString());
return Status::TypeError("Non-DecimalType passed to WidenDecimalToMaxPrecision: " +
type->ToString());
}
auto cast_type = checked_pointer_cast<DecimalType>(type);
switch (type->id()) {
Expand All @@ -548,7 +551,9 @@ Result<std::shared_ptr<DataType>> WidenDecimalToMaxPrecision(std::shared_ptr<Dat
case Type::DECIMAL256:
return Decimal256Type::Make(Decimal256Type::kMaxPrecision, cast_type->scale());
default:
return Status::TypeError("An unknown DecimalType was passed to WidenDecimalToMaxPrecision: " + type->ToString());
return Status::TypeError(
"An unknown DecimalType was passed to WidenDecimalToMaxPrecision: " +
type->ToString());
};
}

Expand Down
5 changes: 3 additions & 2 deletions cpp/src/arrow/compute/kernels/codegen_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1446,9 +1446,10 @@ Status CastDecimalArgs(TypeHolder* begin, size_t count);
/// Given a DataType, if it is a DecimalType, return a DecimalType with the same scale
/// and the maximum precision for that DecimalType.
///
/// If it is _not_ a DecimalType, return type
/// If it is _not_ a DecimalType, return a Status::TypeError
ARROW_EXPORT
Result<std::shared_ptr<DataType>> WidenDecimalToMaxPrecision(std::shared_ptr<DataType> type);
Result<std::shared_ptr<DataType>> WidenDecimalToMaxPrecision(
std::shared_ptr<DataType> type);

ARROW_EXPORT
bool HasDecimal(const std::vector<TypeHolder>& types);
Expand Down
20 changes: 10 additions & 10 deletions cpp/src/arrow/compute/kernels/test_util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ inline std::shared_ptr<Array> DecimalArrayFromJSON(const std::shared_ptr<DataTyp
const auto& ty = checked_cast<const DecimalType&>(*type);
if (ty.scale() >= 0) return ArrayFromJSON(type, json);
auto p = ty.precision() - ty.scale();
std::shared_ptr<DataType> adjusted_ty =
ty.id() == Type::DECIMAL32 ? decimal32(p, 0) :
ty.id() == Type::DECIMAL64 ? decimal64(p, 0) :
ty.id() == Type::DECIMAL128 ? decimal128(p, 0) :
decimal256(p, 0);
std::shared_ptr<DataType> adjusted_ty = ty.id() == Type::DECIMAL32 ? decimal32(p, 0)
: ty.id() == Type::DECIMAL64 ? decimal64(p, 0)
: ty.id() == Type::DECIMAL128
? decimal128(p, 0)
: decimal256(p, 0);
return Cast(ArrayFromJSON(adjusted_ty, json), type).ValueOrDie().make_array();
}

Expand All @@ -91,11 +91,11 @@ inline std::shared_ptr<Scalar> DecimalScalarFromJSON(
const auto& ty = checked_cast<const DecimalType&>(*type);
if (ty.scale() >= 0) return ScalarFromJSON(type, json);
auto p = ty.precision() - ty.scale();
std::shared_ptr<DataType> adjusted_ty =
ty.id() == Type::DECIMAL32 ? decimal32(p, 0) :
ty.id() == Type::DECIMAL64 ? decimal64(p, 0) :
ty.id() == Type::DECIMAL128 ? decimal128(p, 0) :
decimal256(p, 0);
std::shared_ptr<DataType> adjusted_ty = ty.id() == Type::DECIMAL32 ? decimal32(p, 0)
: ty.id() == Type::DECIMAL64 ? decimal64(p, 0)
: ty.id() == Type::DECIMAL128
? decimal128(p, 0)
: decimal256(p, 0);
return Cast(ScalarFromJSON(adjusted_ty, json), type).ValueOrDie().scalar();
}

Expand Down

0 comments on commit 9901d1f

Please sign in to comment.