Fix datetime millisecond rounding bug 6272

This makes us round times to the nearest microsecond when converting
to a boost time, instead of rounding down (after which point we
truncate again when converting to ISO8601).

src/rdb_protocol/pseudo_time.cc

@@ -384,14 +384,19 @@ datum_t iso8601_to_time(
 }
 
 const int64_t sec_incr = INT_MAX;
-void add_seconds_to_ptime(ptime_t *t, double raw_sec) {
+void add_seconds_to_ptime(reql_version_t reql_version, ptime_t *t, double raw_sec) {
     int64_t sec = raw_sec;
-    int64_t microsec = (raw_sec * 1000000.0) - (sec * 1000000);
+    int sign = raw_sec < 0 ? -1 : 1;
+    int64_t microsec;
+    if (reql_version < reql_version_t::v2_4) {
+        microsec = (raw_sec * 1000000.0) - (sec * 1000000);
+    } else {
+        microsec = (raw_sec * 1000000.0) - (sec * 1000000.0) + sign * 0.5;
+    }
 
     // boost::posix_time::seconds doesn't like large numbers, and like any
     // mature library, it reacts by silently overflowing somewhere and producing
     // an incorrect date if you give it a number that it doesn't like.
-    int sign = sec < 0 ? -1 : 1;
     sec *= sign;
     while (sec > 0) {
         int64_t diff = std::min(sec, sec_incr);
@@ -403,10 +408,10 @@ void add_seconds_to_ptime(ptime_t *t, double raw_sec) {
     *t += boost::posix_time::microseconds(microsec);
 }
 
-time_t time_to_boost(datum_t d) {
+time_t time_to_boost(reql_version_t reql_version, datum_t d) {
     double raw_sec = d.get_field(epoch_time_key).as_num();
     ptime_t t(date_t(1970, 1, 1));
-    add_seconds_to_ptime(&t, raw_sec);
+    add_seconds_to_ptime(reql_version, &t, raw_sec);
 
     const datum_t tz = d.get_field(timezone_key, NOTHROW);
     if (tz.has()) {
@@ -428,9 +433,9 @@ const std::locale &no_tz_format() {
     return it;
 }
 
-std::string time_to_iso8601(datum_t d) {
+std::string time_to_iso8601(reql_version_t reql_version, datum_t d) {
     try {
-        time_t t = time_to_boost(d);
+        time_t t = time_to_boost(reql_version, d);
         int year = t.date().year();
         // Boost also accepts year 10000.  I don't think any real users will hit
         // that edge case, but better safe than sorry.
@@ -445,7 +450,7 @@ std::string time_to_iso8601(datum_t d) {
         } else {
             ss.imbue(no_tz_format());
         }
-        ss << time_to_boost(d);
+        ss << t;
         std::string s = ss.str();
         size_t dot_off = s.find('.');
         return (dot_off == std::string::npos) ? s :
@@ -576,11 +581,12 @@ datum_t make_time(double epoch_time, std::string tz) {
 }
 
 datum_t make_time(
+    reql_version_t reql_version,
     int year, int month, int day, int hours, int minutes, double seconds,
     std::string tz, const rcheckable_t *target) {
     try {
         ptime_t ptime(date_t(year, month, day), dur_t(hours, minutes, 0));
-        add_seconds_to_ptime(&ptime, seconds);
+        add_seconds_to_ptime(reql_version, &ptime, seconds);
         try {
             tz = sanitize::tz(tz);
         } catch (const datum_exc_t &e) {
@@ -629,9 +635,9 @@ datum_t time_sub(datum_t time, datum_t time_or_duration) {
     }
 }
 
-double time_portion(datum_t time, time_component_t c) {
+double time_portion(reql_version_t reql_version, datum_t time, time_component_t c) {
     try {
-        ptime_t ptime = time_to_boost(time).local_time();
+        ptime_t ptime = time_to_boost(reql_version, time).local_time();
         switch (c) {
         case YEAR: return ptime.date().year();
         case MONTH: return ptime.date().month();
@@ -661,15 +667,15 @@ time_t boost_date(time_t boost_time) {
     return time_t(ptime_t(d) - zone->base_utc_offset(), zone);
 }
 
-datum_t time_date(datum_t time, const rcheckable_t *target) {
+datum_t time_date(reql_version_t reql_version, datum_t time, const rcheckable_t *target) {
     try {
-        return boost_to_time(boost_date(time_to_boost(time)), target);
+        return boost_to_time(boost_date(time_to_boost(reql_version, time)), target);
     } HANDLE_BOOST_ERRORS(target);
 }
 
-datum_t time_of_day(datum_t time) {
+datum_t time_of_day(reql_version_t reql_version, datum_t time) {
     try {
-        time_t boost_time = time_to_boost(time);
+        time_t boost_time = time_to_boost(reql_version, time);
         double sec =
             (boost_time - boost_date(boost_time)).total_microseconds() / 1000000.0;
         sec = round(sec * 1000) / 1000;

src/rdb_protocol/pseudo_time.hpp

@@ -17,7 +17,7 @@ extern const char *const time_string;
 
 datum_t iso8601_to_time(
     const std::string &s, const std::string &default_tz, const rcheckable_t *t);
-std::string time_to_iso8601(datum_t d);
+std::string time_to_iso8601(reql_version_t reql_version, datum_t d);
 double time_to_epoch_time(datum_t d);
 
 datum_t time_now();
@@ -28,6 +28,7 @@ int time_cmp(const datum_t &x, const datum_t &y);
 void sanitize_time(datum_t *time);
 datum_t make_time(double epoch_time, std::string tz);
 datum_t make_time(
+    reql_version_t reql_version,
     int year, int month, int day, int hours, int minutes, double seconds,
     std::string tz, const rcheckable_t *target);
 datum_t time_add(
@@ -45,10 +46,10 @@ enum time_component_t {
     MINUTES,
     SECONDS
 };
-double time_portion(datum_t time, time_component_t c);
-datum_t time_date(datum_t time,
-                                   const rcheckable_t *target);
-datum_t time_of_day(datum_t time);
+double time_portion(reql_version_t reql_version, datum_t time, time_component_t c);
+datum_t time_date(reql_version_t reql_version, datum_t time,
+                  const rcheckable_t *target);
+datum_t time_of_day(reql_version_t reql_version, datum_t time);
 
 void time_to_str_key(const datum_t &d, std::string *str_out);
 

src/rdb_protocol/terms/time.cc

@@ -33,7 +33,9 @@ class to_iso8601_term_t : public op_term_t {
     scoped_ptr_t<val_t> eval_impl(scope_env_t *env, args_t *args, eval_flags_t) const {
         return new_val(
             datum_t(datum_string_t(
-                pseudo::time_to_iso8601(args->arg(env, 0)->as_ptype(pseudo::time_string)))));
+                pseudo::time_to_iso8601(
+                    env->env->reql_version(),
+                    args->arg(env, 0)->as_ptype(pseudo::time_string)))));
     }
     virtual const char *name() const { return "to_iso8601"; }
 };
@@ -112,7 +114,9 @@ class date_term_t : public op_term_t {
         : op_term_t(env, term, argspec_t(1)) { }
 private:
     scoped_ptr_t<val_t> eval_impl(scope_env_t *env, args_t *args, eval_flags_t) const {
-        return new_val(pseudo::time_date(args->arg(env, 0)->as_ptype(pseudo::time_string), this));
+        return new_val(pseudo::time_date(
+            env->env->reql_version(),
+            args->arg(env, 0)->as_ptype(pseudo::time_string), this));
     }
     virtual const char *name() const { return "date"; }
 };
@@ -123,7 +127,9 @@ class time_of_day_term_t : public op_term_t {
         : op_term_t(env, term, argspec_t(1)) { }
 private:
     scoped_ptr_t<val_t> eval_impl(scope_env_t *env, args_t *args, eval_flags_t) const {
-        return new_val(pseudo::time_of_day(args->arg(env, 0)->as_ptype(pseudo::time_string)));
+        return new_val(pseudo::time_of_day(
+            env->env->reql_version(),
+            args->arg(env, 0)->as_ptype(pseudo::time_string)));
     }
     virtual const char *name() const { return "time_of_day"; }
 };
@@ -146,7 +152,10 @@ class portion_term_t : public op_term_t {
         : op_term_t(env, term, argspec_t(1)), component(_component) { }
 private:
     scoped_ptr_t<val_t> eval_impl(scope_env_t *env, args_t *args, eval_flags_t) const {
-        double d = pseudo::time_portion(args->arg(env, 0)->as_ptype(pseudo::time_string), component);
+        double d = pseudo::time_portion(
+            env->env->reql_version(),
+            args->arg(env, 0)->as_ptype(pseudo::time_string),
+            component);
         return new_val(datum_t(d));
     }
     virtual const char *name() const {
@@ -192,7 +201,8 @@ class time_term_t : public op_term_t {
             r_sanity_check(false);
         }
         return new_val(
-            pseudo::make_time(year, month, day, hours, minutes, seconds, tz, this));
+            pseudo::make_time(env->env->reql_version(), year, month, day, hours, minutes,
+                              seconds, tz, this));
     }
     static std::string parse_tz(scoped_ptr_t<val_t> v) {
         return v->as_str().to_std();

test/rql_test/src/times/constructors.yaml

@@ -45,6 +45,17 @@ tests:
   - cd: r.epoch_time(1.4444445).seconds()
     ot: 1.444
 
+# Check for millisecond truncation bug instead of rounding, see issue 6272.
+  - cd: r.epoch_time(1.444).to_iso8601()
+    js: r.epochTime(1.444).toISO8601()
+    ot: "1970-01-01T00:00:01.444+00:00"
+  - cd: r.epoch_time(1.001).to_iso8601()
+    js: r.epochTime(1.001).toISO8601()
+    ot: "1970-01-01T00:00:01.001+00:00"
+  - cd: r.epoch_time(2.058).to_iso8601()
+    js: r.epochTime(2.058).toISO8601()
+    ot: "1970-01-01T00:00:02.058+00:00"
+
   - cd: r.epoch_time(253430000000).year()
     ot: 10000
   - cd: r.epoch_time(253430000000).to_iso8601()