From b928afc65807b143195ced852d71cf54c8993bb3 Mon Sep 17 00:00:00 2001 From: Xiangjin Date: Thu, 9 Mar 2023 20:19:46 +0800 Subject: [PATCH 1/2] separate --- src/common/src/types/interval.rs | 82 ++++++++++++++++---------------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/src/common/src/types/interval.rs b/src/common/src/types/interval.rs index aa3944af5cac8..60fa5caa2d62a 100644 --- a/src/common/src/types/interval.rs +++ b/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(self, output: &mut T) -> ArrayResult { output.write_i32::(self.months)?; output.write_i32::(self.days)?; @@ -435,6 +395,48 @@ impl IntervalUnit { } } +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() + } + } +} + /// Wrapper so that `Debug for IntervalUnitDisplay` would use the concise format of `Display for /// IntervalUnit`. #[derive(Clone, Copy)] From 0d1939c3e1e75b910c3abc8c20acca8728b7180d Mon Sep 17 00:00:00 2001 From: Xiangjin Date: Thu, 9 Mar 2023 22:03:04 +0800 Subject: [PATCH 2/2] test_utils::IntervalUnitTestExt --- src/batch/src/executor/hop_window.rs | 1 + src/common/src/array/arrow.rs | 1 + src/common/src/array/interval_array.rs | 1 + src/common/src/types/interval.rs | 105 ++++++++++-------- src/expr/benches/expr.rs | 1 + src/expr/src/expr/expr_binary_nonnull.rs | 1 + src/expr/src/expr/expr_literal.rs | 1 + .../src/table_function/generate_series.rs | 1 + src/expr/src/vector_op/tests.rs | 1 + src/frontend/src/binder/expr/value.rs | 1 + src/stream/src/executor/hop_window.rs | 1 + 11 files changed, 71 insertions(+), 44 deletions(-) diff --git a/src/batch/src/executor/hop_window.rs b/src/batch/src/executor/hop_window.rs index e277dcd5bf43c..3fcbb6ccf45ac 100644 --- a/src/batch/src/executor/hop_window.rs +++ b/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; diff --git a/src/common/src/array/arrow.rs b/src/common/src/array/arrow.rs index 7fb068252b4f6..851bb93df2077 100644 --- a/src/common/src/array/arrow.rs +++ b/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] diff --git a/src/common/src/array/interval_array.rs b/src/common/src/array/interval_array.rs index 9ded3f9a3da68..d7a7336f2c1ea 100644 --- a/src/common/src/array/interval_array.rs +++ b/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() { diff --git a/src/common/src/types/interval.rs b/src/common/src/types/interval.rs index 60fa5caa2d62a..b42a27e3e86c3 100644 --- a/src/common/src/types/interval.rs +++ b/src/common/src/types/interval.rs @@ -395,44 +395,59 @@ impl IntervalUnit { } } -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 } - } +/// A separate mod so that `use types::*` or `use interval::*` does not `use IntervalUnitTestExt` by +/// accident. +pub mod test_utils { + use super::*; - #[must_use] - pub fn from_month(months: i32) -> Self { - IntervalUnit { - months, - ..Default::default() + /// 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] - pub fn from_days(days: i32) -> Self { - Self { - days, - ..Default::default() + #[must_use] + fn from_month(months: i32) -> Self { + IntervalUnit { + months, + ..Default::default() + } } - } - #[must_use] - pub fn from_millis(ms: i64) -> Self { - Self { - ms, - ..Default::default() + #[must_use] + fn from_days(days: i32) -> Self { + Self { + days, + ..Default::default() + } } - } - #[must_use] - pub fn from_minutes(minutes: i64) -> Self { - Self { - ms: 1000 * 60 * minutes, - ..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() + } } } } @@ -928,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()) @@ -965,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)))?; @@ -991,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, } @@ -1024,6 +1039,8 @@ impl FromStr for IntervalUnit { #[cfg(test)] mod tests { + use interval::test_utils::IntervalUnitTestExt; + use super::*; use crate::types::ordered_float::OrderedFloat; diff --git a/src/expr/benches/expr.rs b/src/expr/benches/expr.rs index 774c1af4093fe..2f43a37ecf681 100644 --- a/src/expr/benches/expr.rs +++ b/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, diff --git a/src/expr/src/expr/expr_binary_nonnull.rs b/src/expr/src/expr/expr_binary_nonnull.rs index 7b302ce354fa8..4271136a288d1 100644 --- a/src/expr/src/expr/expr_binary_nonnull.rs +++ b/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, }; diff --git a/src/expr/src/expr/expr_literal.rs b/src/expr/src/expr/expr_literal.rs index 40d8b21083f5f..da4097e9b2f62 100644 --- a/src/expr/src/expr/expr_literal.rs +++ b/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}; diff --git a/src/expr/src/table_function/generate_series.rs b/src/expr/src/table_function/generate_series.rs index f6555ef2ad3e0..5c7303a6b2c0e 100644 --- a/src/expr/src/table_function/generate_series.rs +++ b/src/expr/src/table_function/generate_series.rs @@ -179,6 +179,7 @@ pub fn new_generate_series( #[cfg(test)] mod tests { + use risingwave_common::types::test_utils::IntervalUnitTestExt; use risingwave_common::types::{DataType, IntervalUnit, NaiveDateTimeWrapper, ScalarImpl}; use super::*; diff --git a/src/expr/src/vector_op/tests.rs b/src/expr/src/vector_op/tests.rs index e4eca3187f671..35d146b189ee1 100644 --- a/src/expr/src/vector_op/tests.rs +++ b/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, }; diff --git a/src/frontend/src/binder/expr/value.rs b/src/frontend/src/binder/expr/value.rs index d2ec9e1daf504..7b920ef07fb95 100644 --- a/src/frontend/src/binder/expr/value.rs +++ b/src/frontend/src/binder/expr/value.rs @@ -292,6 +292,7 @@ fn unescape_c_style(s: &str) -> Result { #[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; diff --git a/src/stream/src/executor/hop_window.rs b/src/stream/src/executor/hop_window.rs index 527c8df1598e7..c987f2a78cba8 100644 --- a/src/stream/src/executor/hop_window.rs +++ b/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;