impl(spanner): remove Interval arithmetic functions (#15117)

Author: scotthart

ci/abi-dumps/google_cloud_cpp_spanner.expected.abi.dump.gz

@@ -50,15 +50,6 @@ using std::chrono::minutes;
 using std::chrono::nanoseconds;
 using std::chrono::seconds;
 
-StatusOr<absl::TimeZone> LoadTimeZone(absl::string_view name) {
-  absl::TimeZone tz;
-  if (!absl::LoadTimeZone(name, &tz)) {
-    return internal::InvalidArgumentError(
-        absl::StrFormat("%s: Invalid time zone", name), GCP_ERROR_INFO());
-  }
-  return tz;
-}
-
 // Round d to a microsecond boundary (to even in halfway cases).
 microseconds Round(nanoseconds d) {
   auto trunc = duration_cast<microseconds>(d);
@@ -252,56 +243,6 @@ auto NameIs(char name) {
   return [name](ISO8601UnitFactory const& u) { return u.name == name; };
 }
 
-StatusOr<Interval> ParseISO8601Interval(absl::string_view str) {
-  Interval intvl;
-  absl::string_view s = str;
-
-  auto const* units = std::begin(kISO8601DateUnitFactories);
-  auto const* units_end = std::end(kISO8601DateUnitFactories);
-  enum { kValue, kUnit, kNothing } expecting = kValue;
-  bool negated = false;
-
-  for (;;) {
-    if (units == std::begin(kISO8601DateUnitFactories)) {
-      negated = !absl::ConsumePrefix(&s, "+") && absl::ConsumePrefix(&s, "-");
-      if (!absl::ConsumePrefix(&s, "P")) break;
-    }
-    if (units_end == std::end(kISO8601DateUnitFactories)) {
-      if (absl::ConsumePrefix(&s, "T")) {
-        units = std::begin(kISO8601TimeUnitFactories);
-        units_end = std::end(kISO8601TimeUnitFactories);
-        expecting = kValue;
-      }
-    }
-    if (units == units_end) break;
-    double vf;
-    if (ParseDouble(s, vf, true, true)) {
-      expecting = kUnit;
-      if (s.empty()) break;
-      units = std::find_if(units, units_end, NameIs(s.front()));
-      if (units == units_end) break;
-      intvl += units++->factory(1) * vf;
-      expecting = kNothing;
-      s.remove_prefix(1);
-      break;  // must be last unit
-    }
-    std::int32_t vn;
-    if (!ParseInteger(s, vn, true)) break;
-    expecting = kUnit;
-    if (s.empty()) break;
-    units = std::find_if(units, units_end, NameIs(s.front()));
-    if (units == units_end) break;
-    intvl += units++->factory(vn);
-    expecting = kNothing;
-    s.remove_prefix(1);
-  }
-
-  if (!s.empty() || expecting != kNothing) {
-    return SyntaxError(str, s, GCP_ERROR_INFO());
-  }
-  return negated ? -intvl : intvl;
-}
-
 /**
  * https://www.postgresql.org/docs/current/datatype-datetime.html
  *
@@ -398,7 +339,59 @@ Interval PostgreSqlUnit(absl::string_view& s, std::int32_t n) {
   return Interval(0, 0, n);  // default to days without removing unit
 }
 
-StatusOr<Interval> ParsePostgreSqlInterval(absl::string_view str) {
+}  // namespace
+
+StatusOr<Interval> Interval::ParseISO8601Interval(absl::string_view str) {
+  Interval intvl;
+  absl::string_view s = str;
+
+  auto const* units = std::begin(kISO8601DateUnitFactories);
+  auto const* units_end = std::end(kISO8601DateUnitFactories);
+  enum { kValue, kUnit, kNothing } expecting = kValue;
+  bool negated = false;
+
+  for (;;) {
+    if (units == std::begin(kISO8601DateUnitFactories)) {
+      negated = !absl::ConsumePrefix(&s, "+") && absl::ConsumePrefix(&s, "-");
+      if (!absl::ConsumePrefix(&s, "P")) break;
+    }
+    if (units_end == std::end(kISO8601DateUnitFactories)) {
+      if (absl::ConsumePrefix(&s, "T")) {
+        units = std::begin(kISO8601TimeUnitFactories);
+        units_end = std::end(kISO8601TimeUnitFactories);
+        expecting = kValue;
+      }
+    }
+    if (units == units_end) break;
+    double vf;
+    if (ParseDouble(s, vf, true, true)) {
+      expecting = kUnit;
+      if (s.empty()) break;
+      units = std::find_if(units, units_end, NameIs(s.front()));
+      if (units == units_end) break;
+      intvl += units++->factory(1) * vf;
+      expecting = kNothing;
+      s.remove_prefix(1);
+      break;  // must be last unit
+    }
+    std::int32_t vn;
+    if (!ParseInteger(s, vn, true)) break;
+    expecting = kUnit;
+    if (s.empty()) break;
+    units = std::find_if(units, units_end, NameIs(s.front()));
+    if (units == units_end) break;
+    intvl += units++->factory(vn);
+    expecting = kNothing;
+    s.remove_prefix(1);
+  }
+
+  if (!s.empty() || expecting != kNothing) {
+    return SyntaxError(str, s, GCP_ERROR_INFO());
+  }
+  return negated ? -intvl : intvl;
+}
+
+StatusOr<Interval> Interval::ParsePostgreSqlInterval(absl::string_view str) {
   Interval intvl;
   absl::string_view s = str;
 
@@ -441,8 +434,6 @@ StatusOr<Interval> ParsePostgreSqlInterval(absl::string_view str) {
   return intvl;
 }
 
-}  // namespace
-
 bool operator==(Interval const& a, Interval const& b) {
   return Cmp<std::equal_to>(a.months_, a.days_, a.offset_,  //
                             b.months_, b.days_, b.offset_);
@@ -491,10 +482,11 @@ Interval::operator std::string() const {
 }
 
 StatusOr<Interval> MakeInterval(absl::string_view s) {
-  auto interval = ParseISO8601Interval(s);
+  auto interval = Interval::ParseISO8601Interval(s);
   if (interval.ok()) return interval;
 
-  auto pg_interval = ParsePostgreSqlInterval(absl::AsciiStrToLower(s));
+  auto pg_interval =
+      Interval::ParsePostgreSqlInterval(absl::AsciiStrToLower(s));
   if (pg_interval.ok()) return pg_interval;
 
   // We failed to parse the argument using either syntax. The most
@@ -532,34 +524,6 @@ Interval JustifyInterval(Interval intvl) {
   return JustifyDays(JustifyHours(intvl));
 }
 
-StatusOr<Timestamp> Add(Timestamp const& ts, Interval const& intvl,
-                        absl::string_view time_zone) {
-  auto tz = LoadTimeZone(time_zone);
-  if (!tz) return tz.status();
-  auto ci = tz->At(*ts.get<absl::Time>());
-  auto cs = absl::CivilSecond(  // add the civil-time parts
-      ci.cs.year(), ci.cs.month() + intvl.months_, ci.cs.day() + intvl.days_,
-      ci.cs.hour(), ci.cs.minute(), ci.cs.second());
-  return *MakeTimestamp(internal::ToProtoTimestamp(  // overflow saturates
-      absl::FromCivil(cs, *tz) + ci.subsecond +
-      absl::FromChrono(intvl.offset_)));
-}
-
-StatusOr<Interval> Diff(Timestamp const& ts1, Timestamp const& ts2,
-                        absl::string_view time_zone) {
-  auto tz = LoadTimeZone(time_zone);
-  if (!tz) return tz.status();
-  auto ci1 = tz->At(*ts1.get<absl::Time>());
-  auto ci2 = tz->At(*ts2.get<absl::Time>());
-  auto days = absl::CivilDay(ci1.cs) - absl::CivilDay(ci2.cs);
-  auto offset = absl::Hours(ci1.cs.hour() - ci2.cs.hour()) +
-                absl::Minutes(ci1.cs.minute() - ci2.cs.minute()) +
-                absl::Seconds(ci1.cs.second() - ci2.cs.second()) +
-                (ci1.subsecond - ci2.subsecond);
-  return JustifyHours(Interval(0, 0, static_cast<std::int32_t>(days),
-                               absl::ToChronoNanoseconds(offset)));
-}
-
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace spanner
 }  // namespace cloud

google/cloud/spanner/interval.cc

@@ -36,6 +36,8 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
  * @see https://cloud.google.com/spanner/docs/data-types#interval_type
  */
 class Interval {
+  friend class IntervalTest;
+
  public:
   /// Default construction yields a zero-length interval.
   Interval() : Interval(0, 0, 0) {}
@@ -79,24 +81,6 @@ class Interval {
   }
   ///@}
 
-  /// @name Arithmetic operators
-  ///
-  /// Beware: Fractional multiplication similarly assumes 30-days months
-  /// and 24-hour days, and may lead to counterintuitive results.
-  ///@{
-  Interval operator-() const;
-  Interval& operator+=(Interval const&);
-  Interval& operator-=(Interval const& intvl) {
-    *this += -intvl;
-    return *this;
-  }
-  Interval& operator*=(double);
-  Interval& operator/=(double d) {
-    *this *= 1.0 / d;
-    return *this;
-  }
-  ///@}
-
   /// @name Conversion to a string using ISO8601 duration format.
   explicit operator std::string() const;
 
@@ -105,30 +89,39 @@ class Interval {
     return os << std::string(intvl);
   }
 
+  /// @name Negation
+  Interval operator-() const;
+
  private:
   Interval(std::int32_t months, std::int32_t days,
            std::chrono::nanoseconds offset)
       : months_(months), days_(days), offset_(offset) {}
 
+  static StatusOr<Interval> ParseISO8601Interval(absl::string_view str);
+  static StatusOr<Interval> ParsePostgreSqlInterval(absl::string_view str);
+
   friend Interval JustifyDays(Interval);
   friend Interval JustifyHours(Interval);
-  friend StatusOr<Timestamp> Add(Timestamp const&, Interval const&,
-                                 absl::string_view);
+  friend StatusOr<Interval> MakeInterval(absl::string_view);
+  friend class IntervalTest;
+
+  // Beware: Fractional multiplication similarly assumes 30-days months
+  // and 24-hour days, and may lead to counterintuitive results.
+  Interval& operator+=(Interval const&);
+  Interval& operator-=(Interval const& intvl) {
+    *this += -intvl;
+    return *this;
+  }
+  Interval& operator*=(double);
+  Interval operator+(Interval rhs) const { return Interval(*this) += rhs; }
+  Interval operator-(Interval rhs) const { return Interval(*this) -= rhs; }
+  Interval operator*(double rhs) const { return Interval(*this) *= rhs; }
 
   std::int32_t months_;
   std::int32_t days_;
   std::chrono::nanoseconds offset_;
 };
 
-/// @name Binary operators
-///@{
-inline Interval operator+(Interval lhs, Interval rhs) { return lhs += rhs; }
-inline Interval operator-(Interval lhs, Interval rhs) { return lhs -= rhs; }
-inline Interval operator*(Interval lhs, double rhs) { return lhs *= rhs; }
-inline Interval operator*(double lhs, Interval rhs) { return rhs *= lhs; }
-inline Interval operator/(Interval lhs, double rhs) { return lhs /= rhs; }
-///@}
-
 /**
  * Construct an `Interval` from a string. At least handles the format
  * produced by `Interval::operator std::string()`.
@@ -153,23 +146,6 @@ Interval JustifyHours(Interval);
  */
 Interval JustifyInterval(Interval);
 
-/**
- * Add the Interval to the Timestamp in the civil-time space defined by
- * the time zone. Saturates the Timestamp result upon overflow. Returns
- * an error status if the time zone cannot be loaded.
- */
-StatusOr<Timestamp> Add(Timestamp const&, Interval const&,
-                        absl::string_view time_zone);
-
-/**
- * Intervals constructed by subtracting two timestamps are partially
- * justified, returning a whole number of days plus any remainder. A
- * year/month value will never be present. Undefined on days overflow.
- * Returns an error status if the time zone cannot be loaded.
- */
-StatusOr<Interval> Diff(Timestamp const&, Timestamp const&,
-                        absl::string_view time_zone);
-
 GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
 }  // namespace spanner
 }  // namespace cloud