[GitHub] [arrow] lidavidm commented on pull request #11990: ARROW-15032 Add DateStruct Function

[GitBox <gi...@apache.org>]

lidavidm commented on pull request #11990:
URL: https://github.com/apache/arrow/pull/11990#issuecomment-999561189


   > > Updated to make YearMonthDay work more closely to the other Unary funcs. This doesn't build atm because (as far as I can tell) passing a StructArray to the `UnaryTemporalFactory` doesn't resolve to existing specializations. Need to study the factory machinery a little more, but I'm guessing something needs to be changed with the factories themselves to allow this
   > 
   > I think this is also the reason `ISOCalendar` is implemented the way is was.
   
   Apologies, you are right, you cannot implement it exactly the same way as the other unary functions.
   
   I meant something more like this:
   
   ```cpp
   template <typename Duration, typename Localizer>
   struct YearMonthDay {
     explicit YearMonthDay(const FunctionOptions* options, Localizer&& localizer)
         : localizer_(std::move(localizer)) {}
   
     template <typename T, typename Arg0>
     T Call(KernelContext*, Arg0 arg, Status*) const {
       static_assert(std::is_same<T, year_month_day>, "");
       return year_month_day(floor<days>(localizer_.template ConvertTimePoint<Duration>(arg)));
     }
   
     Localizer localizer_;
   };
   
   // ...
   
   // Original PR implementation, but using the YearMonthDay struct to share some of the code between the scalar/array cases
   
   ```
   
   i.e. to just factor out a bit more of the shared code between the scalar/array cases.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org