From ce139c8a232f1be43c2998f372a2b5d17732db83 Mon Sep 17 00:00:00 2001 From: "Lei, Huang" <6406592+v0y4g3r@users.noreply.github.com> Date: Fri, 26 Aug 2022 10:59:23 +0800 Subject: [PATCH] fix: impl scalar value helper and remove range limit (#205) * fix: impl scalar value helper function for DateTime * remove range limit for date * remove range limit for date --- src/common/time/src/date.rs | 44 +++++++-------------------- src/common/time/src/datetime.rs | 41 +++++-------------------- src/common/time/src/error.rs | 8 +---- src/datatypes/src/scalars.rs | 10 +++--- src/datatypes/src/value.rs | 4 +-- src/datatypes/src/vectors/builder.rs | 14 ++++----- src/datatypes/src/vectors/date.rs | 31 ++++++++----------- src/datatypes/src/vectors/datetime.rs | 36 +++++++++++----------- src/datatypes/src/vectors/helper.rs | 16 +++++++++- 9 files changed, 78 insertions(+), 126 deletions(-) diff --git a/src/common/time/src/date.rs b/src/common/time/src/date.rs index deb4893c7e..0f1beb735d 100644 --- a/src/common/time/src/date.rs +++ b/src/common/time/src/date.rs @@ -4,17 +4,15 @@ use std::str::FromStr; use chrono::{Datelike, NaiveDate}; use serde::{Deserialize, Serialize}; use serde_json::Value; -use snafu::{ensure, ResultExt}; +use snafu::ResultExt; use crate::error::Result; -use crate::error::{DateOverflowSnafu, Error, ParseDateStrSnafu}; +use crate::error::{Error, ParseDateStrSnafu}; const UNIX_EPOCH_FROM_CE: i32 = 719_163; /// ISO 8601 [Date] values. The inner representation is a signed 32 bit integer that represents the /// **days since "1970-01-01 00:00:00 UTC" (UNIX Epoch)**. -/// -/// [Date] value ranges between "0000-01-01" to "9999-12-31". #[derive( Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default, Deserialize, Serialize, )] @@ -44,23 +42,13 @@ impl Display for Date { } impl Date { - pub fn try_new(val: i32) -> Result { - ensure!( - val >= Self::MIN.0 && val <= Self::MAX.0, - DateOverflowSnafu { value: val } - ); - - Ok(Self(val)) + pub fn new(val: i32) -> Self { + Self(val) } pub fn val(&self) -> i32 { self.0 } - - /// Max valid Date value: "9999-12-31" - pub const MAX: Date = Date(2932896); - /// Min valid Date value: "1000-01-01" - pub const MIN: Date = Date(-354285); } #[cfg(test)] @@ -71,9 +59,9 @@ mod tests { #[test] pub fn test_print_date2() { - assert_eq!("1969-12-31", Date::try_new(-1).unwrap().to_string()); - assert_eq!("1970-01-01", Date::try_new(0).unwrap().to_string()); - assert_eq!("1970-02-12", Date::try_new(42).unwrap().to_string()); + assert_eq!("1969-12-31", Date::new(-1).to_string()); + assert_eq!("1970-01-01", Date::new(0).to_string()); + assert_eq!("1970-02-12", Date::new(42).to_string()); } #[test] @@ -93,19 +81,9 @@ mod tests { } #[test] - pub fn test_illegal_date_values() { - assert!(Date::try_new(Date::MAX.0 + 1).is_err()); - assert!(Date::try_new(Date::MIN.0 - 1).is_err()); - } - - #[test] - pub fn test_edge_date_values() { - let date = Date::from_str("9999-12-31").unwrap(); - assert_eq!(Date::MAX.0, date.0); - assert_eq!(date, Date::try_new(date.0).unwrap()); - - let date = Date::from_str("1000-01-01").unwrap(); - assert_eq!(Date::MIN.0, date.0); - assert_eq!(date, Date::try_new(date.0).unwrap()); + pub fn test_min_max() { + let mut date = Date::from_str("9999-12-31").unwrap(); + date.0 += 1000; + assert_eq!(date, Date::from_str(&date.to_string()).unwrap()); } } diff --git a/src/common/time/src/datetime.rs b/src/common/time/src/datetime.rs index d927a4b04e..f7feaf6e89 100644 --- a/src/common/time/src/datetime.rs +++ b/src/common/time/src/datetime.rs @@ -3,15 +3,13 @@ use std::str::FromStr; use chrono::NaiveDateTime; use serde::{Deserialize, Serialize}; -use snafu::{ensure, ResultExt}; +use snafu::ResultExt; -use crate::error::{DateTimeOverflowSnafu, Error, ParseDateStrSnafu, Result}; +use crate::error::{Error, ParseDateStrSnafu, Result}; const DATETIME_FORMAT: &str = "%F %T"; /// [DateTime] represents the **seconds elapsed since "1970-01-01 00:00:00 UTC" (UNIX Epoch)**. -/// -/// Valid [DateTime] value ranges from "1000-01-01 00:00:00" to "9999-12-31 23:59:59". #[derive( Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Default, Serialize, Deserialize, )] @@ -41,23 +39,13 @@ impl FromStr for DateTime { } impl DateTime { - pub fn try_new(val: i64) -> Result { - ensure!( - val >= Self::MIN.0 && val <= Self::MAX.0, - DateTimeOverflowSnafu { value: val } - ); - - Ok(Self(val)) + pub fn new(val: i64) -> Self { + Self(val) } pub fn val(&self) -> i64 { self.0 } - - /// Max valid DateTime value: 9999-12-31 23:59:59 - pub const MAX: DateTime = DateTime(253402300799); - /// Min valid DateTime value: 0000-01-01 00:00:00 - pub const MIN: DateTime = DateTime(-30610224000); } #[cfg(test)] @@ -66,24 +54,9 @@ mod tests { #[test] pub fn test_new_date_time() { - assert_eq!( - "1970-01-01 00:00:00", - DateTime::try_new(0).unwrap().to_string() - ); - assert_eq!( - "1970-01-01 00:00:01", - DateTime::try_new(1).unwrap().to_string() - ); - assert_eq!( - "1969-12-31 23:59:59", - DateTime::try_new(-1).unwrap().to_string() - ); - } - - #[test] - pub fn test_max_min() { - assert_eq!("9999-12-31 23:59:59", DateTime::MAX.to_string()); - assert_eq!("1000-01-01 00:00:00", DateTime::MIN.to_string()); + assert_eq!("1970-01-01 00:00:00", DateTime::new(0).to_string()); + assert_eq!("1970-01-01 00:00:01", DateTime::new(1).to_string()); + assert_eq!("1969-12-31 23:59:59", DateTime::new(-1).to_string()); } #[test] diff --git a/src/common/time/src/error.rs b/src/common/time/src/error.rs index b9c9fa28b6..2760785cfd 100644 --- a/src/common/time/src/error.rs +++ b/src/common/time/src/error.rs @@ -1,17 +1,11 @@ use chrono::ParseError; -use snafu::{Backtrace, Snafu}; +use snafu::Snafu; #[derive(Debug, Snafu)] #[snafu(visibility(pub))] pub enum Error { #[snafu(display("Failed to parse string to date, raw: {}, source: {}", raw, source))] ParseDateStr { raw: String, source: ParseError }, - - #[snafu(display("Failed to parse i32 value to Date: {}", value))] - DateOverflow { value: i32, backtrace: Backtrace }, - - #[snafu(display("Failed to parse i64 value to DateTime: {}", value))] - DateTimeOverflow { value: i64, backtrace: Backtrace }, } pub type Result = std::result::Result; diff --git a/src/datatypes/src/scalars.rs b/src/datatypes/src/scalars.rs index 6f8cd9fbbf..5b7a49b78a 100644 --- a/src/datatypes/src/scalars.rs +++ b/src/datatypes/src/scalars.rs @@ -335,12 +335,10 @@ mod tests { #[test] pub fn test_build_date_vector() { let expect: Vec> = vec![ - Some(Date::try_new(0).unwrap()), - Some(Date::try_new(-1).unwrap()), - Some(Date::try_new(1).unwrap()), + Some(Date::new(0)), + Some(Date::new(-1)), None, - Some(Date::MAX), - Some(Date::MIN), + Some(Date::new(1)), ]; let vector: DateVector = build_vector_from_slice(&expect); assert_vector_eq(&expect, &vector); @@ -348,7 +346,7 @@ mod tests { #[test] pub fn test_date_scalar() { - let date = Date::try_new(1).unwrap(); + let date = Date::new(1); assert_eq!(date, date.as_scalar_ref()); assert_eq!(date, date.to_owned_scalar()); } diff --git a/src/datatypes/src/value.rs b/src/datatypes/src/value.rs index 36b44d921e..381da8512c 100644 --- a/src/datatypes/src/value.rs +++ b/src/datatypes/src/value.rs @@ -406,11 +406,11 @@ mod tests { ); assert_eq!( serde_json::Value::Number(5000i32.into()), - to_json(Value::Date(common_time::date::Date::try_new(5000).unwrap())) + to_json(Value::Date(common_time::date::Date::new(5000))) ); assert_eq!( serde_json::Value::Number(5000i64.into()), - to_json(Value::DateTime(DateTime::try_new(5000).unwrap())) + to_json(Value::DateTime(DateTime::new(5000))) ); let json_value: serde_json::Value = diff --git a/src/datatypes/src/vectors/builder.rs b/src/datatypes/src/vectors/builder.rs index 2f379ee3aa..fea07e0a8a 100644 --- a/src/datatypes/src/vectors/builder.rs +++ b/src/datatypes/src/vectors/builder.rs @@ -138,11 +138,9 @@ impl VectorBuilder { (VectorBuilder::String(b), Value::String(v)) => b.push(Some(v.as_utf8())), (VectorBuilder::Binary(b), Value::Binary(v)) => b.push(Some(v)), (VectorBuilder::Date(b), Value::Date(v)) => b.push(Some(*v)), - (VectorBuilder::Date(b), Value::Int32(v)) => b.push(Some(Date::try_new(*v).unwrap())), + (VectorBuilder::Date(b), Value::Int32(v)) => b.push(Some(Date::new(*v))), (VectorBuilder::DateTime(b), Value::DateTime(v)) => b.push(Some(*v)), - (VectorBuilder::DateTime(b), Value::Int64(v)) => { - b.push(Some(DateTime::try_new(*v).unwrap())) - } + (VectorBuilder::DateTime(b), Value::Int64(v)) => b.push(Some(DateTime::new(*v))), _ => panic!( "Value {:?} does not match builder type {:?}", value, @@ -292,11 +290,11 @@ mod tests { let mut builder = VectorBuilder::with_capacity(ConcreteDataType::date_datatype(), 3); assert_eq!(ConcreteDataType::date_datatype(), builder.data_type()); builder.push_null(); - builder.push(&Value::Date(Date::try_new(123).unwrap())); + builder.push(&Value::Date(Date::new(123))); let v = builder.finish(); let v = v.as_any().downcast_ref::().unwrap(); assert_eq!(Value::Null, v.get(0)); - assert_eq!(Value::Date(Date::try_new(123).unwrap()), v.get(1)); + assert_eq!(Value::Date(Date::new(123)), v.get(1)); assert_eq!( &arrow::datatypes::DataType::Date32, v.to_arrow_array().data_type() @@ -308,11 +306,11 @@ mod tests { let mut builder = VectorBuilder::with_capacity(ConcreteDataType::datetime_datatype(), 3); assert_eq!(ConcreteDataType::datetime_datatype(), builder.data_type()); builder.push_null(); - builder.push(&Value::DateTime(DateTime::try_new(123).unwrap())); + builder.push(&Value::DateTime(DateTime::new(123))); let v = builder.finish(); let v = v.as_any().downcast_ref::().unwrap(); assert_eq!(Value::Null, v.get(0)); - assert_eq!(Value::DateTime(DateTime::try_new(123).unwrap()), v.get(1)); + assert_eq!(Value::DateTime(DateTime::new(123)), v.get(1)); assert_eq!( &arrow::datatypes::DataType::Date64, v.to_arrow_array().data_type() diff --git a/src/datatypes/src/vectors/date.rs b/src/datatypes/src/vectors/date.rs index 37cfb5998f..3cddd1d801 100644 --- a/src/datatypes/src/vectors/date.rs +++ b/src/datatypes/src/vectors/date.rs @@ -83,9 +83,7 @@ impl Vector for DateVector { fn get(&self, index: usize) -> Value { match self.array.get(index) { - Value::Int32(v) => { - Value::Date(Date::try_new(v).expect("Not expected to overflow here")) - } + Value::Int32(v) => Value::Date(Date::new(v)), Value::Null => Value::Null, _ => { unreachable!() @@ -114,9 +112,7 @@ impl<'a> Iterator for DateIter<'a> { type Item = Option; fn next(&mut self) -> Option { - self.iter - .next() - .map(|v| v.map(|v| Date::try_new(v).unwrap())) + self.iter.next().map(|v| v.map(Date::new)) } } @@ -128,7 +124,7 @@ impl ScalarVector for DateVector { type Builder = DateVectorBuilder; fn get_data(&self, idx: usize) -> Option> { - self.array.get_data(idx).map(|v| Date::try_new(v).unwrap()) + self.array.get_data(idx).map(Date::new) } fn iter_data(&self) -> Self::Iter<'_> { @@ -143,7 +139,7 @@ impl Serializable for DateVector { Ok(self .array .iter_data() - .map(|v| v.map(|d| Date::try_new(d).unwrap())) + .map(|v| v.map(Date::new)) .map(|v| match v { None => serde_json::Value::Null, Some(v) => v.into(), @@ -205,26 +201,25 @@ mod tests { #[test] pub fn test_build_date_vector() { let mut builder = DateVectorBuilder::with_capacity(4); - builder.push(Some(Date::try_new(1).unwrap())); + builder.push(Some(Date::new(1))); builder.push(None); - builder.push(Some(Date::try_new(-1).unwrap())); + builder.push(Some(Date::new(-1))); let vector = builder.finish(); assert_eq!(3, vector.len()); - assert_eq!(Some(Date::try_new(1).unwrap()), vector.get_data(0)); + assert_eq!(Some(Date::new(1)), vector.get_data(0)); assert_eq!(None, vector.get_data(1)); - assert_eq!(Some(Date::try_new(-1).unwrap()), vector.get_data(2)); + assert_eq!(Some(Date::new(-1)), vector.get_data(2)); let mut iter = vector.iter_data(); - assert_eq!(Some(Date::try_new(1).unwrap()), iter.next().unwrap()); + assert_eq!(Some(Date::new(1)), iter.next().unwrap()); assert_eq!(None, iter.next().unwrap()); - assert_eq!(Some(Date::try_new(-1).unwrap()), iter.next().unwrap()); + assert_eq!(Some(Date::new(-1)), iter.next().unwrap()); } #[test] pub fn test_date_scalar() { - let vector = - DateVector::from_slice(&[Date::try_new(1).unwrap(), Date::try_new(2).unwrap()]); + let vector = DateVector::from_slice(&[Date::new(1), Date::new(2)]); assert_eq!(2, vector.len()); - assert_eq!(Some(Date::try_new(1).unwrap()), vector.get_data(0)); - assert_eq!(Some(Date::try_new(2).unwrap()), vector.get_data(1)); + assert_eq!(Some(Date::new(1)), vector.get_data(0)); + assert_eq!(Some(Date::new(2)), vector.get_data(1)); } } diff --git a/src/datatypes/src/vectors/datetime.rs b/src/datatypes/src/vectors/datetime.rs index 1f51775c5e..130a1e6f11 100644 --- a/src/datatypes/src/vectors/datetime.rs +++ b/src/datatypes/src/vectors/datetime.rs @@ -84,9 +84,7 @@ impl Vector for DateTimeVector { fn get(&self, index: usize) -> Value { match self.array.get(index) { - Value::Int64(v) => { - Value::DateTime(DateTime::try_new(v).expect("Not expected to overflow here")) - } + Value::Int64(v) => Value::DateTime(DateTime::new(v)), Value::Null => Value::Null, _ => { unreachable!() @@ -104,7 +102,7 @@ impl Serializable for DateTimeVector { Ok(self .array .iter_data() - .map(|v| v.map(|d| DateTime::try_new(d).unwrap())) + .map(|v| v.map(DateTime::new)) .map(|v| match v { None => serde_json::Value::Null, Some(v) => v.into(), @@ -113,6 +111,14 @@ impl Serializable for DateTimeVector { } } +impl From>> for DateTimeVector { + fn from(data: Vec>) -> Self { + Self { + array: PrimitiveVector::::from(data), + } + } +} + pub struct DateTimeVectorBuilder { buffer: PrimitiveVectorBuilder, } @@ -167,9 +173,7 @@ impl<'a> Iterator for DateTimeIter<'a> { type Item = Option; fn next(&mut self) -> Option { - self.iter - .next() - .map(|v| v.map(|v| DateTime::try_new(v).unwrap())) + self.iter.next().map(|v| v.map(DateTime::new)) } } @@ -180,9 +184,7 @@ impl ScalarVector for DateTimeVector { type Builder = DateTimeVectorBuilder; fn get_data(&self, idx: usize) -> Option> { - self.array - .get_data(idx) - .map(|v| DateTime::try_new(v).unwrap()) + self.array.get_data(idx).map(DateTime::new) } fn iter_data(&self) -> Self::Iter<'_> { @@ -209,9 +211,9 @@ mod tests { v.to_arrow_array().data_type() ); let mut iter = v.iter_data(); - assert_eq!(Some(DateTime::try_new(1).unwrap()), iter.next().unwrap()); - assert_eq!(Some(DateTime::try_new(2).unwrap()), iter.next().unwrap()); - assert_eq!(Some(DateTime::try_new(3).unwrap()), iter.next().unwrap()); + assert_eq!(Some(DateTime::new(1)), iter.next().unwrap()); + assert_eq!(Some(DateTime::new(2)), iter.next().unwrap()); + assert_eq!(Some(DateTime::new(3)), iter.next().unwrap()); assert!(!v.is_null(0)); assert_eq!(24, v.memory_size()); // size of i64 * 3 @@ -230,14 +232,14 @@ mod tests { #[test] pub fn test_datetime_vector_builder() { let mut builder = DateTimeVectorBuilder::with_capacity(3); - builder.push(Some(DateTime::try_new(1).unwrap())); + builder.push(Some(DateTime::new(1))); builder.push(None); - builder.push(Some(DateTime::try_new(-1).unwrap())); + builder.push(Some(DateTime::new(-1))); let v = builder.finish(); assert_eq!(ConcreteDataType::datetime_datatype(), v.data_type()); - assert_eq!(Value::DateTime(DateTime::try_new(1).unwrap()), v.get(0)); + assert_eq!(Value::DateTime(DateTime::new(1)), v.get(0)); assert_eq!(Value::Null, v.get(1)); - assert_eq!(Value::DateTime(DateTime::try_new(-1).unwrap()), v.get(2)); + assert_eq!(Value::DateTime(DateTime::new(-1)), v.get(2)); } } diff --git a/src/datatypes/src/vectors/helper.rs b/src/datatypes/src/vectors/helper.rs index cc2bcd790e..d7e682a687 100644 --- a/src/datatypes/src/vectors/helper.rs +++ b/src/datatypes/src/vectors/helper.rs @@ -142,6 +142,9 @@ impl Helper { ScalarValue::Date32(v) => { ConstantVector::new(Arc::new(DateVector::from(vec![v])), length) } + ScalarValue::Date64(v) => { + ConstantVector::new(Arc::new(DateTimeVector::from(vec![v])), length) + } _ => { return ConversionSnafu { from: format!("Unsupported scalar value: {}", value), @@ -193,6 +196,7 @@ impl Helper { mod tests { use arrow::array::Int32Array; use common_time::date::Date; + use common_time::datetime::DateTime; use super::*; @@ -230,7 +234,17 @@ mod tests { assert_eq!(ConcreteDataType::date_datatype(), vector.data_type()); assert_eq!(3, vector.len()); for i in 0..vector.len() { - assert_eq!(Value::Date(Date::try_new(42).unwrap()), vector.get(i)); + assert_eq!(Value::Date(Date::new(42)), vector.get(i)); + } + } + + #[test] + pub fn test_try_from_scalar_datetime_value() { + let vector = Helper::try_from_scalar_value(ScalarValue::Date64(Some(42)), 3).unwrap(); + assert_eq!(ConcreteDataType::datetime_datatype(), vector.data_type()); + assert_eq!(3, vector.len()); + for i in 0..vector.len() { + assert_eq!(Value::DateTime(DateTime::new(42)), vector.get(i)); } } }