refactor(common): make certain IntervalUnit constructors test-only (#…

…8464)

src/batch/src/executor/hop_window.rs

@@ -212,6 +212,7 @@ mod tests {
     use futures::stream::StreamExt;
     use risingwave_common::array::{DataChunk, DataChunkTestExt};
     use risingwave_common::catalog::{Field, Schema};
+    use risingwave_common::types::test_utils::IntervalUnitTestExt;
     use risingwave_common::types::DataType;
     use risingwave_expr::expr::test_utils::make_hop_window_expression;
 

src/common/src/array/arrow.rs

@@ -513,6 +513,7 @@ impl From<&arrow_array::StructArray> for StructArray {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::types::interval::test_utils::IntervalUnitTestExt;
     use crate::{array, empty_array};
 
     #[test]

src/common/src/array/interval_array.rs

@@ -23,6 +23,7 @@ mod tests {
     use super::IntervalArray;
     use crate::array::interval_array::{IntervalArrayBuilder, IntervalUnit};
     use crate::array::{Array, ArrayBuilder};
+    use crate::types::interval::test_utils::IntervalUnitTestExt;
 
     #[test]
     fn test_interval_array() {

src/common/src/types/interval.rs

@@ -109,46 +109,6 @@ impl IntervalUnit {
         }
     }
 
-    #[must_use]
-    pub fn from_ymd(year: i32, month: i32, days: i32) -> Self {
-        let months = year * 12 + month;
-        let days = days;
-        let ms = 0;
-        IntervalUnit { months, days, ms }
-    }
-
-    #[must_use]
-    pub fn from_month(months: i32) -> Self {
-        IntervalUnit {
-            months,
-            ..Default::default()
-        }
-    }
-
-    #[must_use]
-    pub fn from_days(days: i32) -> Self {
-        Self {
-            days,
-            ..Default::default()
-        }
-    }
-
-    #[must_use]
-    pub fn from_millis(ms: i64) -> Self {
-        Self {
-            ms,
-            ..Default::default()
-        }
-    }
-
-    #[must_use]
-    pub fn from_minutes(minutes: i64) -> Self {
-        Self {
-            ms: 1000 * 60 * minutes,
-            ..Default::default()
-        }
-    }
-
     pub fn to_protobuf<T: Write>(self, output: &mut T) -> ArrayResult<usize> {
         output.write_i32::<BigEndian>(self.months)?;
         output.write_i32::<BigEndian>(self.days)?;
@@ -435,6 +395,63 @@ impl IntervalUnit {
     }
 }
 
+/// A separate mod so that `use types::*` or `use interval::*` does not `use IntervalUnitTestExt` by
+/// accident.
+pub mod test_utils {
+    use super::*;
+
+    /// These constructors may panic when value out of bound. Only use in tests with known input.
+    pub trait IntervalUnitTestExt {
+        fn from_ymd(year: i32, month: i32, days: i32) -> Self;
+        fn from_month(months: i32) -> Self;
+        fn from_days(days: i32) -> Self;
+        fn from_millis(ms: i64) -> Self;
+        fn from_minutes(minutes: i64) -> Self;
+    }
+
+    impl IntervalUnitTestExt for IntervalUnit {
+        #[must_use]
+        fn from_ymd(year: i32, month: i32, days: i32) -> Self {
+            let months = year * 12 + month;
+            let days = days;
+            let ms = 0;
+            IntervalUnit { months, days, ms }
+        }
+
+        #[must_use]
+        fn from_month(months: i32) -> Self {
+            IntervalUnit {
+                months,
+                ..Default::default()
+            }
+        }
+
+        #[must_use]
+        fn from_days(days: i32) -> Self {
+            Self {
+                days,
+                ..Default::default()
+            }
+        }
+
+        #[must_use]
+        fn from_millis(ms: i64) -> Self {
+            Self {
+                ms,
+                ..Default::default()
+            }
+        }
+
+        #[must_use]
+        fn from_minutes(minutes: i64) -> Self {
+            Self {
+                ms: 1000 * 60 * minutes,
+                ..Default::default()
+            }
+        }
+    }
+}
+
 /// Wrapper so that `Debug for IntervalUnitDisplay` would use the concise format of `Display for
 /// IntervalUnit`.
 #[derive(Clone, Copy)]
@@ -926,21 +943,21 @@ impl IntervalUnit {
         (|| match leading_field {
             Year => {
                 let months = num.checked_mul(12)?;
-                Some(IntervalUnit::from_month(months as i32))
+                Some(IntervalUnit::new(months as i32, 0, 0))
             }
-            Month => Some(IntervalUnit::from_month(num as i32)),
-            Day => Some(IntervalUnit::from_days(num as i32)),
+            Month => Some(IntervalUnit::new(num as i32, 0, 0)),
+            Day => Some(IntervalUnit::new(0, num as i32, 0)),
             Hour => {
                 let ms = num.checked_mul(3600 * 1000)?;
-                Some(IntervalUnit::from_millis(ms))
+                Some(IntervalUnit::new(0, 0, ms))
             }
             Minute => {
                 let ms = num.checked_mul(60 * 1000)?;
-                Some(IntervalUnit::from_millis(ms))
+                Some(IntervalUnit::new(0, 0, ms))
             }
             Second => {
                 let ms = num.checked_mul(1000)?;
-                Some(IntervalUnit::from_millis(ms))
+                Some(IntervalUnit::new(0, 0, ms))
             }
         })()
         .ok_or_else(|| ErrorCode::InvalidInputSyntax(format!("Invalid interval {}.", s)).into())
@@ -963,21 +980,21 @@ impl IntervalUnit {
                     result = result + (|| match interval_unit {
                         Year => {
                             let months = num.checked_mul(12)?;
-                            Some(IntervalUnit::from_month(months as i32))
+                            Some(IntervalUnit::new(months as i32, 0, 0))
                         }
-                        Month => Some(IntervalUnit::from_month(num as i32)),
-                        Day => Some(IntervalUnit::from_days(num as i32)),
+                        Month => Some(IntervalUnit::new(num as i32, 0, 0)),
+                        Day => Some(IntervalUnit::new(0, num as i32, 0)),
                         Hour => {
                             let ms = num.checked_mul(3600 * 1000)?;
-                            Some(IntervalUnit::from_millis(ms))
+                            Some(IntervalUnit::new(0, 0, ms))
                         }
                         Minute => {
                             let ms = num.checked_mul(60 * 1000)?;
-                            Some(IntervalUnit::from_millis(ms))
+                            Some(IntervalUnit::new(0, 0, ms))
                         }
                         Second => {
                             let ms = num.checked_mul(1000)?;
-                            Some(IntervalUnit::from_millis(ms))
+                            Some(IntervalUnit::new(0, 0, ms))
                         }
                     })()
                     .ok_or_else(|| ErrorCode::InvalidInputSyntax(format!("Invalid interval {}.", s)))?;
@@ -989,7 +1006,7 @@ impl IntervalUnit {
                             // If unsatisfied precision is passed as input, we should not return None (Error).
                             // TODO: IntervalUnit only support millisecond precision so the part smaller than millisecond will be truncated.
                             let ms = (second.into_inner() * 1000_f64).round() as i64;
-                            Some(IntervalUnit::from_millis(ms))
+                            Some(IntervalUnit::new(0, 0, ms))
                         }
                         _ => None,
                     }
@@ -1022,6 +1039,8 @@ impl FromStr for IntervalUnit {
 
 #[cfg(test)]
 mod tests {
+    use interval::test_utils::IntervalUnitTestExt;
+
     use super::*;
     use crate::types::ordered_float::OrderedFloat;
 

src/expr/benches/expr.rs

@@ -21,6 +21,7 @@
 
 use criterion::{criterion_group, criterion_main, Criterion};
 use risingwave_common::array::*;
+use risingwave_common::types::test_utils::IntervalUnitTestExt;
 use risingwave_common::types::{
     DataType, DataTypeName, Decimal, IntervalUnit, NaiveDateTimeWrapper, NaiveDateWrapper,
     NaiveTimeWrapper, OrderedF32, OrderedF64,

src/expr/src/expr/expr_binary_nonnull.rs

@@ -842,6 +842,7 @@ fn boolean_le(l: &BoolArray, r: &BoolArray) -> BoolArray {
 mod tests {
     use risingwave_common::array::interval_array::IntervalArray;
     use risingwave_common::array::*;
+    use risingwave_common::types::test_utils::IntervalUnitTestExt;
     use risingwave_common::types::{
         Decimal, IntervalUnit, NaiveDateTimeWrapper, NaiveDateWrapper, Scalar,
     };

src/expr/src/expr/expr_literal.rs

@@ -120,6 +120,7 @@ impl<'a> TryFrom<&'a ExprNode> for LiteralExpression {
 mod tests {
     use risingwave_common::array::{I32Array, StructValue};
     use risingwave_common::array_nonnull;
+    use risingwave_common::types::test_utils::IntervalUnitTestExt;
     use risingwave_common::types::{Decimal, IntervalUnit, IntoOrdered};
     use risingwave_common::util::value_encoding::serialize_datum;
     use risingwave_pb::data::data_type::{IntervalType, TypeName};

src/expr/src/table_function/generate_series.rs

@@ -179,6 +179,7 @@ pub fn new_generate_series<const STOP_INCLUSIVE: bool>(
 
 #[cfg(test)]
 mod tests {
+    use risingwave_common::types::test_utils::IntervalUnitTestExt;
     use risingwave_common::types::{DataType, IntervalUnit, NaiveDateTimeWrapper, ScalarImpl};
 
     use super::*;

src/expr/src/vector_op/tests.rs

@@ -16,6 +16,7 @@ use std::assert_matches::assert_matches;
 use std::str::FromStr;
 
 use chrono::NaiveDateTime;
+use risingwave_common::types::test_utils::IntervalUnitTestExt;
 use risingwave_common::types::{
     Decimal, IntervalUnit, NaiveDateTimeWrapper, NaiveDateWrapper, OrderedF32, OrderedF64,
 };

src/frontend/src/binder/expr/value.rs

@@ -292,6 +292,7 @@ fn unescape_c_style(s: &str) -> Result<String> {
 
 #[cfg(test)]
 mod tests {
+    use risingwave_common::types::test_utils::IntervalUnitTestExt;
     use risingwave_common::types::DataType;
     use risingwave_expr::expr::build_from_prost;
     use risingwave_sqlparser::ast::Value::Number;

src/stream/src/executor/hop_window.rs

@@ -170,6 +170,7 @@ mod tests {
     use futures::StreamExt;
     use risingwave_common::array::stream_chunk::StreamChunkTestExt;
     use risingwave_common::catalog::{Field, Schema};
+    use risingwave_common::types::test_utils::IntervalUnitTestExt;
     use risingwave_common::types::{DataType, IntervalUnit};
     use risingwave_expr::expr::test_utils::make_hop_window_expression;