edponce commented on a change in pull request #12460:
URL: https://github.com/apache/arrow/pull/12460#discussion_r834869602
##########
File path: cpp/src/arrow/compute/api_vector.cc
##########
@@ -325,6 +344,22 @@ Result<std::shared_ptr<Array>> DropNull(const Array&
values, ExecContext* ctx) {
return out.make_array();
}
+Result<std::shared_ptr<Array>> CumulativeSum(const Array& values,
+ const CumulativeSumOptions&
options,
+ ExecContext* ctx) {
+ ARROW_ASSIGN_OR_RAISE(Datum out,
+ CallFunction("cumulative_sum", {Datum(values)},
&options, ctx));
+ return out.make_array();
+}
+
+Result<std::shared_ptr<Array>> CumulativeSum(const ChunkedArray& chunked_array,
+ const CumulativeSumOptions&
options,
+ ExecContext* ctx) {
+ ARROW_ASSIGN_OR_RAISE(
+ Datum out, CallFunction("cumulative_sum", {Datum(chunked_array)},
&options, ctx));
+ return out.make_array();
Review comment:
Need to collapse these functions into a [single one that takes in Datum
as declared in the header
file](https://github.com/apache/arrow/pull/12460/files#diff-d8e3089c085273e425a5106e77566f2e1348c483a79eeb544a4c9ee0e8327f34R544).
##########
File path: cpp/src/arrow/compute/kernels/vector_cumulative_sum.cc
##########
@@ -0,0 +1,105 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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 "arrow/array/array_base.h"
+#include "arrow/compute/api_scalar.h"
+#include "arrow/compute/api_vector.h"
+#include "arrow/compute/kernels/codegen_internal.h"
+#include "arrow/compute/kernels/common.h"
+#include "arrow/compute/kernels/base_arithmetic_internal.h"
+#include "arrow/result.h"
+#include "arrow/visit_type_inline.h"
+
+namespace arrow {
+namespace compute {
+namespace internal {
+
+template <typename OutType, typename ArgType, typename Op>
+struct CumulativeGeneric {
+ using OutValue = typename GetOutputType<OutType>::T;
+ using ArgValue = typename GetViewType<ArgType>::T;
+
+ static Status Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) {
+ const auto& options = OptionsWrapper<CumulativeSumOptions>::Get(ctx);
+ auto start = UnboxScalar<ArgType>::Unbox(*options.start);
+ bool skip_nulls = options.skip_nulls;
+
+ switch (batch[0].kind()) {
+ case Datum::ARRAY: {
+ return Call(ctx, *batch[0].array(), start, out, skip_nulls);
+ }
+ case Datum::CHUNKED_ARRAY: {
+ const auto& input = batch[0].chunked_array();
+
+ for (const auto& chunk : input->chunks()) {
+ RETURN_NOT_OK(Call(ctx, *chunk->data(), start, out, skip_nulls));
+ }
+ }
+ default:
+ return Status::NotImplemented(
+ "Unsupported input type for function 'cumulative_<operator>': ",
+ batch[0].ToString());
+ }
+
+ return Status::OK();
+ }
+
+ static Status Call(KernelContext* ctx, const ArrayData& arg0, ArgValue&
partial_scan,
+ Datum* out, bool skip_nulls) {
+ Status st = Status::OK();
+ ArrayIterator<ArgType> arg0_it(arg0);
+ RETURN_NOT_OK(applicator::OutputAdapter<OutType>::Write(ctx, out, [&]() ->
OutValue {
+ partial_scan = Op::template Call<OutValue, ArgValue, ArgValue>(ctx,
arg0_it(), partial_scan,
Review comment:
Now that I come to think about this a bit more, this approach is for
operators that are safe to apply to null slot values. Most likely will need to
use custom
[visitors](https://github.com/apache/arrow/blob/master/cpp/src/arrow/compute/kernels/codegen_internal.h#L672-L680):
One for skipping nulls and for propagating the first null.
##########
File path: cpp/src/arrow/compute/kernels/vector_cumulative_sum.cc
##########
@@ -0,0 +1,105 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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 "arrow/array/array_base.h"
+#include "arrow/compute/api_scalar.h"
+#include "arrow/compute/api_vector.h"
+#include "arrow/compute/kernels/codegen_internal.h"
+#include "arrow/compute/kernels/common.h"
+#include "arrow/compute/kernels/base_arithmetic_internal.h"
+#include "arrow/result.h"
+#include "arrow/visit_type_inline.h"
+
+namespace arrow {
+namespace compute {
+namespace internal {
+
+template <typename OutType, typename ArgType, typename Op>
+struct CumulativeGeneric {
Review comment:
This file contains `CumulativeGeneratic`, so maybe rename these files to
`vector_cumulative_ops.{h,cc}`.
##########
File path: cpp/src/arrow/compute/kernels/vector_cumulative_sum.cc
##########
@@ -0,0 +1,105 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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 "arrow/array/array_base.h"
+#include "arrow/compute/api_scalar.h"
+#include "arrow/compute/api_vector.h"
+#include "arrow/compute/kernels/codegen_internal.h"
+#include "arrow/compute/kernels/common.h"
+#include "arrow/compute/kernels/base_arithmetic_internal.h"
+#include "arrow/result.h"
+#include "arrow/visit_type_inline.h"
+
+namespace arrow {
+namespace compute {
+namespace internal {
+
+template <typename OutType, typename ArgType, typename Op>
+struct CumulativeGeneric {
+ using OutValue = typename GetOutputType<OutType>::T;
+ using ArgValue = typename GetViewType<ArgType>::T;
+
+ static Status Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) {
+ const auto& options = OptionsWrapper<CumulativeSumOptions>::Get(ctx);
Review comment:
For this to be generic, need to make `CumulativeSumOptions` to a
template parameter.
##########
File path: cpp/src/arrow/compute/kernels/vector_cumulative_sum_test.cc
##########
@@ -0,0 +1,97 @@
+// Licensed to the Apache Software Foundation (ASF) under one // or more
contributor license agreements. See the NOTICE file // distributed with this
work for additional information // regarding copyright ownership. The ASF
licenses this file
+// to you 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 <algorithm>
+#include <cstdint>
+#include <cstdio>
+#include <functional>
+#include <locale>
+#include <memory>
+#include <stdexcept>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+#include "arrow/array.h"
+#include "arrow/array/builder_decimal.h"
+#include "arrow/buffer.h"
+#include "arrow/chunked_array.h"
+#include "arrow/compute/api_vector.h"
+#include "arrow/status.h"
+#include "arrow/testing/gtest_util.h" // IntegralArrowTypes
+#include "arrow/testing/util.h"
+#include "arrow/type.h"
+#include "arrow/type_fwd.h"
+#include "arrow/type_traits.h"
+#include "arrow/util/checked_cast.h"
+#include "arrow/util/decimal.h"
Review comment:
Probably many of these header files are not needed (IWYU).
##########
File path: cpp/src/arrow/compute/kernels/codegen_internal.h
##########
@@ -1134,6 +1134,37 @@ ArrayKernelExec
GeneratePhysicalInteger(detail::GetTypeId get_id) {
}
}
+template <template <typename... Args> class KernelGenerator, typename Op>
+ArrayKernelExec ArithmeticExecFromOp(detail::GetTypeId get_id) {
Review comment:
Need to add a template parameter (a parameter pack) to allow
`CumulativeSumOptions` to be passed to the `KernelGenerator`. For example,
```c++
template <template <typename...> class KernelGenerator, typename Op,
typename... Args>
ArrayKernelExec ArithmeticExecFromOp(detail::GetTypeId get_id) {
...
case Type::INT8:
return KernelGenerator<Int8Type, Int8Type, Op, Args...>::Exec;
...
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
