fix(common): interval should have microsecond precision (#8501)

risingwavelabs · Mar 13, 2023 · cdaa8cf · cdaa8cf
1 parent b235f68
commit cdaa8cf
Show file tree

Hide file tree

Showing 21 changed files with 258 additions and 191 deletions.
diff --git a/dashboard/proto/gen/data.ts b/dashboard/proto/gen/data.ts
diff --git a/e2e_test/batch/types/interval.slt.part b/e2e_test/batch/types/interval.slt.part
@@ -109,8 +109,13 @@ select interval '1 year 1 month 1 day 1:1:1.009';
 ----
 1 year 1 mon 1 day 01:01:01.009
 
-# issue#7059, if we improve precision, then this should be removed.
-query TTTTTT
+# issue #7059
+query T
 select '1 mons 1 days 00:00:00.000001'::INTERVAL;
 ----
+1 mon 1 day 00:00:00.000001
+
+query T
+select '1 mons 1 days 00:00:00.0000001'::INTERVAL;
+----
 1 mon 1 day
diff --git a/e2e_test/batch/types/temporal_arithmetic.slt.part b/e2e_test/batch/types/temporal_arithmetic.slt.part
@@ -66,7 +66,7 @@ select interval '20' / float '12.5';
 query T
 select interval '12 days' / 4.2;
 ----
-2 days 20:34:17.143
+2 days 20:34:17.142857
 
 query T
 SELECT interval '1 month' / 2000;
@@ -136,7 +136,7 @@ select real '6.1' * interval '1' second;
 query T
 select real '61.1' * interval '1' second;
 ----
-00:01:01.1
+00:01:01.099998
 
 query T
 select real '0.0' * interval '1' second;
@@ -161,7 +161,7 @@ select interval '1' second * real '6.1';
 query T
 select interval '1' second * real '61.1';
 ----
-00:01:01.1
+00:01:01.099998
 
 query T
 select interval '1' second * real '0.0';

diff --git a/proto/data.proto b/proto/data.proto
@@ -10,7 +10,7 @@ option optimize_for = SPEED;
 message IntervalUnit {
   int32 months = 1;
   int32 days = 2;
-  int64 ms = 3;
+  int64 usecs = 3;
 }
 
 message DataType {

diff --git a/src/batch/src/executor/order_by.rs b/src/batch/src/executor/order_by.rs
@@ -464,9 +464,9 @@ mod tests {
                 None] },
                 column! { IntervalArray, [
                 None,
-                Some(IntervalUnit::new(1, 2, 3)),
+                Some(IntervalUnit::from_month_day_usec(1, 2, 3)),
                 None,
-                Some(IntervalUnit::new(4, 5, 6)),
+                Some(IntervalUnit::from_month_day_usec(4, 5, 6)),
                 None] },
             ],
             5,
@@ -491,8 +491,8 @@ mod tests {
                 Some(NaiveDateTimeWrapper::with_secs_nsecs(1, 23).unwrap()),
                 Some(NaiveDateTimeWrapper::with_secs_nsecs(7, 89).unwrap())] },
                 column! { IntervalArray, [
-                Some(IntervalUnit::new(4, 5, 6)),
-                Some(IntervalUnit::new(1, 2, 3)),
+                Some(IntervalUnit::from_month_day_usec(4, 5, 6)),
+                Some(IntervalUnit::from_month_day_usec(1, 2, 3)),
                 None,
                 None,
                 None] },

diff --git a/src/common/src/array/arrow.rs b/src/common/src/array/arrow.rs
@@ -348,14 +348,15 @@ impl FromIntoArrow for IntervalUnit {
 
     fn from_arrow(value: Self::ArrowType) -> Self {
         let (months, days, ns) = arrow_array::types::IntervalMonthDayNanoType::to_parts(value);
-        IntervalUnit::new(months, days, ns / 1_000_000)
+        IntervalUnit::from_month_day_usec(months, days, ns / 1000)
     }
 
     fn into_arrow(self) -> Self::ArrowType {
         arrow_array::types::IntervalMonthDayNanoType::make_value(
             self.get_months(),
             self.get_days(),
-            self.get_ms() * 1_000_000,
+            // TODO: this may overflow and we need `try_into`
+            self.get_usecs() * 1000,
         )
     }
 }

