aucahuasi commented on a change in pull request #11075:
URL: https://github.com/apache/arrow/pull/11075#discussion_r703112886
##########
File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc
##########
@@ -660,36 +736,46 @@ struct ISOCalendar {
template <
template <typename...> class Op,
- template <template <typename...> class OpExec, typename Duration, typename
OutType>
+ template <template <typename...> class OpExec, typename Duration, typename
InType, typename OutType>
class ExecTemplate,
typename OutType>
-std::shared_ptr<ScalarFunction> MakeTemporal(
+std::shared_ptr<ScalarFunction> MakeTemporal(bool enable_date,
Review comment:
Thanks, but I suggest to keep the provided implementation in this PR:
- It requires less code/It change less parts.
- It integrates well into the `MakeTemporal*` functions.
- It is setting up the date kernels (optionally) at the moment of creating
the function.
Also, I think that if I add `AddDateKernels` we would need 2 versions: one
for `MakeTemporal` and other for `MakeSimpleUnaryTemporal`.
So I think it makes sense to keep the current code. Let me know what do you
think here @lidavidm
--
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]
