You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/15 20:25:57 UTC

[GitHub] [arrow] rok commented on a diff in pull request #12528: ARROW-15251: [C++] Temporal floor/ceil/round handle ambiguous/nonexistent local time

rok commented on code in PR #12528:
URL: https://github.com/apache/arrow/pull/12528#discussion_r851496621


##########
cpp/src/arrow/compute/kernels/temporal_internal.h:
##########
@@ -120,18 +123,59 @@ struct ZonedLocalizer {
   }
 
   template <typename Duration>
-  Duration ConvertLocalToSys(Duration t, Status* st) const {
+  Duration get_local_time(Duration arg) const {
+    return zoned_time<Duration>(tz, local_time<Duration>(arg))
+        .get_sys_time()
+        .time_since_epoch();
+  }
+
+  template <typename Duration>
+  Duration get_local_time(Duration arg, const arrow_vendored::date::choose choose) const {
+    return zoned_time<Duration>(tz, local_time<Duration>(arg), choose)
+        .get_sys_time()
+        .time_since_epoch();
+  }
+
+  template <typename Duration>
+  Duration ConvertLocalToSys(
+      Duration t, Status* st,
+      const AmbiguousTime ambiguous = AmbiguousTime::AMBIGUOUS_RAISE,
+      const NonexistentTime nonexistent_time = NonexistentTime::NONEXISTENT_RAISE) const {
     try {
       return zoned_time<Duration>{tz, local_time<Duration>(t)}
           .get_sys_time()
           .time_since_epoch();
     } catch (const arrow_vendored::date::nonexistent_local_time& e) {
-      *st = Status::Invalid("Local time does not exist: ", e.what());
-      return Duration{0};
+      switch (nonexistent_time) {
+        case NonexistentTime::NONEXISTENT_RAISE: {
+          *st = Status::Invalid("Timestamp doesn't exist in timezone '", tz,
+                                "': ", e.what());
+          return t;
+        }
+        case NonexistentTime::NONEXISTENT_EARLIEST: {
+          return get_local_time<Duration>(t, arrow_vendored::date::choose::latest) -
+                 Duration{1};
+        }
+        case NonexistentTime::NONEXISTENT_LATEST: {
+          return get_local_time<Duration>(t, arrow_vendored::date::choose::latest);
+        }
+      }
     } catch (const arrow_vendored::date::ambiguous_local_time& e) {
-      *st = Status::Invalid("Local time is ambiguous: ", e.what());
-      return Duration{0};
+      switch (ambiguous) {
+        case AmbiguousTime::AMBIGUOUS_RAISE: {
+          *st = Status::Invalid("Timestamp is ambiguous in timezone '", tz,
+                                "': ", e.what());
+          return t;
+        }
+        case AmbiguousTime::AMBIGUOUS_EARLIEST: {
+          return get_local_time<Duration>(t, arrow_vendored::date::choose::earliest);
+        }
+        case AmbiguousTime::AMBIGUOUS_LATEST: {
+          return get_local_time<Duration>(t, arrow_vendored::date::choose::latest);
+        }
+      }
     }
+    return Duration{0};
   }
 

Review Comment:
   ```suggestion
     template <typename Duration, typename Unit>
     Duration FloorTime(const int64_t t, const RoundTemporalOptions* options) const {
       const sys_time<Duration> st = sys_time<Duration>{Duration{t}};
       const std::chrono::seconds offset = tz->get_info(tz->to_local(st)).first.offset;
       const Unit d = floor<Unit>(st - offset).time_since_epoch();
   
       if (options->multiple == 1) {
         return duration_cast<Duration>(d + offset);
       } else {
         const Unit unit = Unit{options->multiple};
         const Unit m = (d.count() >= 0) ? d / unit * unit : (d - unit + Unit{1}) / unit * unit;
         return duration_cast<Duration>(m + offset);
       }
     }
   
     template <typename Duration, typename Unit>
     Duration CeilTime(const int64_t t, const RoundTemporalOptions* options) const {
       const Duration d = FloorTime<Duration, Unit>(t, options);
       if (d.count() < t) {
         return d + duration_cast<Duration>(Unit{options->multiple});
       }
       return d;
     }
   
     template <typename Duration, typename Unit>
     Duration RoundTime(const int64_t t, const RoundTemporalOptions* options) const {
       const Duration f = FloorTime<Duration, Unit>(t, options);
       Duration c = f;
       if (f.count() < t) {
         c += duration_cast<Duration>(Unit{options->multiple});
       }
       return (t - f.count() >= c.count() - t) ? c : f;
     }
   ```



##########
cpp/src/arrow/compute/kernels/temporal_internal.h:
##########
@@ -101,7 +101,10 @@ struct NonZonedLocalizer {
   }
 
   template <typename Duration>
-  Duration ConvertLocalToSys(Duration t, Status* st) const {
+  Duration ConvertLocalToSys(
+      Duration t, Status* st,
+      const AmbiguousTime ambiguous = AmbiguousTime::AMBIGUOUS_RAISE,
+      const NonexistentTime nonexistent_time = NonexistentTime::NONEXISTENT_RAISE) const {
     return t;
   }
 

Review Comment:
   ```suggestion
     template <typename Duration, typename Unit>
     Duration FloorTime(int64_t t, const RoundTemporalOptions* options) const {
       const Unit d = floor<Unit>(sys_time<Duration>(Duration{t})).time_since_epoch();
   
       if (options->multiple == 1) {
         return duration_cast<Duration>(d);
       } else {
         const Unit unit = Unit{options->multiple};
         const Unit m = (d.count() >= 0) ? d / unit * unit : (d - unit + Unit{1}) / unit * unit;
         return duration_cast<Duration>(m);
       }
     }
   
     template <typename Duration, typename Unit>
     Duration CeilTime(int64_t t, const RoundTemporalOptions* options) const {
       const Duration d = FloorTime<Duration, Unit>(t, options);
       if (d.count() < t) {
         return d + duration_cast<Duration>(Unit{options->multiple});
       }
       return d;
     }
   
     template <typename Duration, typename Unit>
     Duration RoundTime(const int64_t t, const RoundTemporalOptions* options) const {
       const Duration f = FloorTime<Duration, Unit>(t, options);
       Duration c = f;
       if (f.count() < t) {
         c += duration_cast<Duration>(Unit{options->multiple});
       }
       return (t - f.count() >= c.count() - t) ? c : f;
     }
   ```



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