Merge 'Make translate_* functions accept ProgramBuilder' from Levy A.

Simplifies function signatures and allows attaching more context to
ProgramStatus on `translate::translate`, useful for value binding –
#607.

Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>

Closes #670

core/translate/delete.rs

@@ -3,26 +3,23 @@ use crate::translate::emitter::emit_program;
 use crate::translate::optimizer::optimize_plan;
 use crate::translate::plan::{DeletePlan, Plan, SourceOperator};
 use crate::translate::planner::{parse_limit, parse_where};
-use crate::{schema::Schema, storage::sqlite3_ondisk::DatabaseHeader, vdbe::Program};
-use crate::{Connection, Result, SymbolTable};
+use crate::vdbe::builder::ProgramBuilder;
+use crate::{schema::Schema, Result, SymbolTable};
 use sqlite3_parser::ast::{Expr, Limit, QualifiedName};
-use std::rc::Weak;
-use std::{cell::RefCell, rc::Rc};
 
 use super::plan::{TableReference, TableReferenceType};
 
 pub fn translate_delete(
+    program: &mut ProgramBuilder,
     schema: &Schema,
     tbl_name: &QualifiedName,
     where_clause: Option<Expr>,
     limit: Option<Box<Limit>>,
-    database_header: Rc<RefCell<DatabaseHeader>>,
-    connection: Weak<Connection>,
     syms: &SymbolTable,
-) -> Result<Program> {
+) -> Result<()> {
     let mut delete_plan = prepare_delete_plan(schema, tbl_name, where_clause, limit)?;
     optimize_plan(&mut delete_plan)?;
-    emit_program(database_header, delete_plan, connection, syms)
+    emit_program(program, delete_plan, syms)
 }
 
 pub fn prepare_delete_plan(

core/translate/emitter.rs

@@ -1,19 +1,16 @@
 // This module contains code for emitting bytecode instructions for SQL query execution.
 // It handles translating high-level SQL operations into low-level bytecode that can be executed by the virtual machine.
 
-use std::cell::RefCell;
 use std::collections::HashMap;
-use std::rc::{Rc, Weak};
 
 use sqlite3_parser::ast::{self};
 
 use crate::function::Func;
-use crate::storage::sqlite3_ondisk::DatabaseHeader;
 use crate::translate::plan::{DeletePlan, Plan, Search};
 use crate::util::exprs_are_equivalent;
 use crate::vdbe::builder::ProgramBuilder;
-use crate::vdbe::{insn::Insn, BranchOffset, Program};
-use crate::{Connection, Result, SymbolTable};
+use crate::vdbe::{insn::Insn, BranchOffset};
+use crate::{Result, SymbolTable};
 
 use super::aggregation::emit_ungrouped_aggregation;
 use super::group_by::{emit_group_by, init_group_by, GroupByMetadata};
@@ -99,9 +96,9 @@ pub enum OperationMode {
 
 /// Initialize the program with basic setup and return initial metadata and labels
 fn prologue<'a>(
+    program: &mut ProgramBuilder,
     syms: &'a SymbolTable,
-) -> Result<(ProgramBuilder, TranslateCtx<'a>, BranchOffset, BranchOffset)> {
-    let mut program = ProgramBuilder::new();
+) -> Result<(TranslateCtx<'a>, BranchOffset, BranchOffset)> {
     let init_label = program.allocate_label();
 
     program.emit_insn(Insn::Init {
@@ -124,7 +121,7 @@ fn prologue<'a>(
         resolver: Resolver::new(syms),
     };
 
-    Ok((program, t_ctx, init_label, start_offset))
+    Ok((t_ctx, init_label, start_offset))
 }
 
 /// Clean up and finalize the program, resolving any remaining labels
@@ -153,41 +150,34 @@ fn epilogue(
 
 /// Main entry point for emitting bytecode for a SQL query
 /// Takes a query plan and generates the corresponding bytecode program
-pub fn emit_program(
-    database_header: Rc<RefCell<DatabaseHeader>>,
-    plan: Plan,
-    connection: Weak<Connection>,
-    syms: &SymbolTable,
-) -> Result<Program> {
+pub fn emit_program(program: &mut ProgramBuilder, plan: Plan, syms: &SymbolTable) -> Result<()> {
     match plan {
-        Plan::Select(plan) => emit_program_for_select(database_header, plan, connection, syms),
-        Plan::Delete(plan) => emit_program_for_delete(database_header, plan, connection, syms),
+        Plan::Select(plan) => emit_program_for_select(program, plan, syms),
+        Plan::Delete(plan) => emit_program_for_delete(program, plan, syms),
     }
 }
 
 fn emit_program_for_select(
-    database_header: Rc<RefCell<DatabaseHeader>>,
+    program: &mut ProgramBuilder,
     mut plan: SelectPlan,
-    connection: Weak<Connection>,
     syms: &SymbolTable,
-) -> Result<Program> {
-    let (mut program, mut t_ctx, init_label, start_offset) = prologue(syms)?;
+) -> Result<()> {
+    let (mut t_ctx, init_label, start_offset) = prologue(program, syms)?;
 
     // Trivial exit on LIMIT 0
     if let Some(limit) = plan.limit {
         if limit == 0 {
-            epilogue(&mut program, init_label, start_offset)?;
-            return Ok(program.build(database_header, connection));
+            epilogue(program, init_label, start_offset)?;
         }
     }
 
     // Emit main parts of query
-    emit_query(&mut program, &mut plan, &mut t_ctx)?;
+    emit_query(program, &mut plan, &mut t_ctx)?;
 
     // Finalize program
-    epilogue(&mut program, init_label, start_offset)?;
+    epilogue(program, init_label, start_offset)?;
 
-    Ok(program.build(database_header, connection))
+    Ok(())
 }
 
 pub fn emit_query<'a>(
@@ -263,12 +253,11 @@ pub fn emit_query<'a>(
 }
 
 fn emit_program_for_delete(
-    database_header: Rc<RefCell<DatabaseHeader>>,
+    program: &mut ProgramBuilder,
     mut plan: DeletePlan,
-    connection: Weak<Connection>,
     syms: &SymbolTable,
-) -> Result<Program> {
-    let (mut program, mut t_ctx, init_label, start_offset) = prologue(syms)?;
+) -> Result<()> {
+    let (mut t_ctx, init_label, start_offset) = prologue(program, syms)?;
 
     // No rows will be read from source table loops if there is a constant false condition eg. WHERE 0
     let after_main_loop_label = program.allocate_label();
@@ -279,32 +268,27 @@ fn emit_program_for_delete(
     }
 
     // Initialize cursors and other resources needed for query execution
-    init_loop(
-        &mut program,
-        &mut t_ctx,
-        &plan.source,
-        &OperationMode::DELETE,
-    )?;
+    init_loop(program, &mut t_ctx, &plan.source, &OperationMode::DELETE)?;
 
     // Set up main query execution loop
     open_loop(
-        &mut program,
+        program,
         &mut t_ctx,
         &mut plan.source,
         &plan.referenced_tables,
     )?;
 
-    emit_delete_insns(&mut program, &mut t_ctx, &plan.source, &plan.limit)?;
+    emit_delete_insns(program, &mut t_ctx, &plan.source, &plan.limit)?;
 
     // Clean up and close the main execution loop
-    close_loop(&mut program, &mut t_ctx, &plan.source)?;
+    close_loop(program, &mut t_ctx, &plan.source)?;
 
     program.resolve_label(after_main_loop_label, program.offset());
 
     // Finalize program
-    epilogue(&mut program, init_label, start_offset)?;
+    epilogue(program, init_label, start_offset)?;
 
-    Ok(program.build(database_header, connection))
+    Ok(())
 }
 
 fn emit_delete_insns<'a>(

core/translate/insert.rs

@@ -1,5 +1,4 @@
-use std::rc::Weak;
-use std::{cell::RefCell, ops::Deref, rc::Rc};
+use std::ops::Deref;
 
 use sqlite3_parser::ast::{
     DistinctNames, Expr, InsertBody, QualifiedName, ResolveType, ResultColumn, With,
@@ -9,23 +8,22 @@ use crate::error::SQLITE_CONSTRAINT_PRIMARYKEY;
 use crate::schema::BTreeTable;
 use crate::util::normalize_ident;
 use crate::vdbe::BranchOffset;
+use crate::Result;
 use crate::{
     schema::{Column, Schema},
-    storage::sqlite3_ondisk::DatabaseHeader,
     translate::expr::translate_expr,
     vdbe::{
         builder::{CursorType, ProgramBuilder},
         insn::Insn,
-        Program,
     },
     SymbolTable,
 };
-use crate::{Connection, Result};
 
 use super::emitter::Resolver;
 
 #[allow(clippy::too_many_arguments)]
 pub fn translate_insert(
+    program: &mut ProgramBuilder,
     schema: &Schema,
     with: &Option<With>,
     on_conflict: &Option<ResolveType>,
@@ -33,17 +31,14 @@ pub fn translate_insert(
     columns: &Option<DistinctNames>,
     body: &InsertBody,
     _returning: &Option<Vec<ResultColumn>>,
-    database_header: Rc<RefCell<DatabaseHeader>>,
-    connection: Weak<Connection>,
     syms: &SymbolTable,
-) -> Result<Program> {
+) -> Result<()> {
     if with.is_some() {
         crate::bail_parse_error!("WITH clause is not supported");
     }
     if on_conflict.is_some() {
         crate::bail_parse_error!("ON CONFLICT clause is not supported");
     }
-    let mut program = ProgramBuilder::new();
     let resolver = Resolver::new(syms);
     let init_label = program.allocate_label();
     program.emit_insn(Insn::Init {
@@ -118,7 +113,7 @@ pub fn translate_insert(
 
         for value in values {
             populate_column_registers(
-                &mut program,
+                program,
                 value,
                 &column_mappings,
                 column_registers_start,
@@ -157,7 +152,7 @@ pub fn translate_insert(
         program.emit_insn(Insn::OpenWriteAwait {});
 
         populate_column_registers(
-            &mut program,
+            program,
             &values[0],
             &column_mappings,
             column_registers_start,
@@ -262,7 +257,8 @@ pub fn translate_insert(
     program.emit_insn(Insn::Goto {
         target_pc: start_offset,
     });
-    Ok(program.build(database_header, connection))
+
+    Ok(())
 }
 
 #[derive(Debug)]

core/translate/mod.rs

@@ -48,6 +48,7 @@ pub fn translate(
     connection: Weak<Connection>,
     syms: &SymbolTable,
 ) -> Result<Program> {
+    let mut program = ProgramBuilder::new();
     match stmt {
         ast::Stmt::AlterTable(_, _) => bail_parse_error!("ALTER TABLE not supported yet"),
         ast::Stmt::Analyze(_) => bail_parse_error!("ANALYZE not supported yet"),
@@ -64,14 +65,8 @@ pub fn translate(
             if temporary {
                 bail_parse_error!("TEMPORARY table not supported yet");
             }
-            translate_create_table(
-                tbl_name,
-                body,
-                if_not_exists,
-                database_header,
-                connection,
-                schema,
-            )
+
+            translate_create_table(&mut program, tbl_name, body, if_not_exists, schema)?;
         }
         ast::Stmt::CreateTrigger { .. } => bail_parse_error!("CREATE TRIGGER not supported yet"),
         ast::Stmt::CreateView { .. } => bail_parse_error!("CREATE VIEW not supported yet"),
@@ -83,29 +78,23 @@ pub fn translate(
             where_clause,
             limit,
             ..
-        } => translate_delete(
-            schema,
-            &tbl_name,
-            where_clause,
-            limit,
-            database_header,
-            connection,
-            syms,
-        ),
+        } => {
+            translate_delete(&mut program, schema, &tbl_name, where_clause, limit, syms)?;
+        }
         ast::Stmt::Detach(_) => bail_parse_error!("DETACH not supported yet"),
         ast::Stmt::DropIndex { .. } => bail_parse_error!("DROP INDEX not supported yet"),
         ast::Stmt::DropTable { .. } => bail_parse_error!("DROP TABLE not supported yet"),
         ast::Stmt::DropTrigger { .. } => bail_parse_error!("DROP TRIGGER not supported yet"),
         ast::Stmt::DropView { .. } => bail_parse_error!("DROP VIEW not supported yet"),
         ast::Stmt::Pragma(name, body) => {
-            translate_pragma(&name, body, database_header, pager, connection)
+            translate_pragma(&mut program, &name, body, database_header.clone(), pager)?;
         }
         ast::Stmt::Reindex { .. } => bail_parse_error!("REINDEX not supported yet"),
         ast::Stmt::Release(_) => bail_parse_error!("RELEASE not supported yet"),
         ast::Stmt::Rollback { .. } => bail_parse_error!("ROLLBACK not supported yet"),
         ast::Stmt::Savepoint(_) => bail_parse_error!("SAVEPOINT not supported yet"),
         ast::Stmt::Select(select) => {
-            translate_select(schema, *select, database_header, connection, syms)
+            translate_select(&mut program, schema, *select, syms)?;
         }
         ast::Stmt::Update { .. } => bail_parse_error!("UPDATE not supported yet"),
         ast::Stmt::Vacuum(_, _) => bail_parse_error!("VACUUM not supported yet"),
@@ -116,19 +105,21 @@ pub fn translate(
             columns,
             body,
             returning,
-        } => translate_insert(
-            schema,
-            &with,
-            &or_conflict,
-            &tbl_name,
-            &columns,
-            &body,
-            &returning,
-            database_header,
-            connection,
-            syms,
-        ),
+        } => {
+            translate_insert(
+                &mut program,
+                schema,
+                &with,
+                &or_conflict,
+                &tbl_name,
+                &columns,
+                &body,
+                &returning,
+                syms,
+            )?;
+        }
     }
+    Ok(program.build(database_header, connection))
 }
 
 /* Example:
@@ -378,14 +369,12 @@ fn check_automatic_pk_index_required(
 }
 
 fn translate_create_table(
+    program: &mut ProgramBuilder,
     tbl_name: ast::QualifiedName,
     body: ast::CreateTableBody,
     if_not_exists: bool,
-    database_header: Rc<RefCell<DatabaseHeader>>,
-    connection: Weak<Connection>,
     schema: &Schema,
-) -> Result<Program> {
-    let mut program = ProgramBuilder::new();
+) -> Result<()> {
     if schema.get_table(tbl_name.name.0.as_str()).is_some() {
         if if_not_exists {
             let init_label = program.allocate_label();
@@ -403,7 +392,8 @@ fn translate_create_table(
             program.emit_insn(Insn::Goto {
                 target_pc: start_offset,
             });
-            return Ok(program.build(database_header, connection));
+
+            return Ok(());
         }
         bail_parse_error!("Table {} already exists", tbl_name);
     }
@@ -453,7 +443,7 @@ fn translate_create_table(
     // https://github.com/sqlite/sqlite/blob/95f6df5b8d55e67d1e34d2bff217305a2f21b1fb/src/build.c#L2856-L2871
     // https://github.com/sqlite/sqlite/blob/95f6df5b8d55e67d1e34d2bff217305a2f21b1fb/src/build.c#L1334C5-L1336C65
 
-    let index_root_reg = check_automatic_pk_index_required(&body, &mut program, &tbl_name.name.0)?;
+    let index_root_reg = check_automatic_pk_index_required(&body, program, &tbl_name.name.0)?;
     if let Some(index_root_reg) = index_root_reg {
         program.emit_insn(Insn::CreateBtree {
             db: 0,
@@ -476,7 +466,7 @@ fn translate_create_table(
 
     // Add the table entry to sqlite_schema
     emit_schema_entry(
-        &mut program,
+        program,
         sqlite_schema_cursor_id,
         SchemaEntryType::Table,
         &tbl_name.name.0,
@@ -492,7 +482,7 @@ fn translate_create_table(
             PRIMARY_KEY_AUTOMATIC_INDEX_NAME_PREFIX, tbl_name.name.0
         );
         emit_schema_entry(
-            &mut program,
+            program,
             sqlite_schema_cursor_id,
             SchemaEntryType::Index,
             &index_name,
@@ -523,7 +513,8 @@ fn translate_create_table(
     program.emit_insn(Insn::Goto {
         target_pc: start_offset,
     });
-    Ok(program.build(database_header, connection))
+
+    Ok(())
 }
 
 enum PrimaryKeyDefinitionType<'a> {
@@ -532,13 +523,12 @@ enum PrimaryKeyDefinitionType<'a> {
 }
 
 fn translate_pragma(
+    program: &mut ProgramBuilder,
     name: &ast::QualifiedName,
     body: Option<ast::PragmaBody>,
     database_header: Rc<RefCell<DatabaseHeader>>,
     pager: Rc<Pager>,
-    connection: Weak<Connection>,
-) -> Result<Program> {
-    let mut program = ProgramBuilder::new();
+) -> Result<()> {
     let init_label = program.allocate_label();
     program.emit_insn(Insn::Init {
         target_pc: init_label,
@@ -548,17 +538,11 @@ fn translate_pragma(
     match body {
         None => {
             let pragma_name = &name.name.0;
-            query_pragma(pragma_name, database_header.clone(), &mut program)?;
+            query_pragma(pragma_name, database_header.clone(), program)?;
         }
         Some(ast::PragmaBody::Equals(value)) => {
             write = true;
-            update_pragma(
-                &name.name.0,
-                value,
-                database_header.clone(),
-                pager,
-                &mut program,
-            )?;
+            update_pragma(&name.name.0, value, database_header.clone(), pager, program)?;
         }
         Some(ast::PragmaBody::Call(_)) => {
             todo!()
@@ -574,7 +558,8 @@ fn translate_pragma(
     program.emit_insn(Insn::Goto {
         target_pc: start_offset,
     });
-    Ok(program.build(database_header, connection))
+
+    Ok(())
 }
 
 fn update_pragma(

core/translate/select.rs

@@ -1,11 +1,7 @@
-use std::rc::Weak;
-use std::{cell::RefCell, rc::Rc};
-
 use super::emitter::emit_program;
 use super::expr::get_name;
 use super::plan::SelectQueryType;
 use crate::function::Func;
-use crate::storage::sqlite3_ondisk::DatabaseHeader;
 use crate::translate::optimizer::optimize_plan;
 use crate::translate::plan::{Aggregate, Direction, GroupBy, Plan, ResultSetColumn, SelectPlan};
 use crate::translate::planner::{
@@ -13,21 +9,20 @@ use crate::translate::planner::{
     parse_where, resolve_aggregates, OperatorIdCounter,
 };
 use crate::util::normalize_ident;
-use crate::{schema::Schema, vdbe::Program, Result};
-use crate::{Connection, SymbolTable};
+use crate::SymbolTable;
+use crate::{schema::Schema, vdbe::builder::ProgramBuilder, Result};
 use sqlite3_parser::ast;
 use sqlite3_parser::ast::ResultColumn;
 
 pub fn translate_select(
+    program: &mut ProgramBuilder,
     schema: &Schema,
     select: ast::Select,
-    database_header: Rc<RefCell<DatabaseHeader>>,
-    connection: Weak<Connection>,
     syms: &SymbolTable,
-) -> Result<Program> {
+) -> Result<()> {
     let mut select_plan = prepare_select_plan(schema, select)?;
     optimize_plan(&mut select_plan)?;
-    emit_program(database_header, select_plan, connection, syms)
+    emit_program(program, select_plan, syms)
 }
 
 pub fn prepare_select_plan(schema: &Schema, select: ast::Select) -> Result<Plan> {