fix: resolve CI failures after executor de-generic refactor

Author: KKould

examples/transaction.rs

@@ -41,10 +41,8 @@ mod app {
     pub fn run() -> Result<(), DatabaseError> {
         reset_example_dir()?;
         // Optimistic transactions are currently backed by RocksDB.
-        let database = DataBaseBuilder::path(EXAMPLE_DB_PATH).build_optimistic()?;
-        database
-            .run("create table if not exists t1 (c1 int primary key, c2 int)")?
-            .done()?;
+        let mut database = DataBaseBuilder::path(EXAMPLE_DB_PATH).build_optimistic()?;
+        database.ddl("create table if not exists t1 (c1 int primary key, c2 int)")?;
         let mut transaction = database.new_transaction()?;
 
         transaction
@@ -65,6 +63,7 @@ mod app {
             Tuple::new(None, vec![DataValue::Int32(1), DataValue::Int32(1)])
         );
         assert!(iter.next().is_none());
+        iter.done()?;
 
         let mut tx2 = database.new_transaction()?;
         tx2.run("update t1 set c2 = 99 where c1 = 0")?.done()?;
@@ -79,7 +78,7 @@ mod app {
         );
         drop(tx2);
 
-        database.run("drop table t1")?.done()?;
+        database.ddl("drop table t1")?;
 
         Ok(())
     }

src/binder/expr.rs

@@ -357,13 +357,12 @@ impl<'a, T: Transaction, A: AsRef<[(&'static str, DataValue)]>> Binder<'a, '_, T
         }
         if let Some(table) = table_name.or(bind_table_name) {
             let (source, position_offset) =
-                match Self::resolve_source_columns_in_scope(&self.context, &table) {
+                match Self::resolve_source_columns_in_scope(&self.context, table) {
                     Ok(source) => source,
                     Err(err) => {
                         if let Some(parent) = self.parent {
                             self.context.mark_outer_ref();
-                            Self::resolve_source_columns_in_scope(parent, &table)
-                                .map_err(|_| err)?
+                            Self::resolve_source_columns_in_scope(parent, table).map_err(|_| err)?
                         } else {
                             return Err(err);
                         }

src/binder/mod.rs

@@ -181,11 +181,11 @@ impl UsingColumn {
     }
 
     fn left_expr(&self) -> ScalarExpression {
-        ScalarExpression::column_expr(self.left_column.clone(), self.left_position)
+        ScalarExpression::column_expr(self.left_column, self.left_position)
     }
 
     fn right_expr(&self) -> ScalarExpression {
-        ScalarExpression::column_expr(self.right_column.clone(), self.right_position)
+        ScalarExpression::column_expr(self.right_column, self.right_position)
     }
 
     pub(crate) fn visible_expr(
@@ -529,9 +529,9 @@ impl<'a, T: Transaction> BinderContext<'a, T> {
         }
         let using_column = UsingColumn::new(
             join_type,
-            left_column.clone(),
+            *left_column,
             left_position,
-            right_column.clone(),
+            *right_column,
             right_position,
         );
         self.using.insert(name, using_column);

src/binder/parser.rs

@@ -1341,6 +1341,7 @@ where
     T: Transaction,
     A: AsRef<[(&'static str, DataValue)]>,
 {
+    #[allow(clippy::wrong_self_convention)]
     pub(crate) fn from_sql(
         self,
         from: Vec<TableWithJoins>,

src/binder/select.rs

@@ -433,6 +433,7 @@ where
     T: Transaction,
     A: AsRef<[(&'static str, DataValue)]>,
 {
+    #[allow(clippy::wrong_self_convention)]
     pub(crate) fn from_plan(
         self,
         plan: LogicalPlan,
@@ -504,26 +505,27 @@ where
                 |binder, orderby| bind_sort_field(binder, self.arena, orderby),
             )?;
         }
-
         if !self.binder.context.agg_calls.is_empty()
             || !self.binder.context.group_by_exprs.is_empty()
         {
             let agg_calls = std::mem::take(&mut self.binder.context.agg_calls);
             let group_by_exprs = std::mem::take(&mut self.binder.context.group_by_exprs);
+            let output_exprs = self
+                .select_list
+                .iter_mut()
+                .chain(having_orderby.0.iter_mut())
+                .chain(
+                    having_orderby
+                        .1
+                        .iter_mut()
+                        .flat_map(|fields| fields.iter_mut().map(|field| &mut field.expr)),
+                );
             self.binder.bind_aggregate_output_exprs_with_outputs(
                 &agg_calls,
                 &group_by_exprs,
-                self.select_list.iter_mut(),
+                output_exprs,
                 self.arena,
             )?;
-            if let Some(orderby) = having_orderby.1.as_mut() {
-                self.binder.bind_aggregate_output_exprs_with_outputs(
-                    &agg_calls,
-                    &group_by_exprs,
-                    orderby.iter_mut().map(|field| &mut field.expr),
-                    self.arena,
-                )?;
-            }
             self.plan = self
                 .binder
                 .bind_aggregate(self.plan, agg_calls, group_by_exprs)?;
@@ -1062,7 +1064,7 @@ impl<'a: 'b, 'b, T: Transaction, A: AsRef<[(&'static str, DataValue)]>> Binder<'
                 let source_column = arena.column(column);
                 (
                     source_column.clone(),
-                    source_column.id().unwrap_or(ColumnId::new()),
+                    source_column.id().unwrap_or_default(),
                     matches!(
                         &source_column.summary().relation,
                         ColumnRelation::Table { is_temp: true, .. }
@@ -1107,7 +1109,7 @@ impl<'a: 'b, 'b, T: Transaction, A: AsRef<[(&'static str, DataValue)]>> Binder<'
                 let mut source_column = column_catalog.clone();
                 source_column.set_ref_table(
                     source_name.clone(),
-                    column_catalog.id().unwrap_or(ColumnId::new()),
+                    column_catalog.id().unwrap_or_default(),
                     true,
                 );
                 source_column
@@ -1303,7 +1305,7 @@ impl<'a: 'b, 'b, T: Transaction, A: AsRef<[(&'static str, DataValue)]>> Binder<'
             else {
                 continue;
             };
-            if !fn_not_on_using(&column) {
+            if !fn_not_on_using(column) {
                 continue;
             }
             exprs.push(ScalarExpression::column_expr(
@@ -1318,7 +1320,7 @@ impl<'a: 'b, 'b, T: Transaction, A: AsRef<[(&'static str, DataValue)]>> Binder<'
         }
 
         for (position, column) in source.schema().iter().enumerate() {
-            if !fn_not_on_using(&column) {
+            if !fn_not_on_using(column) {
                 continue;
             }
             exprs.push(ScalarExpression::column_expr(
@@ -1523,6 +1525,7 @@ impl<'a: 'b, 'b, T: Transaction, A: AsRef<[(&'static str, DataValue)]>> Binder<'
         Vec::new()
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn prepare_mark_apply(
         predicate: &mut ScalarExpression,
         output_column: &ColumnRef,
@@ -2270,11 +2273,11 @@ mod tests {
             true,
             ColumnDesc::new(LogicalType::Integer, None, false, None).unwrap(),
         ));
-        let right_schema = vec![right_column.clone()];
+        let right_schema = vec![right_column];
         let mut expr = ScalarExpression::Binary {
             op: crate::expression::BinaryOperator::Eq,
             left_expr: Box::new(ScalarExpression::column_expr(left_column, 0)),
-            right_expr: Box::new(ScalarExpression::column_expr(right_column.clone(), 0)),
+            right_expr: Box::new(ScalarExpression::column_expr(right_column, 0)),
             evaluator: None,
             ty: LogicalType::Boolean,
         };

src/db.rs

@@ -508,7 +508,7 @@ impl<S: Storage> State<S> {
             BinderContext::new(
                 self.table_cache(),
                 self.view_cache(),
-                &*transaction,
+                transaction,
                 self.scala_functions(),
                 self.table_functions(),
             ),
@@ -1832,7 +1832,7 @@ pub(crate) mod test {
         let collect_ids = |sql: &str| -> Result<Vec<i32>, DatabaseError> {
             let mut iter = kite_sql.run(sql)?;
             let mut ids = Vec::new();
-            while let Some(row) = iter.next() {
+            for row in iter.by_ref() {
                 let row = row?;
                 ids.push(row.values[0].i32().unwrap());
             }
@@ -1964,7 +1964,7 @@ pub(crate) mod test {
         let collect_ids = |sql: &str| -> Result<Vec<i32>, DatabaseError> {
             let mut iter = kite_sql.run(sql)?;
             let mut ids = Vec::new();
-            while let Some(row) = iter.next() {
+            for row in iter.by_ref() {
                 let row = row?;
                 ids.push(row.values[0].i32().unwrap());
             }

src/execution/ddl/change_column.rs

@@ -102,8 +102,7 @@ impl<'a> ExecutorNode<'a> for ChangeColumn {
             };
             if let Some(index_name) = affected_index_name {
                 return Err(DatabaseError::UnsupportedStmt(format!(
-                    "cannot alter type of indexed column `{}`; drop index `{}` first",
-                    old_column_name, index_name
+                    "cannot alter type of indexed column `{old_column_name}`; drop index `{index_name}` first"
                 )));
             }
         }

src/execution/dml/analyze.rs

@@ -117,7 +117,7 @@ impl<'a> ExecutorNode<'a> for Analyze {
         while runtime.next_tuple(input, plan_arena)? {
             let tuple = runtime.result_tuple();
             for State { exprs, builder, .. } in builders.iter_mut() {
-                let values = Projection::projection(tuple, &exprs)?;
+                let values = Projection::projection(tuple, exprs)?;
 
                 if values.len() == 1 {
                     builder.append(&values[0])?;

src/execution/dml/copy_from_file.rs

@@ -176,7 +176,7 @@ mod tests {
             schema_ref: columns,
         };
 
-        let mut transaction = db.storage.transaction()?;
+        let transaction = db.storage.transaction()?;
         let mut executor = crate::execution::execute_input_mut::<_, CopyFromFile>(
             op,
             (
@@ -185,7 +185,7 @@ mod tests {
                 db.state.meta_cache(),
             ),
             plan_arena,
-            &mut transaction,
+            &transaction,
         );
 
         let result = executor.next().expect("copy from file should yield once")?;

src/execution/dml/copy_to_file.rs

@@ -164,7 +164,7 @@ mod tests {
         db.run("insert into t1 values (3, 2.1, 'Kite')")?.done()?;
 
         let plan_arena = crate::planner::PlanArena::new(db.state.table_arena());
-        let mut transaction = db.storage.transaction()?;
+        let transaction = db.storage.transaction()?;
         let table = transaction
             .table(db.state.table_cache(), "t1".to_string().into())?
             .unwrap();
@@ -179,7 +179,7 @@ mod tests {
                 db.state.meta_cache(),
             ),
             plan_arena,
-            &mut transaction,
+            &transaction,
         );
 
         let tuple = executor.next().expect("executor should yield once")?;