diff --git a/src/common/src/array/column_proto_readers.rs b/src/common/src/array/column_proto_readers.rs
@@ -101,9 +101,9 @@ pub fn read_interval_unit(cursor: &mut Cursor<&[u8]>) -> ArrayResult<IntervalUni
     let mut read = || {
         let months = cursor.read_i32::<BigEndian>()?;
         let days = cursor.read_i32::<BigEndian>()?;
-        let ms = cursor.read_i64::<BigEndian>()?;
+        let usecs = cursor.read_i64::<BigEndian>()?;
 
-        Ok::<_, std::io::Error>(IntervalUnit::new(months, days, ms))
+        Ok::<_, std::io::Error>(IntervalUnit::from_month_day_usec(months, days, usecs))
     };
 
     match read() {

diff --git a/src/common/src/hash/key.rs b/src/common/src/hash/key.rs
@@ -365,14 +365,14 @@ impl HashKeySerDe<'_> for IntervalUnit {
         let mut ret = [0; 16];
         ret[0..4].copy_from_slice(&self.get_months().to_ne_bytes());
         ret[4..8].copy_from_slice(&self.get_days().to_ne_bytes());
-        ret[8..16].copy_from_slice(&self.get_ms().to_ne_bytes());
+        ret[8..16].copy_from_slice(&self.get_usecs().to_ne_bytes());
 
         ret
     }
 
     fn deserialize<R: Read>(source: &mut R) -> Self {
         let value = Self::read_fixed_size_bytes::<R, 16>(source);
-        IntervalUnit::new(
+        IntervalUnit::from_month_day_usec(
             i32::from_ne_bytes(value[0..4].try_into().unwrap()),
             i32::from_ne_bytes(value[4..8].try_into().unwrap()),
             i64::from_ne_bytes(value[8..16].try_into().unwrap()),

diff --git a/src/common/src/row/owned_row.rs b/src/common/src/row/owned_row.rs
@@ -228,7 +228,9 @@ mod tests {
             Some(ScalarImpl::Float32(4.0.into())),
             Some(ScalarImpl::Float64(5.0.into())),
             Some(ScalarImpl::Decimal("-233.3".parse().unwrap())),
-            Some(ScalarImpl::Interval(IntervalUnit::new(7, 8, 9))),
+            Some(ScalarImpl::Interval(IntervalUnit::from_month_day_usec(
+                7, 8, 9,
+            ))),
         ]);
         let value_indices = (0..9).collect_vec();
         let bytes = (&row).project(&value_indices).value_serialize();
@@ -261,10 +263,14 @@ mod tests {
             Some(ScalarImpl::Float32(4.0.into())),
             Some(ScalarImpl::Float64(5.0.into())),
             Some(ScalarImpl::Decimal("-233.3".parse().unwrap())),
-            Some(ScalarImpl::Interval(IntervalUnit::new(7, 8, 9))),
+            Some(ScalarImpl::Interval(IntervalUnit::from_month_day_usec(
+                7, 8, 9,
+            ))),
         ]);
         let row2 = OwnedRow::new(vec![
-            Some(ScalarImpl::Interval(IntervalUnit::new(7, 8, 9))),
+            Some(ScalarImpl::Interval(IntervalUnit::from_month_day_usec(
+                7, 8, 9,
+            ))),
             Some(ScalarImpl::Utf8("string".into())),
             Some(ScalarImpl::Bool(true)),
             Some(ScalarImpl::Int16(1)),

diff --git a/src/common/src/test_utils/rand_array.rs b/src/common/src/test_utils/rand_array.rs
@@ -78,8 +78,8 @@ impl RandValue for IntervalUnit {
     fn rand_value<R: Rng>(rand: &mut R) -> Self {
         let months = rand.gen_range(0..100);
         let days = rand.gen_range(0..200);
-        let ms = rand.gen_range(0..100_000);
-        IntervalUnit::new(months, days, ms)
+        let usecs = rand.gen_range(0..100_000);
+        IntervalUnit::from_month_day_usec(months, days, usecs)
     }
 }
 

diff --git a/src/common/src/types/chrono_wrapper.rs b/src/common/src/types/chrono_wrapper.rs
@@ -565,7 +565,7 @@ impl CheckedAdd<IntervalUnit> for NaiveDateTimeWrapper {
         }
         let mut datetime = NaiveDateTime::new(date, self.0.time());
         datetime = datetime.checked_add_signed(Duration::days(rhs.get_days().into()))?;
-        datetime = datetime.checked_add_signed(Duration::milliseconds(rhs.get_ms()))?;
+        datetime = datetime.checked_add_signed(Duration::microseconds(rhs.get_usecs()))?;
 
         Some(NaiveDateTimeWrapper::new(datetime))
     }