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


Reply via email to