zanmato1984 commented on code in PR #45763:
URL: https://github.com/apache/arrow/pull/45763#discussion_r1995006714


##########
cpp/src/arrow/compute/kernels/codegen_internal.h:
##########
@@ -988,9 +988,9 @@ struct FailFunctor<VectorKernel::ChunkedExec> {
 };
 
 // GD for numeric types (integer and floating point)
-template <template <typename...> class Generator, typename Type0,
-          typename KernelType = ArrayKernelExec, typename... Args>
-KernelType GenerateNumeric(detail::GetTypeId get_id) {
+template <template <typename...> class Generator, typename Type0, typename... 
Args>

Review Comment:
   Is this change necessary? I mean given that there are quite a few similar 
generators, it may worth to clean them up all together probably in an 
individual PR?



##########
cpp/src/arrow/compute/kernels/aggregate_quantile.cc:
##########
@@ -524,23 +530,13 @@ void AddQuantileKernels(VectorFunction* func) {
     base.signature = KernelSignature::Make({InputType(ty)}, 
OutputType(ResolveOutput));
     // output type is determined at runtime, set template argument to nulltype
     base.exec = GenerateNumeric<QuantileExecutor, NullType>(*ty);
-    base.exec_chunked =
-        GenerateNumeric<QuantileExecutorChunked, NullType, 
VectorKernel::ChunkedExec>(
-            *ty);
-    DCHECK_OK(func->AddKernel(base));
-  }
-  {
-    base.signature =
-        KernelSignature::Make({InputType(Type::DECIMAL128)}, 
OutputType(ResolveOutput));
-    base.exec = QuantileExecutor<NullType, Decimal128Type>::Exec;
-    base.exec_chunked = QuantileExecutorChunked<NullType, 
Decimal128Type>::Exec;
+    base.exec_chunked = GenerateNumeric<QuantileExecutorChunked, 
NullType>(*ty);
     DCHECK_OK(func->AddKernel(base));
   }
-  {
-    base.signature =
-        KernelSignature::Make({InputType(Type::DECIMAL256)}, 
OutputType(ResolveOutput));
-    base.exec = QuantileExecutor<NullType, Decimal256Type>::Exec;
-    base.exec_chunked = QuantileExecutorChunked<NullType, 
Decimal256Type>::Exec;
+  for (auto type_id : DecimalTypeIds()) {

Review Comment:
   For the record, this is actually extending the supported decimal types by 
`Decimal32` and `Decimal64` that is independent of function `winsorize`?



-- 
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]

Reply via email to