Merge pull request #9168 from lichuang/issue_9057

refactor: remove column to_values function

Author: mergify[bot]

src/query/datavalues/src/columns/column.rs

@@ -149,10 +149,6 @@ pub trait Column: Send + Sync {
         DFTryFrom::try_from(value)
     }
 
-    fn to_values(&self) -> Vec<DataValue> {
-        (0..self.len()).map(|i| self.get(i)).collect()
-    }
-
     /// Visit each row value of Column
     fn for_each<F>(&self, f: F)
     where

src/query/functions/src/aggregates/aggregate_approx_count_distinct.rs

@@ -120,9 +120,7 @@ impl AggregateFunction for AggregateApproxCountDistinctFunction {
     fn accumulate_row(&self, place: StateAddr, columns: &[ColumnRef], _row: usize) -> Result<()> {
         let state = place.get::<AggregateApproxCountDistinctState>();
         for column in columns {
-            column.to_values().iter().for_each(|value| {
-                state.hll.push(value);
-            });
+            (0..column.len()).for_each(|i| state.hll.push(&column.get(i)));
         }
         Ok(())
     }

src/query/pipeline/sinks/src/processors/sinks/subquery_receive_sink.rs

@@ -73,7 +73,7 @@ impl Sink for SubqueryReceiveSink {
     fn consume(&mut self, data_block: DataBlock) -> Result<()> {
         for column_index in 0..data_block.num_columns() {
             let column = data_block.column(column_index);
-            let mut values = column.to_values();
+            let mut values = (0..column.len()).map(|i| column.get(i)).collect();
             self.input_columns[column_index].append(&mut values)
         }
 

src/query/service/tests/it/storages/statistics/column_statistics.rs

@@ -19,6 +19,7 @@ use common_datavalues::ColumnRef;
 use common_datavalues::DataField;
 use common_datavalues::DataSchemaRefExt;
 use common_datavalues::DataTypeImpl;
+use common_datavalues::DataValue;
 use common_datavalues::Series;
 use common_datavalues::SeriesFrom;
 use common_datavalues::StructColumn;
@@ -102,15 +103,17 @@ fn test_column_statistic() -> Result<()> {
 
     (0..5).for_each(|i| {
         let stats = col_stats.get(&(i as u32)).unwrap();
+        let column = &sample_cols[i];
+        let values: Vec<DataValue> = (0..column.len()).map(|i| column.get(i)).collect();
         assert_eq!(
             &stats.min,
-            sample_cols[i].to_values().iter().min().unwrap(),
+            values.iter().min().unwrap(),
             "checking min of col {}",
             i
         );
         assert_eq!(
             &stats.max,
-            sample_cols[i].to_values().iter().max().unwrap(),
+            values.iter().max().unwrap(),
             "checking max of col {}",
             i
         );

src/query/storages/index/benches/build_from_block.rs

@@ -44,17 +44,18 @@ fn bench_u64(c: &mut Criterion) {
     let column = block.try_column_by_name("a").unwrap();
 
     let mut builder = Xor8Builder::create();
-    builder.add_keys(&column.to_values());
+    (0..column.len()).for_each(|i| builder.add_key(&column.get(i)));
     let filter = builder.build().unwrap();
 
-    for key in column.to_values() {
+    for i in 0..column.len() {
+        let key = column.get(i);
         assert!(filter.contains(&key), "key {} present", key);
     }
 
     c.bench_function("xor8_filter_u64_1m_rows_build_from_column_to_values", |b| {
         b.iter(|| {
             let mut builder = Xor8Builder::create();
-            builder.add_keys(&criterion::black_box(column.to_values()));
+            (0..column.len()).for_each(|i| builder.add_key(&column.get(i)));
             let _filter = criterion::black_box(builder.build().unwrap());
         })
     });
@@ -65,10 +66,11 @@ fn bench_string(c: &mut Criterion) {
     let column = block.try_column_by_name("a").unwrap();
 
     let mut builder = Xor8Builder::create();
-    builder.add_keys(&column.to_values());
+    (0..column.len()).for_each(|i| builder.add_key(&column.get(i)));
     let filter = builder.build().unwrap();
 
-    for key in column.to_values() {
+    for i in 0..column.len() {
+        let key = column.get(i);
         assert!(filter.contains(&key), "key {} present", key);
     }
 
@@ -77,7 +79,7 @@ fn bench_string(c: &mut Criterion) {
         |b| {
             b.iter(|| {
                 let mut builder = Xor8Builder::create();
-                builder.add_keys(&criterion::black_box(column.to_values()));
+                (0..column.len()).for_each(|i| builder.add_key(&column.get(i)));
                 let _filter = criterion::black_box(builder.build().unwrap());
             })
         },

src/query/storages/index/tests/it/filters/xor8.rs

@@ -195,10 +195,11 @@ fn test_xor_bitmap_data_block() -> Result<()> {
     let column = block.try_column_by_name("a")?;
 
     let mut builder = Xor8Builder::create();
-    builder.add_keys(&column.to_values());
+    let values: Vec<DataValue> = (0..column.len()).map(|i| column.get(i)).collect();
+    builder.add_keys(&values);
     let filter = builder.build()?;
 
-    for key in column.to_values() {
+    for key in values {
         assert!(filter.contains(&key), "key {} not present", key);
     }