Merge pull request #9129 from sundy-li/improve-concat

mergify[bot] · web-flow · commit 7d5a8825945f · 2022-12-08T01:32:30.000Z
chore(query): improve concat kernel in new expression
diff --git a/src/query/expression/src/kernels/concat.rs b/src/query/expression/src/kernels/concat.rs
@@ -182,10 +182,9 @@ impl Column {
             .iter()
             .map(|c| T::try_downcast_column(c).unwrap())
             .collect();
+
         for col in columns {
-            for item in T::iter_column(&col) {
-                T::push_item(&mut builder, item)
-            }
+            T::append_column(&mut builder, &col);
         }
         T::upcast_column(T::build_column(builder))
     }
diff --git a/src/query/expression/src/types.rs b/src/query/expression/src/types.rs
@@ -230,7 +230,7 @@ pub trait ValueType: Debug + Clone + PartialEq + Sized + 'static {
     fn builder_len(builder: &Self::ColumnBuilder) -> usize;
     fn push_item(builder: &mut Self::ColumnBuilder, item: Self::ScalarRef<'_>);
     fn push_default(builder: &mut Self::ColumnBuilder);
-    fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder);
+    fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column);
     fn build_column(builder: Self::ColumnBuilder) -> Self::Column;
     fn build_scalar(builder: Self::ColumnBuilder) -> Self::Scalar;
 
