From bbcd702c65a3673b5635ea6a32682d3d282e93fb Mon Sep 17 00:00:00 2001 From: Jk Xu <54522439+Dousir9@users.noreply.github.com> Date: Fri, 23 Feb 2024 12:12:01 +0800 Subject: [PATCH] chore(query): improve new filter execution (#14715) * improve new filter execution * chore: refine code --- src/query/expression/src/filter/select.rs | 18 +++--- src/query/expression/src/filter/select_op.rs | 17 ++++++ .../expression/src/filter/select_value/mod.rs | 56 +++++++++++++++---- .../src/filter/select_value/select_column.rs | 10 ++-- .../select_value/select_column_scalar.rs | 10 ++-- .../src/filter/select_value/select_scalar.rs | 10 ++-- src/query/expression/src/types.rs | 14 ----- 7 files changed, 89 insertions(+), 46 deletions(-) diff --git a/src/query/expression/src/filter/select.rs b/src/query/expression/src/filter/select.rs index 56ac4ea748db..5a99c2d1d71b 100644 --- a/src/query/expression/src/filter/select.rs +++ b/src/query/expression/src/filter/select.rs @@ -95,7 +95,7 @@ impl<'a> Selector<'a> { match left_data_type.remove_nullable() { DataType::Number(ty) => { with_number_mapped_type!(|T| match ty { - NumberDataType::T => self.select_type_values::>( + NumberDataType::T => self.select_type_values_cmp::>( &op, left, right, @@ -112,7 +112,7 @@ impl<'a> Selector<'a> { DataType::Decimal(ty) => { with_decimal_mapped_type!(|T| match ty { - DecimalDataType::T(_) => self.select_type_values::>( + DecimalDataType::T(_) => self.select_type_values_cmp::>( &op, left, right, @@ -126,7 +126,7 @@ impl<'a> Selector<'a> { ), }) } - DataType::Date => self.select_type_values::( + DataType::Date => self.select_type_values_cmp::( &op, left, right, @@ -138,7 +138,7 @@ impl<'a> Selector<'a> { select_strategy, count, ), - DataType::Timestamp => self.select_type_values::( + DataType::Timestamp => self.select_type_values_cmp::( &op, left, right, @@ -150,7 +150,7 @@ impl<'a> Selector<'a> { select_strategy, count, ), - DataType::String => self.select_type_values::( + DataType::String => self.select_type_values_cmp::( &op, left, right, @@ -162,7 +162,7 @@ impl<'a> Selector<'a> { select_strategy, count, ), - DataType::Variant => self.select_type_values::( + DataType::Variant => self.select_type_values_cmp::( &op, left, right, @@ -174,7 +174,7 @@ impl<'a> Selector<'a> { select_strategy, count, ), - DataType::Boolean => self.select_type_values::( + DataType::Boolean => self.select_type_values_cmp::( &op, left, right, @@ -186,7 +186,7 @@ impl<'a> Selector<'a> { select_strategy, count, ), - DataType::EmptyArray => self.select_type_values::( + DataType::EmptyArray => self.select_type_values_cmp::( &op, left, right, @@ -198,7 +198,7 @@ impl<'a> Selector<'a> { select_strategy, count, ), - _ => self.select_type_values::( + _ => self.select_type_values_cmp::( &op, left, right, diff --git a/src/query/expression/src/filter/select_op.rs b/src/query/expression/src/filter/select_op.rs index 788c533d3b3c..c7bda8d962d6 100644 --- a/src/query/expression/src/filter/select_op.rs +++ b/src/query/expression/src/filter/select_op.rs @@ -53,3 +53,20 @@ impl SelectOp { } } } + +#[macro_export] +macro_rules! with_mapped_cmp_method { + ( | $t:tt | $($tail:tt)* ) => { + match_template::match_template! { + $t = [ + Equal => equal, + NotEqual => not_equal, + Gt => greater_than, + Lt => less_than, + Gte => greater_than_equal, + Lte => less_than_equal, + ], + $($tail)* + } + } +} diff --git a/src/query/expression/src/filter/select_value/mod.rs b/src/query/expression/src/filter/select_value/mod.rs index 28334399ff50..f3fa84cf11e5 100644 --- a/src/query/expression/src/filter/select_value/mod.rs +++ b/src/query/expression/src/filter/select_value/mod.rs @@ -17,6 +17,7 @@ use databend_common_exception::Result; use crate::types::AnyType; use crate::types::ValueType; +use crate::with_mapped_cmp_method; use crate::SelectOp; use crate::SelectStrategy; use crate::Selector; @@ -28,7 +29,7 @@ mod select_scalar; impl<'a> Selector<'a> { #[allow(clippy::too_many_arguments)] - pub(crate) fn select_type_values( + pub(crate) fn select_type_values_cmp( &self, op: &SelectOp, left: Value, @@ -40,11 +41,44 @@ impl<'a> Selector<'a> { mutable_false_idx: &mut usize, select_strategy: SelectStrategy, count: usize, + ) -> Result { + with_mapped_cmp_method!(|OP| match op { + SelectOp::OP => self.select_type_values::( + T::OP, + left, + right, + validity, + true_selection, + false_selection, + mutable_true_idx, + mutable_false_idx, + select_strategy, + count, + ), + }) + } + + #[allow(clippy::too_many_arguments)] + pub(crate) fn select_type_values< + T: ValueType, + C: Fn(T::ScalarRef<'_>, T::ScalarRef<'_>) -> bool, + >( + &self, + cmp: C, + left: Value, + right: Value, + validity: Option, + true_selection: &mut [u32], + false_selection: (&mut [u32], bool), + mutable_true_idx: &mut usize, + mutable_false_idx: &mut usize, + select_strategy: SelectStrategy, + count: usize, ) -> Result { let has_false = false_selection.1; match (left, right) { - (Value::Scalar(left), Value::Scalar(right)) => self.select_scalars::( - op, + (Value::Scalar(left), Value::Scalar(right)) => self.select_scalars::( + cmp, left, right, true_selection, @@ -59,8 +93,8 @@ impl<'a> Selector<'a> { let right = T::try_downcast_column(&right).unwrap(); if has_false { - self.select_columns::( - op, + self.select_columns::( + cmp, left, right, validity, @@ -72,8 +106,8 @@ impl<'a> Selector<'a> { count, ) } else { - self.select_columns::( - op, + self.select_columns::( + cmp, left, right, validity, @@ -93,8 +127,8 @@ impl<'a> Selector<'a> { let scalar = T::try_downcast_scalar(&scalar).unwrap(); if has_false { - self.select_column_scalar::( - op, + self.select_column_scalar::( + cmp, column, scalar, validity, @@ -106,8 +140,8 @@ impl<'a> Selector<'a> { count, ) } else { - self.select_column_scalar::( - op, + self.select_column_scalar::( + cmp, column, scalar, validity, diff --git a/src/query/expression/src/filter/select_value/select_column.rs b/src/query/expression/src/filter/select_value/select_column.rs index b1348c2fb89a..e2aa34137cc0 100644 --- a/src/query/expression/src/filter/select_value/select_column.rs +++ b/src/query/expression/src/filter/select_value/select_column.rs @@ -15,7 +15,6 @@ use databend_common_arrow::arrow::bitmap::Bitmap; use databend_common_exception::Result; -use crate::filter::SelectOp; use crate::filter::SelectStrategy; use crate::filter::Selector; use crate::types::ValueType; @@ -23,9 +22,13 @@ use crate::types::ValueType; impl<'a> Selector<'a> { // Select indices by comparing two columns. #[allow(clippy::too_many_arguments)] - pub(crate) fn select_columns( + pub(crate) fn select_columns< + T: ValueType, + C: Fn(T::ScalarRef<'_>, T::ScalarRef<'_>) -> bool, + const FALSE: bool, + >( &self, - op: &SelectOp, + cmp: C, left: T::Column, right: T::Column, validity: Option, @@ -39,7 +42,6 @@ impl<'a> Selector<'a> { let mut true_idx = *mutable_true_idx; let mut false_idx = *mutable_false_idx; - let cmp = T::compare_operation(op); match select_strategy { SelectStrategy::True => unsafe { let start = *mutable_true_idx; diff --git a/src/query/expression/src/filter/select_value/select_column_scalar.rs b/src/query/expression/src/filter/select_value/select_column_scalar.rs index 502c9996f5d5..d1d7906fb1b1 100644 --- a/src/query/expression/src/filter/select_value/select_column_scalar.rs +++ b/src/query/expression/src/filter/select_value/select_column_scalar.rs @@ -15,7 +15,6 @@ use databend_common_arrow::arrow::bitmap::Bitmap; use databend_common_exception::Result; -use crate::filter::SelectOp; use crate::filter::SelectStrategy; use crate::filter::Selector; use crate::types::ValueType; @@ -23,9 +22,13 @@ use crate::types::ValueType; impl<'a> Selector<'a> { // Select indices by comparing scalar and column. #[allow(clippy::too_many_arguments)] - pub(crate) fn select_column_scalar( + pub(crate) fn select_column_scalar< + T: ValueType, + C: Fn(T::ScalarRef<'_>, T::ScalarRef<'_>) -> bool, + const FALSE: bool, + >( &self, - op: &SelectOp, + cmp: C, column: T::Column, scalar: T::ScalarRef<'a>, validity: Option, @@ -39,7 +42,6 @@ impl<'a> Selector<'a> { let mut true_idx = *mutable_true_idx; let mut false_idx = *mutable_false_idx; - let cmp = T::compare_operation(op); match select_strategy { SelectStrategy::True => unsafe { let start = *mutable_true_idx; diff --git a/src/query/expression/src/filter/select_value/select_scalar.rs b/src/query/expression/src/filter/select_value/select_scalar.rs index 5de9aa5227f0..c76f6285a767 100644 --- a/src/query/expression/src/filter/select_value/select_scalar.rs +++ b/src/query/expression/src/filter/select_value/select_scalar.rs @@ -18,14 +18,16 @@ use crate::filter::SelectStrategy; use crate::filter::Selector; use crate::types::ValueType; use crate::Scalar; -use crate::SelectOp; impl<'a> Selector<'a> { #[allow(clippy::too_many_arguments)] // Select indices by comparing two scalars. - pub(crate) fn select_scalars( + pub(crate) fn select_scalars< + T: ValueType, + C: Fn(T::ScalarRef<'_>, T::ScalarRef<'_>) -> bool, + >( &self, - op: &SelectOp, + cmp: C, left: Scalar, right: Scalar, true_selection: &mut [u32], @@ -39,7 +41,7 @@ impl<'a> Selector<'a> { let left = T::try_downcast_scalar(&left).unwrap(); let right = right.as_ref(); let right = T::try_downcast_scalar(&right).unwrap(); - let result = T::compare_operation(op)(left, right); + let result = cmp(left, right); let count = self.select_boolean_scalar_adapt( result, true_selection, diff --git a/src/query/expression/src/types.rs b/src/query/expression/src/types.rs index 69f42e5fdfbc..8a249f0cb05a 100755 --- a/src/query/expression/src/types.rs +++ b/src/query/expression/src/types.rs @@ -65,7 +65,6 @@ use crate::values::Column; use crate::values::Scalar; use crate::ColumnBuilder; use crate::ScalarRef; -use crate::SelectOp; pub type GenericMap = [DataType]; @@ -384,19 +383,6 @@ pub trait ValueType: Debug + Clone + PartialEq + Sized + 'static { None } - /// Return the comparison function for the given select operation, some data types not support comparison. - #[inline(always)] - fn compare_operation(op: &SelectOp) -> fn(Self::ScalarRef<'_>, Self::ScalarRef<'_>) -> bool { - match op { - SelectOp::Equal => Self::equal, - SelectOp::NotEqual => Self::not_equal, - SelectOp::Gt => Self::greater_than, - SelectOp::Gte => Self::greater_than_equal, - SelectOp::Lt => Self::less_than, - SelectOp::Lte => Self::less_than_equal, - } - } - /// Equal comparison between two scalars, some data types not support comparison. #[inline(always)] fn equal(left: Self::ScalarRef<'_>, right: Self::ScalarRef<'_>) -> bool {