From 587bdc980002fa76aa6e49752122039bf0becae0 Mon Sep 17 00:00:00 2001 From: Ruihang Xia Date: Thu, 8 Dec 2022 11:38:07 +0800 Subject: [PATCH] fix: fix other compile error in common-function (#719) * further fixing Signed-off-by: Ruihang Xia * fix all compile errors in common function Signed-off-by: Ruihang Xia Signed-off-by: Ruihang Xia --- .../function/src/scalars/aggregate/argmax.rs | 2 +- .../function/src/scalars/aggregate/mean.rs | 18 +++---- .../function/src/scalars/aggregate/polyval.rs | 52 ++++++++----------- src/common/function/src/scalars/math/pow.rs | 3 +- src/common/function/src/scalars/numpy/clip.rs | 19 ++++++- src/common/function/src/scalars/test.rs | 2 +- .../src/scalars/timestamp/from_unixtime.rs | 26 +++++----- 7 files changed, 65 insertions(+), 57 deletions(-) diff --git a/src/common/function/src/scalars/aggregate/argmax.rs b/src/common/function/src/scalars/aggregate/argmax.rs index 63a45fa855..d42d4550c6 100644 --- a/src/common/function/src/scalars/aggregate/argmax.rs +++ b/src/common/function/src/scalars/aggregate/argmax.rs @@ -35,7 +35,7 @@ pub struct Argmax { impl Argmax where - T: PartialOrd, + T: PartialOrd + Copy, { fn update(&mut self, value: T, index: u64) { if let Some(Ordering::Less) = self.max.partial_cmp(&Some(value)) { diff --git a/src/common/function/src/scalars/aggregate/mean.rs b/src/common/function/src/scalars/aggregate/mean.rs index f3dc723b41..ce619bb253 100644 --- a/src/common/function/src/scalars/aggregate/mean.rs +++ b/src/common/function/src/scalars/aggregate/mean.rs @@ -42,7 +42,7 @@ where { #[inline(always)] fn push(&mut self, value: T) { - self.sum += value.as_(); + self.sum += value.into_native().as_(); self.n += 1; } @@ -149,7 +149,7 @@ impl AggregateFunctionCreator for MeanAccumulatorCreator { with_match_primitive_type_id!( input_type.logical_type_id(), |$S| { - Ok(Box::new(Mean::<$S>::default())) + Ok(Box::new(Mean::<<$S as LogicalPrimitiveType>::Native>::default())) }, { let err_msg = format!( @@ -181,7 +181,7 @@ impl AggregateFunctionCreator for MeanAccumulatorCreator { #[cfg(test)] mod test { - use datatypes::vectors::PrimitiveVector; + use datatypes::vectors::Int32Vector; use super::*; #[test] @@ -193,21 +193,19 @@ mod test { // test update one not-null value let mut mean = Mean::::default(); - let v: Vec = vec![Arc::new(PrimitiveVector::::from(vec![Some(42)]))]; + let v: Vec = vec![Arc::new(Int32Vector::from(vec![Some(42)]))]; assert!(mean.update_batch(&v).is_ok()); assert_eq!(Value::from(42.0_f64), mean.evaluate().unwrap()); // test update one null value let mut mean = Mean::::default(); - let v: Vec = vec![Arc::new(PrimitiveVector::::from(vec![ - Option::::None, - ]))]; + let v: Vec = vec![Arc::new(Int32Vector::from(vec![Option::::None]))]; assert!(mean.update_batch(&v).is_ok()); assert_eq!(Value::Null, mean.evaluate().unwrap()); // test update no null-value batch let mut mean = Mean::::default(); - let v: Vec = vec![Arc::new(PrimitiveVector::::from(vec![ + let v: Vec = vec![Arc::new(Int32Vector::from(vec![ Some(-1i32), Some(1), Some(2), @@ -217,7 +215,7 @@ mod test { // test update null-value batch let mut mean = Mean::::default(); - let v: Vec = vec![Arc::new(PrimitiveVector::::from(vec![ + let v: Vec = vec![Arc::new(Int32Vector::from(vec![ Some(-2i32), None, Some(3), @@ -229,7 +227,7 @@ mod test { // test update with constant vector let mut mean = Mean::::default(); let v: Vec = vec![Arc::new(ConstantVector::new( - Arc::new(PrimitiveVector::::from_vec(vec![4])), + Arc::new(Int32Vector::from_vec(vec![4])), 10, ))]; assert!(mean.update_batch(&v).is_ok()); diff --git a/src/common/function/src/scalars/aggregate/polyval.rs b/src/common/function/src/scalars/aggregate/polyval.rs index 409137212e..0a8fc818c5 100644 --- a/src/common/function/src/scalars/aggregate/polyval.rs +++ b/src/common/function/src/scalars/aggregate/polyval.rs @@ -60,9 +60,10 @@ where impl Accumulator for Polyval where T: WrapperType, - PolyT: WrapperType, T::Native: AsPrimitive, + PolyT: WrapperType + std::iter::Sum<::Native>, PolyT::Native: std::ops::Mul + std::iter::Sum, + i64: AsPrimitive<::Native>, { fn state(&self) -> Result> { let nums = self @@ -73,7 +74,7 @@ where Ok(vec![ Value::List(ListValue::new( Some(Box::new(nums)), - T::default().into().data_type(), + T::LogicalType::build_data_type(), )), self.x.into(), ]) @@ -106,7 +107,7 @@ where }); let x = &values[1]; - let x = Helper::check_get_scalar::(x).context(error::InvalidInputsSnafu { + let x = Helper::check_get_scalar::(x).context(error::InvalidInputTypeSnafu { err_msg: "expecting \"POLYVAL\" function's second argument to be a positive integer", })?; // `get(0)` is safe because we have checked `values[1].len() == values[0].len() != 0` @@ -175,12 +176,14 @@ where ), })?; for value in values.values_iter() { - let value = value.context(FromScalarValueSnafu)?; - let column: &::VectorType = unsafe { Helper::static_cast(&value) }; - for v in column.iter_data().flatten() { - self.push(v); + if let Some(value) = value.context(FromScalarValueSnafu)? { + let column: &::VectorType = unsafe { Helper::static_cast(&value) }; + for v in column.iter_data().flatten() { + self.push(v); + } } } + Ok(()) } @@ -199,7 +202,7 @@ where .values .iter() .enumerate() - .map(|(i, &value)| value.as_() * (x.pow((len - 1 - i) as u32)).as_()) + .map(|(i, &value)| value.into_native().as_() * x.pow((len - 1 - i) as u32).as_()) .sum(); Ok(polyval.into()) } @@ -216,7 +219,7 @@ impl AggregateFunctionCreator for PolyvalAccumulatorCreator { with_match_primitive_type_id!( input_type.logical_type_id(), |$S| { - Ok(Box::new(Polyval::<$S,<<$S as LogicalPrimitiveType>::LargestType as LogicalPrimitiveType>::Wrapper>::default())) + Ok(Box::new(Polyval::<<$S as LogicalPrimitiveType>::Wrapper, <<$S as LogicalPrimitiveType>::LargestType as LogicalPrimitiveType>::Wrapper>::default())) }, { let err_msg = format!( @@ -257,7 +260,7 @@ impl AggregateFunctionCreator for PolyvalAccumulatorCreator { #[cfg(test)] mod test { - use datatypes::vectors::PrimitiveVector; + use datatypes::vectors::Int32Vector; use super::*; #[test] @@ -271,8 +274,8 @@ mod test { // test update one not-null value let mut polyval = Polyval::::default(); let v: Vec = vec![ - Arc::new(PrimitiveVector::::from(vec![Some(3)])), - Arc::new(PrimitiveVector::::from(vec![Some(2_i64)])), + Arc::new(Int32Vector::from(vec![Some(3)])), + Arc::new(Int64Vector::from(vec![Some(2_i64)])), ]; assert!(polyval.update_batch(&v).is_ok()); assert_eq!(Value::Int64(3), polyval.evaluate().unwrap()); @@ -280,8 +283,8 @@ mod test { // test update one null value let mut polyval = Polyval::::default(); let v: Vec = vec![ - Arc::new(PrimitiveVector::::from(vec![Option::::None])), - Arc::new(PrimitiveVector::::from(vec![Some(2_i64)])), + Arc::new(Int32Vector::from(vec![Option::::None])), + Arc::new(Int64Vector::from(vec![Some(2_i64)])), ]; assert!(polyval.update_batch(&v).is_ok()); assert_eq!(Value::Null, polyval.evaluate().unwrap()); @@ -289,12 +292,8 @@ mod test { // test update no null-value batch let mut polyval = Polyval::::default(); let v: Vec = vec![ - Arc::new(PrimitiveVector::::from(vec![ - Some(3), - Some(0), - Some(1), - ])), - Arc::new(PrimitiveVector::::from(vec![ + Arc::new(Int32Vector::from(vec![Some(3), Some(0), Some(1)])), + Arc::new(Int64Vector::from(vec![ Some(2_i64), Some(2_i64), Some(2_i64), @@ -306,13 +305,8 @@ mod test { // test update null-value batch let mut polyval = Polyval::::default(); let v: Vec = vec![ - Arc::new(PrimitiveVector::::from(vec![ - Some(3), - Some(0), - None, - Some(1), - ])), - Arc::new(PrimitiveVector::::from(vec![ + Arc::new(Int32Vector::from(vec![Some(3), Some(0), None, Some(1)])), + Arc::new(Int64Vector::from(vec![ Some(2_i64), Some(2_i64), Some(2_i64), @@ -326,10 +320,10 @@ mod test { let mut polyval = Polyval::::default(); let v: Vec = vec![ Arc::new(ConstantVector::new( - Arc::new(PrimitiveVector::::from_vec(vec![4])), + Arc::new(Int32Vector::from_vec(vec![4])), 2, )), - Arc::new(PrimitiveVector::::from(vec![Some(5_i64), Some(5_i64)])), + Arc::new(Int64Vector::from(vec![Some(5_i64), Some(5_i64)])), ]; assert!(polyval.update_batch(&v).is_ok()); assert_eq!(Value::Int64(24), polyval.evaluate().unwrap()); diff --git a/src/common/function/src/scalars/math/pow.rs b/src/common/function/src/scalars/math/pow.rs index fe28789e9b..6a4e1937dd 100644 --- a/src/common/function/src/scalars/math/pow.rs +++ b/src/common/function/src/scalars/math/pow.rs @@ -19,6 +19,7 @@ use common_query::error::Result; use common_query::prelude::{Signature, Volatility}; use datatypes::data_type::DataType; use datatypes::prelude::ConcreteDataType; +use datatypes::types::LogicalPrimitiveType; use datatypes::vectors::VectorRef; use datatypes::with_match_primitive_type_id; use num::traits::Pow; @@ -46,7 +47,7 @@ impl Function for PowFunction { fn eval(&self, _func_ctx: FunctionContext, columns: &[VectorRef]) -> Result { with_match_primitive_type_id!(columns[0].data_type().logical_type_id(), |$S| { with_match_primitive_type_id!(columns[1].data_type().logical_type_id(), |$T| { - let col = scalar_binary_op::<$S, $T, f64, _>(&columns[0], &columns[1], scalar_pow, &mut EvalContext::default())?; + let col = scalar_binary_op::<<$S as LogicalPrimitiveType>::Native, <$T as LogicalPrimitiveType>::Native, f64, _>(&columns[0], &columns[1], scalar_pow, &mut EvalContext::default())?; Ok(Arc::new(col)) },{ unreachable!() diff --git a/src/common/function/src/scalars/numpy/clip.rs b/src/common/function/src/scalars/numpy/clip.rs index be58614d70..fc2c97a55a 100644 --- a/src/common/function/src/scalars/numpy/clip.rs +++ b/src/common/function/src/scalars/numpy/clip.rs @@ -38,8 +38,23 @@ macro_rules! define_eval { with_match_primitive_type_id!(columns[1].data_type().logical_type_id(), |$T| { with_match_primitive_type_id!(columns[2].data_type().logical_type_id(), |$R| { // clip(a, min, max) is equals to min(max(a, min), max) - let col: VectorRef = Arc::new(scalar_binary_op::<<$S as LogicalPrimitiveType>::Wrapper, <$T as LogicalPrimitiveType>::Wrapper, $O, _>(&columns[0], &columns[1], scalar_max, &mut EvalContext::default())?); - let col = scalar_binary_op::<$O, <$R as LogicalPrimitiveType>::Wrapper, $O, _>(&col, &columns[2], scalar_min, &mut EvalContext::default())?; + let col: VectorRef = Arc::new(scalar_binary_op::< + <$S as LogicalPrimitiveType>::Wrapper, + <$T as LogicalPrimitiveType>::Wrapper, + $O, + _, + >( + &columns[0], + &columns[1], + scalar_max, + &mut EvalContext::default(), + )?); + let col = scalar_binary_op::<$O, <$R as LogicalPrimitiveType>::Wrapper, $O, _>( + &col, + &columns[2], + scalar_min, + &mut EvalContext::default(), + )?; Ok(Arc::new(col)) }, { unreachable!() diff --git a/src/common/function/src/scalars/test.rs b/src/common/function/src/scalars/test.rs index 7d74ff5d83..8e81d1f025 100644 --- a/src/common/function/src/scalars/test.rs +++ b/src/common/function/src/scalars/test.rs @@ -15,11 +15,11 @@ use std::fmt; use std::sync::Arc; +use common_query::error::Result; use common_query::prelude::{Signature, Volatility}; use datatypes::data_type::ConcreteDataType; use datatypes::prelude::VectorRef; -use crate::error::Result; use crate::scalars::expression::{scalar_binary_op, EvalContext}; use crate::scalars::function::{Function, FunctionContext}; diff --git a/src/common/function/src/scalars/timestamp/from_unixtime.rs b/src/common/function/src/scalars/timestamp/from_unixtime.rs index ab8197d61b..8b3ce7478a 100644 --- a/src/common/function/src/scalars/timestamp/from_unixtime.rs +++ b/src/common/function/src/scalars/timestamp/from_unixtime.rs @@ -17,16 +17,16 @@ use std::fmt; use std::sync::Arc; -use common_query::error::{IntoVectorSnafu, UnsupportedInputDataTypeSnafu}; +use common_query::error::{ + ArrowComputeSnafu, IntoVectorSnafu, Result, UnsupportedInputDataTypeSnafu, +}; use common_query::prelude::{Signature, Volatility}; -use datatypes::arrow::compute::arithmetics; -use datatypes::arrow::datatypes::DataType as ArrowDatatype; -use datatypes::arrow::scalar::PrimitiveScalar; +use datatypes::arrow::compute; +use datatypes::arrow::datatypes::{DataType as ArrowDatatype, Int64Type}; use datatypes::prelude::ConcreteDataType; -use datatypes::vectors::{TimestampVector, VectorRef}; +use datatypes::vectors::{TimestampMillisecondVector, VectorRef}; use snafu::ResultExt; -use crate::error::Result; use crate::scalars::function::{Function, FunctionContext}; #[derive(Clone, Debug, Default)] @@ -56,15 +56,15 @@ impl Function for FromUnixtimeFunction { ConcreteDataType::Int64(_) => { let array = columns[0].to_arrow_array(); // Our timestamp vector's time unit is millisecond - let array = arithmetics::mul_scalar( - &*array, - &PrimitiveScalar::new(ArrowDatatype::Int64, Some(1000i64)), - ); + let array = compute::multiply_scalar_dyn::(&array, 1000i64) + .context(ArrowComputeSnafu)?; Ok(Arc::new( - TimestampVector::try_from_arrow_array(array).context(IntoVectorSnafu { - data_type: ArrowDatatype::Int64, - })?, + TimestampMillisecondVector::try_from_arrow_array(array).context( + IntoVectorSnafu { + data_type: ArrowDatatype::Int64, + }, + )?, )) } _ => UnsupportedInputDataTypeSnafu {