diff --git a/src/query/expression/src/types/any.rs b/src/query/expression/src/types/any.rs
@@ -116,8 +116,8 @@ impl ValueType for AnyType {
         builder.push_default();
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other: &Self::ColumnBuilder) {
-        builder.append(other);
+    fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {
+        builder.append_column(other);
     }
 
     fn build_column(builder: Self::ColumnBuilder) -> Self::Column {
diff --git a/src/query/expression/src/types/array.rs b/src/query/expression/src/types/array.rs
@@ -129,8 +129,8 @@ impl<T: ValueType> ValueType for ArrayType<T> {
         builder.push_default();
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder) {
-        builder.append(other_builder);
+    fn append_column(builder: &mut Self::ColumnBuilder, other_builder: &Self::Column) {
+        builder.append_column(other_builder);
     }
 
     fn build_column(builder: Self::ColumnBuilder) -> Self::Column {
@@ -268,10 +268,9 @@ impl<T: ValueType> ArrayColumnBuilder<T> {
 
     pub fn repeat(array: &T::Column, n: usize) -> Self {
         let mut builder = T::column_to_builder(array.clone());
-        let builder_clone = builder.clone();
         let len = T::builder_len(&builder);
         for _ in 1..n {
-            T::append_builder(&mut builder, &builder_clone)
+            T::append_column(&mut builder, array)
         }
         let offsets = once(0)
             .chain((0..n).map(|i| (len * (i + 1)) as u64))
@@ -296,8 +295,7 @@ impl<T: ValueType> ArrayColumnBuilder<T> {
     }
 
     pub fn push(&mut self, item: T::Column) {
-        let other_col = T::column_to_builder(item);
-        T::append_builder(&mut self.builder, &other_col);
+        T::append_column(&mut self.builder, &item);
         let len = T::builder_len(&self.builder);
         self.offsets.push(len as u64);
     }
@@ -307,8 +305,8 @@ impl<T: ValueType> ArrayColumnBuilder<T> {
         self.offsets.push(len as u64);
     }
 
-    pub fn append(&mut self, other: &Self) {
-        T::append_builder(&mut self.builder, &other.builder);
+    pub fn append_column(&mut self, other: &ArrayColumn<T>) {
+        T::append_column(&mut self.builder, &other.values);
         let end = self.offsets.last().cloned().unwrap();
         self.offsets
             .extend(other.offsets.iter().skip(1).map(|offset| offset + end));
diff --git a/src/query/expression/src/types/boolean.rs b/src/query/expression/src/types/boolean.rs
@@ -130,8 +130,8 @@ impl ValueType for BooleanType {
         builder.push(false);
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder) {
-        builder.extend_from_slice(other_builder.as_slice(), 0, other_builder.len());
+    fn append_column(builder: &mut Self::ColumnBuilder, bitmap: &Self::Column) {
+        builder.extend_from_bitmap(bitmap)
     }
 
     fn build_column(builder: Self::ColumnBuilder) -> Self::Column {
diff --git a/src/query/expression/src/types/date.rs b/src/query/expression/src/types/date.rs
@@ -157,7 +157,7 @@ impl ValueType for DateType {
         builder.push(Self::Scalar::default());
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other: &Self::ColumnBuilder) {
+    fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {
         builder.extend_from_slice(other);
     }
 
diff --git a/src/query/expression/src/types/empty_array.rs b/src/query/expression/src/types/empty_array.rs
@@ -127,7 +127,7 @@ impl ValueType for EmptyArrayType {
         *len += 1
     }
 
-    fn append_builder(len: &mut Self::ColumnBuilder, other_len: &Self::ColumnBuilder) {
+    fn append_column(len: &mut Self::ColumnBuilder, other_len: &Self::Column) {
         *len += other_len
     }
 
diff --git a/src/query/expression/src/types/generic.rs b/src/query/expression/src/types/generic.rs
@@ -119,8 +119,8 @@ impl<const INDEX: usize> ValueType for GenericType<INDEX> {
         builder.push_default();
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other: &Self::ColumnBuilder) {
-        builder.append(other);
+    fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {
+        builder.append_column(other);
     }
 
     fn build_column(builder: Self::ColumnBuilder) -> Self::Column {
diff --git a/src/query/expression/src/types/map.rs b/src/query/expression/src/types/map.rs
@@ -141,8 +141,8 @@ impl<K: ValueType, V: ValueType> ValueType for KvPair<K, V> {
         builder.push_default();
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder) {
-        builder.append(other_builder);
+    fn append_column(builder: &mut Self::ColumnBuilder, other_builder: &Self::Column) {
+        builder.append_column(other_builder);
     }
 
     fn build_column(builder: Self::ColumnBuilder) -> Self::Column {
@@ -249,9 +249,9 @@ impl<K: ValueType, V: ValueType> KvColumnBuilder<K, V> {
         V::push_default(&mut self.values);
     }
 
-    pub fn append(&mut self, other: &Self) {
-        K::append_builder(&mut self.keys, &other.keys);
-        V::append_builder(&mut self.values, &other.values);
+    pub fn append_column(&mut self, other: &KvColumn<K, V>) {
+        K::append_column(&mut self.keys, &other.keys);
+        V::append_column(&mut self.values, &other.values);
     }
 
     pub fn build(self) -> KvColumn<K, V> {
@@ -392,8 +392,8 @@ impl<T: ValueType> ValueType for MapType<T> {
         <MapInternal<T> as ValueType>::push_default(builder)
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder) {
-        <MapInternal<T> as ValueType>::append_builder(builder, other_builder)
+    fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {
+        <MapInternal<T> as ValueType>::append_column(builder, other)
     }
 
     fn build_column(builder: Self::ColumnBuilder) -> Self::Column {
diff --git a/src/query/expression/src/types/null.rs b/src/query/expression/src/types/null.rs
@@ -134,7 +134,7 @@ impl ValueType for NullType {
         *len += 1
     }
 
-    fn append_builder(len: &mut Self::ColumnBuilder, other_len: &Self::ColumnBuilder) {
+    fn append_column(len: &mut Self::ColumnBuilder, other_len: &Self::Column) {
         *len += other_len
     }
 
diff --git a/src/query/expression/src/types/nullable.rs b/src/query/expression/src/types/nullable.rs
@@ -154,8 +154,8 @@ impl<T: ValueType> ValueType for NullableType<T> {
         builder.push_null();
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder) {
-        builder.append(other_builder);
+    fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {
+        builder.append_column(other);
     }
 
     fn build_column(builder: Self::ColumnBuilder) -> Self::Column {
@@ -317,10 +317,9 @@ impl<T: ValueType> NullableColumnBuilder<T> {
         self.validity.push(false);
     }
 
-    pub fn append(&mut self, other: &Self) {
-        T::append_builder(&mut self.builder, &other.builder);
-        self.validity
-            .extend_from_slice(other.validity.as_slice(), 0, other.validity.len());
+    pub fn append_column(&mut self, other: &NullableColumn<T>) {
+        T::append_column(&mut self.builder, &other.column);
+        self.validity.extend_from_bitmap(&other.validity)
     }
 
     pub fn build(self) -> NullableColumn<T> {
diff --git a/src/query/expression/src/types/number.rs b/src/query/expression/src/types/number.rs
@@ -145,7 +145,7 @@ impl<Num: Number> ValueType for NumberType<Num> {
         builder.push(Num::default());
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other: &Self::ColumnBuilder) {
+    fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {
         builder.extend_from_slice(other);
     }
 
@@ -536,13 +536,10 @@ impl NumberColumnBuilder {
         })
     }
 
-    pub fn append(&mut self, other: &NumberColumnBuilder) {
+    pub fn append_column(&mut self, other: &NumberColumn) {
         crate::with_number_type!(|NUM_TYPE| match (self, other) {
-            (
-                NumberColumnBuilder::NUM_TYPE(builder),
-                NumberColumnBuilder::NUM_TYPE(other_builder),
-            ) => {
-                builder.extend_from_slice(other_builder);
+            (NumberColumnBuilder::NUM_TYPE(builder), NumberColumn::NUM_TYPE(other)) => {
+                builder.extend_from_slice(other);
             }
             (this, other) => unreachable!("unable append {other:?} onto {this:?}"),
         })
diff --git a/src/query/expression/src/types/string.rs b/src/query/expression/src/types/string.rs
@@ -128,8 +128,8 @@ impl ValueType for StringType {
         builder.commit_row();
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder) {
-        builder.append(other_builder)
+    fn append_column(builder: &mut Self::ColumnBuilder, other_builder: &Self::Column) {
+        builder.append_column(other_builder)
     }
 
     fn build_column(builder: Self::ColumnBuilder) -> Self::Column {
@@ -310,7 +310,7 @@ impl StringColumnBuilder {
         self.offsets.push(self.data.len() as u64);
     }
 
-    pub fn append(&mut self, other: &Self) {
+    pub fn append_column(&mut self, other: &StringColumn) {
         // the first offset of other column may not be zero
         let other_start = *other.offsets.first().unwrap();
         let start = self.offsets.last().cloned().unwrap();
diff --git a/src/query/expression/src/types/timestamp.rs b/src/query/expression/src/types/timestamp.rs
@@ -159,7 +159,7 @@ impl ValueType for TimestampType {
         builder.push(Self::Scalar::default());
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other: &Self::ColumnBuilder) {
+    fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {
         builder.extend_from_slice(other);
     }
 
diff --git a/src/query/expression/src/types/variant.rs b/src/query/expression/src/types/variant.rs
@@ -137,8 +137,8 @@ impl ValueType for VariantType {
         builder.commit_row();
     }
 
-    fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder) {
-        builder.append(other_builder)
+    fn append_column(builder: &mut Self::ColumnBuilder, other_builder: &Self::Column) {
+        builder.append_column(other_builder)
     }
 
     fn build_column(builder: Self::ColumnBuilder) -> Self::Column {
diff --git a/src/query/expression/src/utils/arrow.rs b/src/query/expression/src/utils/arrow.rs
@@ -44,8 +44,8 @@ pub fn repeat_bitmap(bitmap: &mut Bitmap, n: usize) -> MutableBitmap {
     builder
 }
 
-pub fn append_bitmap(bitmap: &mut MutableBitmap, other: &MutableBitmap) {
-    bitmap.extend_from_slice(other.as_slice(), 0, other.len());
+pub fn append_bitmap(bitmap: &mut MutableBitmap, other: &Bitmap) {
+    bitmap.extend_from_bitmap(other)
 }
 
 pub fn constant_bitmap(value: bool, len: usize) -> MutableBitmap {
diff --git a/src/query/expression/src/values.rs b/src/query/expression/src/values.rs
@@ -1311,45 +1311,45 @@ impl ColumnBuilder {
         }
     }
 
-    pub fn append(&mut self, other: &ColumnBuilder) {
+    pub fn append_column(&mut self, other: &Column) {
         match (self, other) {
-            (ColumnBuilder::Null { len }, ColumnBuilder::Null { len: other_len }) => {
+            (ColumnBuilder::Null { len }, Column::Null { len: other_len }) => {
                 *len += other_len;
             }
-            (ColumnBuilder::EmptyArray { len }, ColumnBuilder::EmptyArray { len: other_len }) => {
+            (ColumnBuilder::EmptyArray { len }, Column::EmptyArray { len: other_len }) => {
                 *len += other_len;
             }
-            (ColumnBuilder::Number(builder), ColumnBuilder::Number(other_builder)) => {
-                builder.append(other_builder);
+            (ColumnBuilder::Number(builder), Column::Number(column)) => {
+                builder.append_column(column);
             }
-            (ColumnBuilder::Boolean(builder), ColumnBuilder::Boolean(other_builder)) => {
-                append_bitmap(builder, other_builder);
+            (ColumnBuilder::Boolean(builder), Column::Boolean(other)) => {
+                append_bitmap(builder, other);
             }
-            (ColumnBuilder::String(builder), ColumnBuilder::String(other_builder)) => {
-                builder.append(other_builder);
+            (ColumnBuilder::String(builder), Column::String(other)) => {
+                builder.append_column(other);
             }
-            (ColumnBuilder::Timestamp(builder), ColumnBuilder::Timestamp(other_builder)) => {
-                builder.extend_from_slice(other_builder);
+            (ColumnBuilder::Timestamp(builder), Column::Timestamp(other)) => {
+                builder.extend_from_slice(other);
             }
-            (ColumnBuilder::Date(builder), ColumnBuilder::Date(other_builder)) => {
-                builder.extend_from_slice(other_builder);
+            (ColumnBuilder::Date(builder), Column::Date(other)) => {
+                builder.extend_from_slice(other);
             }
-            (ColumnBuilder::Array(builder), ColumnBuilder::Array(other_builder)) => {
-                builder.append(other_builder);
+            (ColumnBuilder::Array(builder), Column::Array(other)) => {
+                builder.append_column(other.as_ref());
             }
-            (ColumnBuilder::Nullable(builder), ColumnBuilder::Nullable(other_builder)) => {
-                builder.append(other_builder);
+            (ColumnBuilder::Nullable(builder), Column::Nullable(other)) => {
+                builder.append_column(other);
             }
             (
                 ColumnBuilder::Tuple { fields, len },
-                ColumnBuilder::Tuple {
+                Column::Tuple {
                     fields: other_fields,
                     len: other_len,
                 },
             ) => {
                 assert_eq!(fields.len(), other_fields.len());
                 for (field, other_field) in fields.iter_mut().zip(other_fields.iter()) {
-                    field.append(other_field);
+                    field.append_column(other_field);
                 }
                 *len += other_len;
             }

Original file line number	Diff line number	Diff line change
`@@ -182,10 +182,9 @@ impl Column {`
`182`	`182`	`.iter()`
`183`	`183`	`.map(\|c\| T::try_downcast_column(c).unwrap())`
`184`	`184`	`.collect();`
	`185`	`+`
`185`	`186`	`for col in columns {`
`186`		`- for item in T::iter_column(&col) {`
`187`		`- T::push_item(&mut builder, item)`
`188`		`- }`
	`187`	`+ T::append_column(&mut builder, &col);`
`189`	`188`	`}`
`190`	`189`	`T::upcast_column(T::build_column(builder))`
`191`	`190`	`}`
Original file line number	Diff line number	Diff line change
`@@ -116,8 +116,8 @@ impl ValueType for AnyType {`
`116`	`116`	`builder.push_default();`
`117`	`117`	`}`
`118`	`118`
`119`		`- fn append_builder(builder: &mut Self::ColumnBuilder, other: &Self::ColumnBuilder) {`
`120`		`- builder.append(other);`
	`119`	`+ fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {`
	`120`	`+ builder.append_column(other);`
`121`	`121`	`}`
`122`	`122`
`123`	`123`	`fn build_column(builder: Self::ColumnBuilder) -> Self::Column {`
Original file line number	Diff line number	Diff line change
`@@ -130,8 +130,8 @@ impl ValueType for BooleanType {`
`130`	`130`	`builder.push(false);`
`131`	`131`	`}`
`132`	`132`
`133`		`- fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder) {`
`134`		`- builder.extend_from_slice(other_builder.as_slice(), 0, other_builder.len());`
	`133`	`+ fn append_column(builder: &mut Self::ColumnBuilder, bitmap: &Self::Column) {`
	`134`	`+ builder.extend_from_bitmap(bitmap)`
`135`	`135`	`}`
`136`	`136`
`137`	`137`	`fn build_column(builder: Self::ColumnBuilder) -> Self::Column {`
Original file line number	Diff line number	Diff line change
`@@ -157,7 +157,7 @@ impl ValueType for DateType {`
`157`	`157`	`builder.push(Self::Scalar::default());`
`158`	`158`	`}`
`159`	`159`
`160`		`- fn append_builder(builder: &mut Self::ColumnBuilder, other: &Self::ColumnBuilder) {`
	`160`	`+ fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {`
`161`	`161`	`builder.extend_from_slice(other);`
`162`	`162`	`}`
`163`	`163`
Original file line number	Diff line number	Diff line change
`@@ -127,7 +127,7 @@ impl ValueType for EmptyArrayType {`
`127`	`127`	`*len += 1`
`128`	`128`	`}`
`129`	`129`
`130`		`- fn append_builder(len: &mut Self::ColumnBuilder, other_len: &Self::ColumnBuilder) {`
	`130`	`+ fn append_column(len: &mut Self::ColumnBuilder, other_len: &Self::Column) {`
`131`	`131`	`*len += other_len`
`132`	`132`	`}`
`133`	`133`
Original file line number	Diff line number	Diff line change
`@@ -119,8 +119,8 @@ impl<const INDEX: usize> ValueType for GenericType<INDEX> {`
`119`	`119`	`builder.push_default();`
`120`	`120`	`}`
`121`	`121`
`122`		`- fn append_builder(builder: &mut Self::ColumnBuilder, other: &Self::ColumnBuilder) {`
`123`		`- builder.append(other);`
	`122`	`+ fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {`
	`123`	`+ builder.append_column(other);`
`124`	`124`	`}`
`125`	`125`
`126`	`126`	`fn build_column(builder: Self::ColumnBuilder) -> Self::Column {`
Original file line number	Diff line number	Diff line change
`@@ -141,8 +141,8 @@ impl<K: ValueType, V: ValueType> ValueType for KvPair<K, V> {`
`141`	`141`	`builder.push_default();`
`142`	`142`	`}`
`143`	`143`
`144`		`- fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder) {`
`145`		`- builder.append(other_builder);`
	`144`	`+ fn append_column(builder: &mut Self::ColumnBuilder, other_builder: &Self::Column) {`
	`145`	`+ builder.append_column(other_builder);`
`146`	`146`	`}`
`147`	`147`
`148`	`148`	`fn build_column(builder: Self::ColumnBuilder) -> Self::Column {`
`@@ -249,9 +249,9 @@ impl<K: ValueType, V: ValueType> KvColumnBuilder<K, V> {`
`249`	`249`	`V::push_default(&mut self.values);`
`250`	`250`	`}`
`251`	`251`
`252`		`- pub fn append(&mut self, other: &Self) {`
`253`		`- K::append_builder(&mut self.keys, &other.keys);`
`254`		`- V::append_builder(&mut self.values, &other.values);`
	`252`	`+ pub fn append_column(&mut self, other: &KvColumn<K, V>) {`
	`253`	`+ K::append_column(&mut self.keys, &other.keys);`
	`254`	`+ V::append_column(&mut self.values, &other.values);`
`255`	`255`	`}`
`256`	`256`
`257`	`257`	`pub fn build(self) -> KvColumn<K, V> {`
`@@ -392,8 +392,8 @@ impl<T: ValueType> ValueType for MapType<T> {`
`392`	`392`	`<MapInternal<T> as ValueType>::push_default(builder)`
`393`	`393`	`}`
`394`	`394`
`395`		`- fn append_builder(builder: &mut Self::ColumnBuilder, other_builder: &Self::ColumnBuilder) {`
`396`		`- <MapInternal<T> as ValueType>::append_builder(builder, other_builder)`
	`395`	`+ fn append_column(builder: &mut Self::ColumnBuilder, other: &Self::Column) {`
	`396`	`+ <MapInternal<T> as ValueType>::append_column(builder, other)`
`397`	`397`	`}`
`398`	`398`
`399`	`399`	`fn build_column(builder: Self::ColumnBuilder) -> Self::Column {`
Original file line number	Diff line number	Diff line change
`@@ -134,7 +134,7 @@ impl ValueType for NullType {`
`134`	`134`	`*len += 1`
`135`	`135`	`}`
`136`	`136`
`137`		`- fn append_builder(len: &mut Self::ColumnBuilder, other_len: &Self::ColumnBuilder) {`
	`137`	`+ fn append_column(len: &mut Self::ColumnBuilder, other_len: &Self::Column) {`
`138`	`138`	`*len += other_len`
`139`	`139`	`}`
`140`	`140`