From 67f887da413b32ec640bc03a1e090566a438745f Mon Sep 17 00:00:00 2001
From: Yijun Zhao <ariesdevil77@gmail.com>
Date: Sat, 7 Oct 2023 21:16:42 +0800
Subject: [PATCH] fix reviewer comments

---
 src/query/expression/src/evaluator.rs         | 26 +++++--------------
 src/query/sql/src/evaluator/block_operator.rs | 15 +++++++----
 2 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/src/query/expression/src/evaluator.rs b/src/query/expression/src/evaluator.rs
index f245e5436616f..32a0f88837cb6 100644
--- a/src/query/expression/src/evaluator.rs
+++ b/src/query/expression/src/evaluator.rs
@@ -104,23 +104,11 @@ impl<'a> Evaluator<'a> {
         }
     }
 
-    pub fn run_exprs(&self, exprs: &[Expr]) -> Result<Vec<BlockEntry>> {
-        let mut columns: Vec<BlockEntry> = Vec::with_capacity(exprs.len());
-        for expr in exprs {
-            // try get result from cache
-            if let Some(cached_values) = &self.cached_values {
-                if let Some(cached_result) = cached_values.borrow().get(expr) {
-                    let col = BlockEntry::new(expr.data_type().clone(), cached_result.clone());
-                    columns.push(col);
-                    continue;
-                }
-            }
-
-            let result = self.run(expr)?;
-            let col = BlockEntry::new(expr.data_type().clone(), result);
-            columns.push(col);
-        }
-        Ok(columns)
+    pub fn run_exprs(&self, exprs: &[Expr]) -> Result<Vec<Value<AnyType>>> {
+        exprs
+            .iter()
+            .map(|expr| self.run(expr))
+            .collect::<Result<Vec<_>>>()
     }
 
     pub fn run(&self, expr: &Expr) -> Result<Value<AnyType>> {
@@ -138,7 +126,7 @@ impl<'a> Evaluator<'a> {
         self.check_expr(expr);
 
         // try get result from cache
-        if let Some(cached_values) = &self.cached_values {
+        if !matches!(expr, Expr::ColumnRef {..} | Expr::Constant{..}) && let Some(cached_values) = &self.cached_values {
             if let Some(cached_result) = cached_values.borrow().get(expr) {
                 return Ok(cached_result.clone());
             }
@@ -235,7 +223,7 @@ impl<'a> Evaluator<'a> {
                             .enumerate()
                             .collect(),
                         self.func_ctx,
-                        self.fn_registry
+                        self.fn_registry,
                     )
                     .1,
                     None,
diff --git a/src/query/sql/src/evaluator/block_operator.rs b/src/query/sql/src/evaluator/block_operator.rs
index aa910dd84b872..a32d267f95144 100644
--- a/src/query/sql/src/evaluator/block_operator.rs
+++ b/src/query/sql/src/evaluator/block_operator.rs
@@ -91,14 +91,19 @@ impl BlockOperator {
                         None => Ok(input),
                     }
                 } else {
-                    // Local variable run in single thread, so `Rc` and `RefCell` are enough.
                     let evaluator =
                         Evaluator::new(&input, func_ctx, &BUILTIN_FUNCTIONS).with_cache();
-                    let result = evaluator.run_exprs(exprs)?;
-                    let block = DataBlock::new(result, input.num_rows());
+                    let results = evaluator.run_exprs(exprs)?;
+                    results
+                        .into_iter()
+                        .zip(exprs.iter())
+                        .for_each(|(result, expr)| {
+                            let col = BlockEntry::new(expr.data_type().clone(), result);
+                            input.add_column(col);
+                        });
                     match projections {
-                        Some(projections) => Ok(block.project(projections)),
-                        None => Ok(block),
+                        Some(projections) => Ok(input.project(projections)),
+                        None => Ok(input),
                     }
                 }
             }