Skip to content

Commit 2f626d9

Browse files
xiangjinwuxiangjinwu
authored
refactor(common): make certain IntervalUnit constructors test-only (risingwavelabs#8464)
1 parent 86188ef commit 2f626d9

File tree

11 files changed

+82
-53
lines changed

11 files changed

+82
-53
lines changed

‎src/batch/src/executor/hop_window.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ mod tests {
212212
use futures::stream::StreamExt;
213213
use risingwave_common::array::{DataChunk, DataChunkTestExt};
214214
use risingwave_common::catalog::{Field, Schema};
215+
use risingwave_common::types::test_utils::IntervalUnitTestExt;
215216
use risingwave_common::types::DataType;
216217
use risingwave_expr::expr::test_utils::make_hop_window_expression;
217218

‎src/common/src/array/arrow.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,7 @@ impl From<&arrow_array::StructArray> for StructArray {
513513
#[cfg(test)]
514514
mod tests {
515515
use super::*;
516+
use crate::types::interval::test_utils::IntervalUnitTestExt;
516517
use crate::{array, empty_array};
517518

518519
#[test]

‎src/common/src/array/interval_array.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ mod tests {
2323
use super::IntervalArray;
2424
use crate::array::interval_array::{IntervalArrayBuilder, IntervalUnit};
2525
use crate::array::{Array, ArrayBuilder};
26+
use crate::types::interval::test_utils::IntervalUnitTestExt;
2627

2728
#[test]
2829
fn test_interval_array() {

‎src/common/src/types/interval.rs

Lines changed: 72 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -109,46 +109,6 @@ impl IntervalUnit {
109109
}
110110
}
111111

112-
#[must_use]
113-
pub fn from_ymd(year: i32, month: i32, days: i32) -> Self {
114-
let months = year * 12 + month;
115-
let days = days;
116-
let ms = 0;
117-
IntervalUnit { months, days, ms }
118-
}
119-
120-
#[must_use]
121-
pub fn from_month(months: i32) -> Self {
122-
IntervalUnit {
123-
months,
124-
..Default::default()
125-
}
126-
}
127-
128-
#[must_use]
129-
pub fn from_days(days: i32) -> Self {
130-
Self {
131-
days,
132-
..Default::default()
133-
}
134-
}
135-
136-
#[must_use]
137-
pub fn from_millis(ms: i64) -> Self {
138-
Self {
139-
ms,
140-
..Default::default()
141-
}
142-
}
143-
144-
#[must_use]
145-
pub fn from_minutes(minutes: i64) -> Self {
146-
Self {
147-
ms: 1000 * 60 * minutes,
148-
..Default::default()
149-
}
150-
}
151-
152112
pub fn to_protobuf<T: Write>(self, output: &mut T) -> ArrayResult<usize> {
153113
output.write_i32::<BigEndian>(self.months)?;
154114
output.write_i32::<BigEndian>(self.days)?;
@@ -435,6 +395,63 @@ impl IntervalUnit {
435395
}
436396
}
437397

398+
/// A separate mod so that `use types::*` or `use interval::*` does not `use IntervalUnitTestExt` by
399+
/// accident.
400+
pub mod test_utils {
401+
use super::*;
402+
403+
/// These constructors may panic when value out of bound. Only use in tests with known input.
404+
pub trait IntervalUnitTestExt {
405+
fn from_ymd(year: i32, month: i32, days: i32) -> Self;
406+
fn from_month(months: i32) -> Self;
407+
fn from_days(days: i32) -> Self;
408+
fn from_millis(ms: i64) -> Self;
409+
fn from_minutes(minutes: i64) -> Self;
410+
}
411+
412+
impl IntervalUnitTestExt for IntervalUnit {
413+
#[must_use]
414+
fn from_ymd(year: i32, month: i32, days: i32) -> Self {
415+
let months = year * 12 + month;
416+
let days = days;
417+
let ms = 0;
418+
IntervalUnit { months, days, ms }
419+
}
420+
421+
#[must_use]
422+
fn from_month(months: i32) -> Self {
423+
IntervalUnit {
424+
months,
425+
..Default::default()
426+
}
427+
}
428+
429+
#[must_use]
430+
fn from_days(days: i32) -> Self {
431+
Self {
432+
days,
433+
..Default::default()
434+
}
435+
}
436+
437+
#[must_use]
438+
fn from_millis(ms: i64) -> Self {
439+
Self {
440+
ms,
441+
..Default::default()
442+
}
443+
}
444+
445+
#[must_use]
446+
fn from_minutes(minutes: i64) -> Self {
447+
Self {
448+
ms: 1000 * 60 * minutes,
449+
..Default::default()
450+
}
451+
}
452+
}
453+
}
454+
438455
/// Wrapper so that `Debug for IntervalUnitDisplay` would use the concise format of `Display for
439456
/// IntervalUnit`.
440457
#[derive(Clone, Copy)]
@@ -926,21 +943,21 @@ impl IntervalUnit {
926943
(|| match leading_field {
927944
Year => {
928945
let months = num.checked_mul(12)?;
929-
Some(IntervalUnit::from_month(months as i32))
946+
Some(IntervalUnit::new(months as i32, 0, 0))
930947
}
931-
Month => Some(IntervalUnit::from_month(num as i32)),
932-
Day => Some(IntervalUnit::from_days(num as i32)),
948+
Month => Some(IntervalUnit::new(num as i32, 0, 0)),
949+
Day => Some(IntervalUnit::new(0, num as i32, 0)),
933950
Hour => {
934951
let ms = num.checked_mul(3600 * 1000)?;
935-
Some(IntervalUnit::from_millis(ms))
952+
Some(IntervalUnit::new(0, 0, ms))
936953
}
937954
Minute => {
938955
let ms = num.checked_mul(60 * 1000)?;
939-
Some(IntervalUnit::from_millis(ms))
956+
Some(IntervalUnit::new(0, 0, ms))
940957
}
941958
Second => {
942959
let ms = num.checked_mul(1000)?;
943-
Some(IntervalUnit::from_millis(ms))
960+
Some(IntervalUnit::new(0, 0, ms))
944961
}
945962
})()
946963
.ok_or_else(|| ErrorCode::InvalidInputSyntax(format!("Invalid interval {}.", s)).into())
@@ -963,21 +980,21 @@ impl IntervalUnit {
963980
result = result + (|| match interval_unit {
964981
Year => {
965982
let months = num.checked_mul(12)?;
966-
Some(IntervalUnit::from_month(months as i32))
983+
Some(IntervalUnit::new(months as i32, 0, 0))
967984
}
968-
Month => Some(IntervalUnit::from_month(num as i32)),
969-
Day => Some(IntervalUnit::from_days(num as i32)),
985+
Month => Some(IntervalUnit::new(num as i32, 0, 0)),
986+
Day => Some(IntervalUnit::new(0, num as i32, 0)),
970987
Hour => {
971988
let ms = num.checked_mul(3600 * 1000)?;
972-
Some(IntervalUnit::from_millis(ms))
989+
Some(IntervalUnit::new(0, 0, ms))
973990
}
974991
Minute => {
975992
let ms = num.checked_mul(60 * 1000)?;
976-
Some(IntervalUnit::from_millis(ms))
993+
Some(IntervalUnit::new(0, 0, ms))
977994
}
978995
Second => {
979996
let ms = num.checked_mul(1000)?;
980-
Some(IntervalUnit::from_millis(ms))
997+
Some(IntervalUnit::new(0, 0, ms))
981998
}
982999
})()
9831000
.ok_or_else(|| ErrorCode::InvalidInputSyntax(format!("Invalid interval {}.", s)))?;
@@ -989,7 +1006,7 @@ impl IntervalUnit {
9891006
// If unsatisfied precision is passed as input, we should not return None (Error).
9901007
// TODO: IntervalUnit only support millisecond precision so the part smaller than millisecond will be truncated.
9911008
let ms = (second.into_inner() * 1000_f64).round() as i64;
992-
Some(IntervalUnit::from_millis(ms))
1009+
Some(IntervalUnit::new(0, 0, ms))
9931010
}
9941011
_ => None,
9951012
}
@@ -1022,6 +1039,8 @@ impl FromStr for IntervalUnit {
10221039

10231040
#[cfg(test)]
10241041
mod tests {
1042+
use interval::test_utils::IntervalUnitTestExt;
1043+
10251044
use super::*;
10261045
use crate::types::ordered_float::OrderedFloat;
10271046

‎src/expr/benches/expr.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
use criterion::{criterion_group, criterion_main, Criterion};
2323
use risingwave_common::array::*;
24+
use risingwave_common::types::test_utils::IntervalUnitTestExt;
2425
use risingwave_common::types::{
2526
DataType, DataTypeName, Decimal, IntervalUnit, NaiveDateTimeWrapper, NaiveDateWrapper,
2627
NaiveTimeWrapper, OrderedF32, OrderedF64,

‎src/expr/src/expr/expr_binary_nonnull.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,7 @@ fn boolean_le(l: &BoolArray, r: &BoolArray) -> BoolArray {
842842
mod tests {
843843
use risingwave_common::array::interval_array::IntervalArray;
844844
use risingwave_common::array::*;
845+
use risingwave_common::types::test_utils::IntervalUnitTestExt;
845846
use risingwave_common::types::{
846847
Decimal, IntervalUnit, NaiveDateTimeWrapper, NaiveDateWrapper, Scalar,
847848
};

‎src/expr/src/expr/expr_literal.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ impl<'a> TryFrom<&'a ExprNode> for LiteralExpression {
120120
mod tests {
121121
use risingwave_common::array::{I32Array, StructValue};
122122
use risingwave_common::array_nonnull;
123+
use risingwave_common::types::test_utils::IntervalUnitTestExt;
123124
use risingwave_common::types::{Decimal, IntervalUnit, IntoOrdered};
124125
use risingwave_common::util::value_encoding::serialize_datum;
125126
use risingwave_pb::data::data_type::{IntervalType, TypeName};

‎src/expr/src/table_function/generate_series.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,7 @@ pub fn new_generate_series<const STOP_INCLUSIVE: bool>(
179179

180180
#[cfg(test)]
181181
mod tests {
182+
use risingwave_common::types::test_utils::IntervalUnitTestExt;
182183
use risingwave_common::types::{DataType, IntervalUnit, NaiveDateTimeWrapper, ScalarImpl};
183184

184185
use super::*;

‎src/expr/src/vector_op/tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use std::assert_matches::assert_matches;
1616
use std::str::FromStr;
1717

1818
use chrono::NaiveDateTime;
19+
use risingwave_common::types::test_utils::IntervalUnitTestExt;
1920
use risingwave_common::types::{
2021
Decimal, IntervalUnit, NaiveDateTimeWrapper, NaiveDateWrapper, OrderedF32, OrderedF64,
2122
};

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ fn unescape_c_style(s: &str) -> Result<String> {
292292

293293
#[cfg(test)]
294294
mod tests {
295+
use risingwave_common::types::test_utils::IntervalUnitTestExt;
295296
use risingwave_common::types::DataType;
296297
use risingwave_expr::expr::build_from_prost;
297298
use risingwave_sqlparser::ast::Value::Number;

‎src/stream/src/executor/hop_window.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ mod tests {
170170
use futures::StreamExt;
171171
use risingwave_common::array::stream_chunk::StreamChunkTestExt;
172172
use risingwave_common::catalog::{Field, Schema};
173+
use risingwave_common::types::test_utils::IntervalUnitTestExt;
173174
use risingwave_common::types::{DataType, IntervalUnit};
174175
use risingwave_expr::expr::test_utils::make_hop_window_expression;
175176

0 commit comments

Comments
 (0)