pitrou commented on a change in pull request #10598: URL: https://github.com/apache/arrow/pull/10598#discussion_r664031227
########## File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc ########## @@ -118,16 +141,31 @@ struct Day { // ---------------------------------------------------------------------- // Extract day of week from timestamp +// +// By default week starts on Monday represented by 0 and ends on Sunday represented +// by 6. Start day of the week (Monday=1, Sunday=7) and numbering start (0 or 1) can be +// set using DayOfWeekOptions template <typename Duration> struct DayOfWeek { + explicit DayOfWeek(DayOfWeekOptions options) : options(options) { Review comment: Nit: can you usually pass options as `const&`. ########## File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc ########## @@ -80,6 +84,25 @@ struct TemporalComponentExtract { } }; +template <typename Op, typename OutType> +struct DayOfWeekExec { + using OutValue = typename internal::GetOutputType<OutType>::T; + + static Status Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) { + DayOfWeekOptions options = DayOfWeekState::Get(ctx); Review comment: Nit: could be `const&`. ########## File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc ########## @@ -118,16 +141,31 @@ struct Day { // ---------------------------------------------------------------------- // Extract day of week from timestamp +// +// By default week starts on Monday represented by 0 and ends on Sunday represented +// by 6. Start day of the week (Monday=1, Sunday=7) and numbering start (0 or 1) can be +// set using DayOfWeekOptions template <typename Duration> struct DayOfWeek { + explicit DayOfWeek(DayOfWeekOptions options) : options(options) { + for (int i = 0; i < 7; i++) { + lookup_table[i] = i + 8 - options.week_start; + lookup_table[i] = (lookup_table[i] > 6) ? lookup_table[i] - 7 : lookup_table[i]; + lookup_table[i] += options.one_based_numbering; + } + } + template <typename T, typename Arg0> - static T Call(KernelContext*, Arg0 arg, Status*) { - return static_cast<T>( - weekday(year_month_day(floor<days>(sys_time<Duration>(Duration{arg})))) - .iso_encoding() - - 1); + T Call(KernelContext*, Arg0 arg, Status*) const { + const auto wd = arrow_vendored::date::year_month_weekday( + floor<days>(sys_time<Duration>(Duration{arg}))) + .weekday() + .iso_encoding(); + return lookup_table[wd - 1]; } + std::array<int64_t, 7> lookup_table; + DayOfWeekOptions options; Review comment: This member doesn't seem used anymore? -- 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-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org