From 182be0584b0e8a5a214492c140a2fbf9d835e235 Mon Sep 17 00:00:00 2001 From: QuenKar <47681251+QuenKar@users.noreply.github.com> Date: Wed, 15 Nov 2023 15:19:49 +0800 Subject: [PATCH] chore: cr comment Co-authored-by: Dennis Zhuang Co-authored-by: Yingwen --- src/common/decimal/src/decimal128.rs | 8 +- src/common/grpc/src/select.rs | 1 + src/datatypes/src/data_type.rs | 10 +- src/datatypes/src/error.rs | 14 +- src/datatypes/src/scalars.rs | 2 +- src/datatypes/src/types.rs | 2 +- src/datatypes/src/types/decimal_type.rs | 6 +- src/datatypes/src/value.rs | 10 +- src/datatypes/src/vectors/decimal.rs | 283 ++++++++++++++---------- src/datatypes/src/vectors/helper.rs | 35 ++- src/sql/src/statements.rs | 5 +- 11 files changed, 237 insertions(+), 139 deletions(-) diff --git a/src/common/decimal/src/decimal128.rs b/src/common/decimal/src/decimal128.rs index ec345e9e946d..49000904d1f6 100644 --- a/src/common/decimal/src/decimal128.rs +++ b/src/common/decimal/src/decimal128.rs @@ -51,8 +51,8 @@ pub struct Decimal128 { } impl Decimal128 { - /// Create a new Decimal128 from i128, precision and scale. - pub fn new_unchecked(value: i128, precision: u8, scale: i8) -> Self { + /// Create a new Decimal128 from i128, precision and scale without any validation. + pub fn new(value: i128, precision: u8, scale: i8) -> Self { Self { value, precision, @@ -60,6 +60,7 @@ impl Decimal128 { } } + /// Try new Decimal128 from i128, precision and scale with validation. pub fn try_new(value: i128, precision: u8, scale: i8) -> error::Result { // make sure the precision and scale is valid. valid_precision_and_scale(precision, scale)?; @@ -70,6 +71,7 @@ impl Decimal128 { }) } + /// Return underlying value without precision and scale pub fn val(&self) -> i128 { self.value } @@ -281,7 +283,7 @@ mod tests { #[test] fn test_common_decimal128() { - let decimal = Decimal128::new_unchecked(123456789, 9, 3); + let decimal = Decimal128::new(123456789, 9, 3); assert_eq!(decimal.to_string(), "123456.789"); let decimal = Decimal128::try_new(123456789, 9, 0); diff --git a/src/common/grpc/src/select.rs b/src/common/grpc/src/select.rs index 02d0a2be6b37..4c6e4a6af99c 100644 --- a/src/common/grpc/src/select.rs +++ b/src/common/grpc/src/select.rs @@ -71,6 +71,7 @@ macro_rules! convert_arrow_array_to_grpc_vals { return Ok(vals); }, )+ + // TODO(QuenKar): support gRPC for Decimal128 ConcreteDataType::Null(_) | ConcreteDataType::List(_) | ConcreteDataType::Dictionary(_) | ConcreteDataType::Decimal128(_) => unreachable!("Should not send {:?} in gRPC", $data_type), } }}; diff --git a/src/datatypes/src/data_type.rs b/src/datatypes/src/data_type.rs index 193338a0a325..877bf47e3bf5 100644 --- a/src/datatypes/src/data_type.rs +++ b/src/datatypes/src/data_type.rs @@ -29,7 +29,7 @@ use serde::{Deserialize, Serialize}; use crate::error::{self, Error, Result}; use crate::type_id::LogicalTypeId; use crate::types::{ - BinaryType, BooleanType, DateTimeType, DateType, DecimalType, DictionaryType, + BinaryType, BooleanType, DateTimeType, DateType, Decimal128Type, DictionaryType, DurationMicrosecondType, DurationMillisecondType, DurationNanosecondType, DurationSecondType, DurationType, Float32Type, Float64Type, Int16Type, Int32Type, Int64Type, Int8Type, IntervalDayTimeType, IntervalMonthDayNanoType, IntervalType, IntervalYearMonthType, ListType, @@ -58,8 +58,8 @@ pub enum ConcreteDataType { Float32(Float32Type), Float64(Float64Type), - // Decimal type: - Decimal128(DecimalType), + // Decimal128 type: + Decimal128(Decimal128Type), // String types: Binary(BinaryType), @@ -248,7 +248,7 @@ impl ConcreteDataType { } } - pub fn as_decimal(&self) -> Option { + pub fn as_decimal128(&self) -> Option { match self { ConcreteDataType::Decimal128(d) => Some(*d), _ => None, @@ -479,7 +479,7 @@ impl ConcreteDataType { } pub fn decimal128_datatype(precision: u8, scale: i8) -> ConcreteDataType { - ConcreteDataType::Decimal128(DecimalType::new(precision, scale)) + ConcreteDataType::Decimal128(Decimal128Type::new(precision, scale)) } pub fn decimal128_default_datatype() -> ConcreteDataType { diff --git a/src/datatypes/src/error.rs b/src/datatypes/src/error.rs index 9024c4d384a0..316b50e3276c 100644 --- a/src/datatypes/src/error.rs +++ b/src/datatypes/src/error.rs @@ -123,8 +123,18 @@ pub enum Error { #[snafu(display("Failed to unpack value to given type: {}", reason))] TryFromValue { reason: String, location: Location }, - #[snafu(display("Invalid arguments, reason: {}", error))] - InvalidArguments { + #[snafu(display("Failed to specify the precision {} and scale {}", precision, scale))] + InvalidPrecisionOrScale { + precision: u8, + scale: i8, + #[snafu(source)] + error: arrow::error::ArrowError, + location: Location, + }, + + #[snafu(display("Value exceeds the precision {} bound", precision))] + ValueExceedsPrecision { + precision: u8, #[snafu(source)] error: arrow::error::ArrowError, location: Location, diff --git a/src/datatypes/src/scalars.rs b/src/datatypes/src/scalars.rs index da79fcff080b..8539819a439c 100644 --- a/src/datatypes/src/scalars.rs +++ b/src/datatypes/src/scalars.rs @@ -420,7 +420,7 @@ mod tests { #[test] fn test_decimal_scalar() { - let decimal = Decimal128::new_unchecked(1, 1, 1); + let decimal = Decimal128::new(1, 1, 1); assert_eq!(decimal, decimal.as_scalar_ref()); assert_eq!(decimal, decimal.to_owned_scalar()); } diff --git a/src/datatypes/src/types.rs b/src/datatypes/src/types.rs index 668daa4ae440..686fd9c49f10 100644 --- a/src/datatypes/src/types.rs +++ b/src/datatypes/src/types.rs @@ -33,7 +33,7 @@ pub use boolean_type::BooleanType; pub use cast::{cast, cast_with_opt}; pub use date_type::DateType; pub use datetime_type::DateTimeType; -pub use decimal_type::DecimalType; +pub use decimal_type::Decimal128Type; pub use dictionary_type::DictionaryType; pub use duration_type::{ DurationMicrosecondType, DurationMillisecondType, DurationNanosecondType, DurationSecondType, diff --git a/src/datatypes/src/types/decimal_type.rs b/src/datatypes/src/types/decimal_type.rs index 8e4881e90ff9..edda8fe9f7eb 100644 --- a/src/datatypes/src/types/decimal_type.rs +++ b/src/datatypes/src/types/decimal_type.rs @@ -25,12 +25,12 @@ use crate::vectors::{Decimal128VectorBuilder, MutableVector}; #[derive( Debug, Default, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize, )] -pub struct DecimalType { +pub struct Decimal128Type { precision: u8, scale: i8, } -impl DecimalType { +impl Decimal128Type { pub fn new(precision: u8, scale: i8) -> Self { Self { precision, scale } } @@ -44,7 +44,7 @@ impl DecimalType { } } -impl DataType for DecimalType { +impl DataType for Decimal128Type { fn name(&self) -> &str { "decimal128" } diff --git a/src/datatypes/src/value.rs b/src/datatypes/src/value.rs index d4bd2d7694ea..23135a6e44a8 100644 --- a/src/datatypes/src/value.rs +++ b/src/datatypes/src/value.rs @@ -120,7 +120,7 @@ impl Display for Value { .join(", "); write!(f, "{}[{}]", v.datatype.name(), items) } - Value::Decimal128(v) => write!(f, "{:?}", v), + Value::Decimal128(v) => write!(f, "{}", v), } } } @@ -2255,9 +2255,6 @@ mod tests { #[test] fn test_value_ref_estimated_size() { - // MacOS is 48, Ubuntu is 32 - // assert_eq!(std::mem::size_of::(), 48); - check_value_ref_size_eq(&ValueRef::Boolean(true), 1); check_value_ref_size_eq(&ValueRef::UInt8(1), 1); check_value_ref_size_eq(&ValueRef::UInt16(1), 2); @@ -2334,9 +2331,6 @@ mod tests { }), 85, ); - check_value_ref_size_eq( - &ValueRef::Decimal128(Decimal128::new_unchecked(1234, 3, 1)), - 32, - ) + check_value_ref_size_eq(&ValueRef::Decimal128(Decimal128::new(1234, 3, 1)), 32) } } diff --git a/src/datatypes/src/vectors/decimal.rs b/src/datatypes/src/vectors/decimal.rs index d191fa25077a..b440d2337d2c 100644 --- a/src/datatypes/src/vectors/decimal.rs +++ b/src/datatypes/src/vectors/decimal.rs @@ -16,17 +16,19 @@ use std::any::Any; use std::fmt::Debug; use std::sync::Arc; -use arrow::array::ArrayData; use arrow_array::builder::{ArrayBuilder, Decimal128Builder}; use arrow_array::iterator::ArrayIter; use arrow_array::{Array, ArrayRef, Decimal128Array}; +use common_decimal::decimal128::{DECIMAL128_DEFAULT_SCALE, DECIMAL128_MAX_PRECISION}; use common_decimal::Decimal128; use snafu::{OptionExt, ResultExt}; use super::{MutableVector, Validity, Vector, VectorRef}; use crate::arrow::datatypes::DataType as ArrowDataType; use crate::data_type::ConcreteDataType; -use crate::error::{self, CastTypeSnafu, InvalidArgumentsSnafu, Result}; +use crate::error::{ + self, CastTypeSnafu, InvalidPrecisionOrScaleSnafu, Result, ValueExceedsPrecisionSnafu, +}; use crate::prelude::{ScalarVector, ScalarVectorBuilder}; use crate::serialize::Serializable; use crate::value::{Value, ValueRef}; @@ -39,16 +41,11 @@ pub struct Decimal128Vector { } impl Decimal128Vector { + /// New a Decimal128Vector from Arrow Decimal128Array pub fn new(array: Decimal128Array) -> Self { Self { array } } - pub fn from_array_data(array: ArrayData) -> Self { - Self { - array: Decimal128Array::from(array), - } - } - /// Construct Vector from i128 values pub fn from_values>(iter: I) -> Self { Self { @@ -56,6 +53,7 @@ impl Decimal128Vector { } } + /// Construct Vector from i128 values slice pub fn from_slice>(slice: P) -> Self { let iter = slice.as_ref().iter().copied(); Self { @@ -63,6 +61,7 @@ impl Decimal128Vector { } } + /// Construct Vector from Wrapper(Decimal128) values slice pub fn from_wrapper_slice>(slice: P) -> Self { let iter = slice.as_ref().iter().copied().map(|v| v.val()); Self { @@ -70,35 +69,38 @@ impl Decimal128Vector { } } - pub fn to_array_data(&self) -> ArrayData { - self.array.to_data() - } - + /// Get decimal128 value from vector by offset and length. pub fn get_slice(&self, offset: usize, length: usize) -> Self { - let data = self.array.to_data().slice(offset, length); - Self::from_array_data(data) + let array = self.array.slice(offset, length); + Self { array } } - /// Change the precision and scale of the Decimal128Vector, - /// And check precision and scale if compatible. + /// Returns a Decimal vector with the same data as self, with the + /// specified precision and scale(should in Decimal128 range), + /// and return error if value is out of precision bound. + /// + /// + /// For example: + /// value = 12345, precision = 3, return error. pub fn with_precision_and_scale(self, precision: u8, scale: i8) -> Result { + // validate if precision is too small + self.validate_decimal_precision(precision)?; let array = self .array .with_precision_and_scale(precision, scale) - .context(InvalidArgumentsSnafu {})?; + .context(InvalidPrecisionOrScaleSnafu { precision, scale })?; Ok(Self { array }) } - pub fn null_if_overflow_precision(&self, precision: u8) -> Self { - Self { - array: self.array.null_if_overflow_precision(precision), - } - } - - pub fn validate_decimal_precision(&self, precision: u8) -> Result<()> { - self.array - .validate_decimal_precision(precision) - .context(InvalidArgumentsSnafu {}) + /// Returns a Decimal vector with the same data as self, with the + /// specified precision and scale(should in Decimal128 range), + /// and return null if value is out of precision bound. + /// + /// For example: + /// value = 12345, precision = 3, the value will be casted to null. + pub fn with_precision_and_scale_to_null(self, precision: u8, scale: i8) -> Result { + self.null_if_overflow_precision(precision) + .with_precision_and_scale(precision, scale) } /// Return decimal value as string @@ -106,17 +108,46 @@ impl Decimal128Vector { self.array.value_as_string(idx) } + /// Return decimal128 vector precision pub fn precision(&self) -> u8 { self.array.precision() } + /// Return decimal128 vector scale pub fn scale(&self) -> i8 { self.array.scale() } + /// Return decimal128 vector inner array pub(crate) fn as_arrow(&self) -> &dyn Array { &self.array } + + /// Validate decimal precision, if precision is invalid, return error. + fn validate_decimal_precision(&self, precision: u8) -> Result<()> { + self.array + .validate_decimal_precision(precision) + .context(ValueExceedsPrecisionSnafu { precision }) + } + + /// Values that exceed the precision bounds will be casted to Null. + fn null_if_overflow_precision(&self, precision: u8) -> Self { + Self { + array: self.array.null_if_overflow_precision(precision), + } + } + + /// Get decimal128 Value from array by index. + fn get_decimal128_value_from_array(&self, index: usize) -> Option { + if self.array.is_valid(index) { + // Safety: The index have been checked by `is_valid()`. + let value = unsafe { self.array.value_unchecked(index) }; + // Safety: The precision and scale have been checked by Vector. + Some(Decimal128::new(value, self.precision(), self.scale())) + } else { + None + } + } } impl Vector for Decimal128Vector { @@ -141,13 +172,11 @@ impl Vector for Decimal128Vector { } fn to_arrow_array(&self) -> ArrayRef { - let data = self.array.to_data(); - Arc::new(Decimal128Array::from(data)) + Arc::new(self.array.clone()) } fn to_boxed_arrow_array(&self) -> Box { - let data = self.array.to_data(); - Box::new(Decimal128Array::from(data)) + Box::new(self.array.clone()) } fn validity(&self) -> Validity { @@ -167,37 +196,23 @@ impl Vector for Decimal128Vector { } fn slice(&self, offset: usize, length: usize) -> VectorRef { - let data = self.array.to_data().slice(offset, length); - Arc::new(Self::from_array_data(data)) + let array = self.array.slice(offset, length); + Arc::new(Self { array }) } fn get(&self, index: usize) -> Value { - if !self.array.is_valid(index) { - return Value::Null; - } - - match self.array.data_type() { - ArrowDataType::Decimal128(precision, scale) => { - // Safety: The index have been checked by `is_valid()`. - let value = unsafe { self.array.value_unchecked(index) }; - Value::Decimal128(Decimal128::new_unchecked(value, *precision, *scale)) - } - _ => Value::Null, + if let Some(decimal) = self.get_decimal128_value_from_array(index) { + Value::Decimal128(decimal) + } else { + Value::Null } } fn get_ref(&self, index: usize) -> ValueRef { - if !self.array.is_valid(index) { - return ValueRef::Null; - } - - match self.array.data_type() { - ArrowDataType::Decimal128(precision, scale) => { - // Safety: The index have been checked by `is_valid()`. - let value = unsafe { self.array.value_unchecked(index) }; - ValueRef::Decimal128(Decimal128::new_unchecked(value, *precision, *scale)) - } - _ => ValueRef::Null, + if let Some(decimal) = self.get_decimal128_value_from_array(index) { + ValueRef::Decimal128(decimal) + } else { + ValueRef::Null } } } @@ -220,7 +235,7 @@ impl Serializable for Decimal128Vector { self.iter_data() .map(|v| match v { None => Ok(serde_json::Value::Null), // if decimal vector not present, map to NULL - Some(vec) => serde_json::to_value(vec), + Some(d) => serde_json::to_value(d), }) .collect::>() .context(error::SerializeSnafu) @@ -228,7 +243,8 @@ impl Serializable for Decimal128Vector { } pub struct Decimal128Iter<'a> { - data_type: &'a ArrowDataType, + precision: u8, + scale: i8, iter: ArrayIter<&'a Decimal128Array>, } @@ -236,14 +252,11 @@ impl<'a> Iterator for Decimal128Iter<'a> { type Item = Option; fn next(&mut self) -> Option { - Some(self.iter.next().and_then(|v| { - v.and_then(|v| match self.data_type { - ArrowDataType::Decimal128(precision, scale) => { - Some(Decimal128::new_unchecked(v, *precision, *scale)) - } - _ => None, - }) - })) + Some( + self.iter + .next() + .and_then(|v| v.map(|v| Decimal128::new(v, self.precision, self.scale))), + ) } fn size_hint(&self) -> (usize, Option) { @@ -261,36 +274,27 @@ impl ScalarVector for Decimal128Vector { type Builder = Decimal128VectorBuilder; fn get_data(&self, idx: usize) -> Option> { - if !self.array.is_valid(idx) { - return None; - } - - match self.array.data_type() { - ArrowDataType::Decimal128(precision, scale) => { - // Safety: The index have been checked by `is_valid()`. - let value = unsafe { self.array.value_unchecked(idx) }; - // Safety: The precision and scale have been checked by ArrowDataType. - Some(Decimal128::new_unchecked(value, *precision, *scale)) - } - _ => None, - } + self.get_decimal128_value_from_array(idx) } fn iter_data(&self) -> Self::Iter<'_> { Self::Iter { - data_type: self.array.data_type(), + precision: self.precision(), + scale: self.scale(), iter: self.array.iter(), } } } pub struct Decimal128VectorBuilder { + precision: u8, + scale: i8, mutable_array: Decimal128Builder, } impl MutableVector for Decimal128VectorBuilder { fn data_type(&self) -> ConcreteDataType { - unimplemented!() + ConcreteDataType::decimal128_datatype(self.precision, self.scale) } fn len(&self) -> usize { @@ -331,9 +335,8 @@ impl MutableVector for Decimal128VectorBuilder { ), })?; let slice = decimal_vector.get_slice(offset, length); - for i in slice.iter_data() { - self.mutable_array.append_option(i.map(|v| v.val())); - } + self.mutable_array + .extend(slice.iter_data().map(|v| v.map(|d| d.val()))); Ok(()) } } @@ -343,6 +346,8 @@ impl ScalarVectorBuilder for Decimal128VectorBuilder { fn with_capacity(capacity: usize) -> Self { Self { + precision: DECIMAL128_MAX_PRECISION, + scale: DECIMAL128_DEFAULT_SCALE, mutable_array: Decimal128Builder::with_capacity(capacity), } } @@ -352,9 +357,29 @@ impl ScalarVectorBuilder for Decimal128VectorBuilder { } fn finish(&mut self) -> Self::VectorType { + // Arrow array builder will discard precision and scale information when finish. + // This behavior may not be reasonable. Decimal128Vector { array: self.mutable_array.finish(), } + .with_precision_and_scale(self.precision, self.scale) + // unwrap is safe because we have checked the precision and scale in builder. + .unwrap() + } +} + +impl Decimal128VectorBuilder { + /// Change the precision and scale of the Decimal128VectorBuilder. + pub fn with_precison_and_scale(self, precision: u8, scale: i8) -> Result { + let mutable_array = self + .mutable_array + .with_precision_and_scale(precision, scale) + .context(InvalidPrecisionOrScaleSnafu { precision, scale })?; + Ok(Self { + precision, + scale, + mutable_array, + }) } } @@ -382,8 +407,8 @@ pub mod tests { fn test_from_slice() { let decimal_vector = Decimal128Vector::from_slice([123, 456]); let decimal_vector2 = Decimal128Vector::from_wrapper_slice([ - Decimal128::new_unchecked(123, 10, 2), - Decimal128::new_unchecked(456, 10, 2), + Decimal128::new(123, 10, 2), + Decimal128::new(456, 10, 2), ]); let expect = Decimal128Vector::from_values(vec![123, 456]); @@ -392,68 +417,104 @@ pub mod tests { } #[test] - fn test_decimal128_vector_basic() { + fn test_decimal128_vector_slice() { let data = vec![100, 200, 300]; // create a decimal vector let decimal_vector = Decimal128Vector::from_values(data.clone()) .with_precision_and_scale(10, 2) .unwrap(); - - assert_eq!(decimal_vector.len(), 3); - // check the first value + let decimal_vector2 = decimal_vector.slice(1, 2); + assert_eq!(decimal_vector2.len(), 2); assert_eq!( - decimal_vector.get(0), - Value::Decimal128(Decimal128::new_unchecked(100, 10, 2)) + decimal_vector2.get(0), + Value::Decimal128(Decimal128::new(200, 10, 2)) + ); + assert_eq!( + decimal_vector2.get(1), + Value::Decimal128(Decimal128::new(300, 10, 2)) ); + } + + #[test] + fn test_decimal128_vector_basic() { + let data = vec![100, 200, 300]; + // create a decimal vector + let decimal_vector = Decimal128Vector::from_values(data.clone()) + .with_precision_and_scale(10, 2) + .unwrap(); + + // can use value_of_string(idx) get a decimal string + assert_eq!(decimal_vector.value_as_string(0), "1.00"); // iterator for-loop - for (i, _) in data.iter().enumerate() { + for i in 0..data.len() { assert_eq!( decimal_vector.get_data(i), - Some(Decimal128::new_unchecked((i + 1) as i128 * 100, 10, 2)) + Some(Decimal128::new((i + 1) as i128 * 100, 10, 2)) ); assert_eq!( decimal_vector.get(i), - Value::Decimal128(Decimal128::new_unchecked((i + 1) as i128 * 100, 10, 2)) - ) + Value::Decimal128(Decimal128::new((i + 1) as i128 * 100, 10, 2)) + ); + assert_eq!( + decimal_vector.get_ref(i), + ValueRef::Decimal128(Decimal128::new((i + 1) as i128 * 100, 10, 2)) + ); } + + // directly convert vector with precision = 2 and scale = 1, + // then all of value will be null because of precision. + let decimal_vector = decimal_vector + .with_precision_and_scale_to_null(2, 1) + .unwrap(); + assert_eq!(decimal_vector.len(), 3); + assert!(decimal_vector.is_null(0)); + assert!(decimal_vector.is_null(1)); + assert!(decimal_vector.is_null(2)); } #[test] fn test_decimal128_vector_builder() { - let mut decimal_builder = Decimal128VectorBuilder::with_capacity(3); - decimal_builder.push(Some(Decimal128::new_unchecked(100, 10, 2))); - decimal_builder.push(Some(Decimal128::new_unchecked(200, 10, 2))); - decimal_builder.push(Some(Decimal128::new_unchecked(300, 10, 2))); - let decimal_vector = decimal_builder - .finish() - .with_precision_and_scale(10, 2) + let mut decimal_builder = Decimal128VectorBuilder::with_capacity(3) + .with_precison_and_scale(10, 2) .unwrap(); + decimal_builder.push(Some(Decimal128::new(100, 10, 2))); + decimal_builder.push(Some(Decimal128::new(200, 10, 2))); + decimal_builder.push(Some(Decimal128::new(300, 10, 2))); + let decimal_vector = decimal_builder.finish(); assert_eq!(decimal_vector.len(), 3); + assert_eq!(decimal_vector.precision(), 10); + assert_eq!(decimal_vector.scale(), 2); assert_eq!( decimal_vector.get(0), - Value::Decimal128(Decimal128::new_unchecked(100, 10, 2)) + Value::Decimal128(Decimal128::new(100, 10, 2)) ); assert_eq!( decimal_vector.get(1), - Value::Decimal128(Decimal128::new_unchecked(200, 10, 2)) + Value::Decimal128(Decimal128::new(200, 10, 2)) ); assert_eq!( decimal_vector.get(2), - Value::Decimal128(Decimal128::new_unchecked(300, 10, 2)) + Value::Decimal128(Decimal128::new(300, 10, 2)) ); // push value error let mut decimal_builder = Decimal128VectorBuilder::with_capacity(3); - decimal_builder.push(Some(Decimal128::new_unchecked(123, 10, 2))); - decimal_builder.push(Some(Decimal128::new_unchecked(1234, 10, 2))); - decimal_builder.push(Some(Decimal128::new_unchecked(12345, 10, 2))); - let decimal_vector = decimal_builder.finish().with_precision_and_scale(3, 2); - assert!(decimal_vector.is_ok()); + decimal_builder.push(Some(Decimal128::new(123, 38, 10))); + decimal_builder.push(Some(Decimal128::new(1234, 38, 10))); + decimal_builder.push(Some(Decimal128::new(12345, 38, 10))); + let decimal_vector = decimal_builder.finish(); + assert_eq!(decimal_vector.precision(), 38); + assert_eq!(decimal_vector.scale(), 10); + let result = decimal_vector.with_precision_and_scale(3, 2); + assert_eq!( + "Value exceeds the precision 3 bound", + result.unwrap_err().to_string() + ); } #[test] - fn test_cast() { + fn test_cast_to_decimal128() { let vector = Int8Vector::from_values(vec![1, 2, 3, 4, 100]); let casted_vector = vector.cast(&ConcreteDataType::decimal128_datatype(3, 1)); assert!(casted_vector.is_ok()); diff --git a/src/datatypes/src/vectors/helper.rs b/src/datatypes/src/vectors/helper.rs index 9e3d20f88940..90f9ff3935cf 100644 --- a/src/datatypes/src/vectors/helper.rs +++ b/src/datatypes/src/vectors/helper.rs @@ -404,8 +404,10 @@ mod tests { }; use arrow::datatypes::{Field, Int32Type}; use arrow_array::DictionaryArray; + use common_decimal::Decimal128; use common_time::time::Time; - use common_time::{Date, DateTime, Interval}; + use common_time::timestamp::TimeUnit; + use common_time::{Date, DateTime, Duration, Interval}; use super::*; use crate::value::Value; @@ -457,6 +459,37 @@ mod tests { } } + #[test] + fn test_try_from_scalar_duration_value() { + let vector = + Helper::try_from_scalar_value(ScalarValue::DurationSecond(Some(42)), 3).unwrap(); + assert_eq!( + ConcreteDataType::duration_second_datatype(), + vector.data_type() + ); + assert_eq!(3, vector.len()); + for i in 0..vector.len() { + assert_eq!( + Value::Duration(Duration::new(42, TimeUnit::Second)), + vector.get(i) + ); + } + } + + #[test] + fn test_try_from_scalar_decimal128_value() { + let vector = + Helper::try_from_scalar_value(ScalarValue::Decimal128(Some(42), 1, 1), 3).unwrap(); + assert_eq!( + ConcreteDataType::decimal128_datatype(1, 1), + vector.data_type() + ); + assert_eq!(3, vector.len()); + for i in 0..vector.len() { + assert_eq!(Value::Decimal128(Decimal128::new(42, 1, 1)), vector.get(i)); + } + } + #[test] fn test_try_from_list_value() { let value = ScalarValue::List( diff --git a/src/sql/src/statements.rs b/src/sql/src/statements.rs index 1c4c49f00ead..5e3f4d0746fb 100644 --- a/src/sql/src/statements.rs +++ b/src/sql/src/statements.rs @@ -145,10 +145,7 @@ macro_rules! parse_number_to_value { let n = parse_sql_number::($n)?; Ok(Value::Timestamp(Timestamp::new(n, t.unit()))) }, - ConcreteDataType::Decimal128(_) => { - // TODO(QuenKar): parse decimal128 string with precision and scale - unimplemented!("insert Decimal128 is not supported yet") - } + // TODO(QuenKar): parse decimal128 string with precision and scale _ => ParseSqlValueSnafu { msg: format!("Fail to parse number {}, invalid column type: {:?}",