From 53bf5d5ef5a5bfb9ef84212fec0fa54dd11ceb0b Mon Sep 17 00:00:00 2001 From: pedrocarlo Date: Wed, 21 May 2025 16:41:10 -0300 Subject: [PATCH] adjust translate functions to take a program instead of `Option` + remove any Init emission in traslate functions + use epilogue in all places necessary --- core/translate/delete.rs | 9 +--- core/translate/index.rs | 47 +++----------------- core/translate/insert.rs | 9 +--- core/translate/mod.rs | 10 ++--- core/translate/pragma.rs | 34 ++++---------- core/translate/schema.rs | 83 +++++------------------------------ core/translate/select.rs | 9 +--- core/translate/transaction.rs | 22 ++++------ core/translate/update.rs | 9 +--- core/vdbe/builder.rs | 23 ++-------- 10 files changed, 53 insertions(+), 202 deletions(-) diff --git a/core/translate/delete.rs b/core/translate/delete.rs index 5f525b359..c6d7a7869 100644 --- a/core/translate/delete.rs +++ b/core/translate/delete.rs @@ -16,7 +16,7 @@ pub fn translate_delete( where_clause: Option>, limit: Option>, syms: &SymbolTable, - program: Option, + mut program: ProgramBuilder, ) -> Result { let mut delete_plan = prepare_delete_plan(schema, tbl_name, where_clause, limit)?; optimize_plan(&mut delete_plan, schema)?; @@ -29,12 +29,7 @@ pub fn translate_delete( approx_num_insns: estimate_num_instructions(delete), approx_num_labels: 0, }; - let mut program = if let Some(mut program) = program { - program.extend(&opts); - program - } else { - ProgramBuilder::new(opts) - }; + program.extend(&opts); emit_program(&mut program, delete_plan, syms)?; Ok(program) } diff --git a/core/translate/index.rs b/core/translate/index.rs index cbdbc0410..7b1e898de 100644 --- a/core/translate/index.rs +++ b/core/translate/index.rs @@ -21,7 +21,7 @@ pub fn translate_create_index( tbl_name: &str, columns: &[SortedColumn], schema: &Schema, - program: Option, + mut program: ProgramBuilder, ) -> crate::Result { let idx_name = normalize_ident(idx_name); let tbl_name = normalize_ident(tbl_name); @@ -31,12 +31,7 @@ pub fn translate_create_index( approx_num_insns: 40, approx_num_labels: 5, }; - let mut program = if let Some(mut program) = program { - program.extend(&opts); - program - } else { - ProgramBuilder::new(opts) - }; + program.extend(&opts); // Check if the index is being created on a valid btree table and // the name is globally unique in the schema. @@ -51,10 +46,6 @@ pub fn translate_create_index( }; let columns = resolve_sorted_columns(&tbl, columns)?; - // Prologue: - let init_label = program.emit_init(); - let start_offset = program.offset(); - let idx = Arc::new(Index { name: idx_name.clone(), table_name: tbl.name.clone(), @@ -249,11 +240,7 @@ pub fn translate_create_index( }); // Epilogue: - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_transaction(true); - program.emit_constant_insns(); - program.emit_goto(start_offset); + program.epilogue(super::emitter::TransactionMode::Write); Ok(program) } @@ -318,7 +305,7 @@ pub fn translate_drop_index( idx_name: &str, if_exists: bool, schema: &Schema, - program: Option, + mut program: ProgramBuilder, ) -> crate::Result { let idx_name = normalize_ident(idx_name); let opts = crate::vdbe::builder::ProgramBuilderOpts { @@ -327,12 +314,7 @@ pub fn translate_drop_index( approx_num_insns: 40, approx_num_labels: 5, }; - let mut program = if let Some(mut program) = program { - program.extend(&opts); - program - } else { - ProgramBuilder::new(opts) - }; + program.extend(&opts); // Find the index in Schema let mut maybe_index = None; @@ -352,13 +334,7 @@ pub fn translate_drop_index( // then return normaly, otherwise show an error. if maybe_index.is_none() { if if_exists { - let init_label = program.emit_init(); - let start_offset = program.offset(); - program.emit_halt(); - program.resolve_label(init_label, program.offset()); - program.emit_transaction(true); - program.emit_constant_insns(); - program.emit_goto(start_offset); + program.epilogue(super::emitter::TransactionMode::Write); return Ok(program); } else { return Err(crate::error::LimboError::InvalidArgument(format!( @@ -368,11 +344,6 @@ pub fn translate_drop_index( } } - // 1. Init - // 2. Goto - let init_label = program.emit_init(); - let start_offset = program.offset(); - // According to sqlite should emit Null instruction // but why? let null_reg = program.alloc_register(); @@ -488,11 +459,7 @@ pub fn translate_drop_index( } // Epilogue: - program.emit_halt(); - program.resolve_label(init_label, program.offset()); - program.emit_transaction(true); - program.emit_constant_insns(); - program.emit_goto(start_offset); + program.epilogue(super::emitter::TransactionMode::Write); Ok(program) } diff --git a/core/translate/insert.rs b/core/translate/insert.rs index db271d65a..41af2fa45 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -34,7 +34,7 @@ pub fn translate_insert( body: &mut InsertBody, _returning: &Option>, syms: &SymbolTable, - program: Option, + mut program: ProgramBuilder, ) -> Result { let opts = ProgramBuilderOpts { query_mode, @@ -42,12 +42,7 @@ pub fn translate_insert( approx_num_insns: 30, approx_num_labels: 5, }; - let mut program = if let Some(mut program) = program { - program.extend(&opts); - program - } else { - ProgramBuilder::new(opts) - }; + program.extend(&opts); if with.is_some() { crate::bail_parse_error!("WITH clause is not supported"); } diff --git a/core/translate/mod.rs b/core/translate/mod.rs index 4cb6cfd08..ff1e14ce8 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -84,9 +84,9 @@ pub fn translate( body.map(|b| *b), database_header.clone(), pager, - Some(program), + program, )?, - stmt => translate_inner(schema, stmt, syms, query_mode, Some(program))?, + stmt => translate_inner(schema, stmt, syms, query_mode, program)?, }; // TODO: bring epilogue here when I can sort out what instructions correspond to a Write or a Read transaction @@ -102,7 +102,7 @@ pub fn translate_inner( stmt: ast::Stmt, syms: &SymbolTable, query_mode: QueryMode, - program: Option, + program: ProgramBuilder, ) -> Result { let program = match stmt { ast::Stmt::AlterTable(a) => { @@ -159,8 +159,8 @@ pub fn translate_inner( } ast::Stmt::Analyze(_) => bail_parse_error!("ANALYZE not supported yet"), ast::Stmt::Attach { .. } => bail_parse_error!("ATTACH not supported yet"), - ast::Stmt::Begin(tx_type, tx_name) => translate_tx_begin(tx_type, tx_name)?, - ast::Stmt::Commit(tx_name) => translate_tx_commit(tx_name)?, + ast::Stmt::Begin(tx_type, tx_name) => translate_tx_begin(tx_type, tx_name, program)?, + ast::Stmt::Commit(tx_name) => translate_tx_commit(tx_name, program)?, ast::Stmt::CreateIndex { unique, if_not_exists, diff --git a/core/translate/pragma.rs b/core/translate/pragma.rs index c727c1e7f..cd9cebd9b 100644 --- a/core/translate/pragma.rs +++ b/core/translate/pragma.rs @@ -13,25 +13,17 @@ use crate::storage::wal::CheckpointMode; use crate::util::normalize_ident; use crate::vdbe::builder::{ProgramBuilder, ProgramBuilderOpts, QueryMode}; use crate::vdbe::insn::{Cookie, Insn}; -use crate::vdbe::BranchOffset; use crate::{bail_parse_error, Pager}; use std::str::FromStr; use strum::IntoEnumIterator; -fn list_pragmas( - program: &mut ProgramBuilder, - init_label: BranchOffset, - start_offset: BranchOffset, -) { +fn list_pragmas(program: &mut ProgramBuilder) { for x in PragmaName::iter() { let register = program.emit_string8_new_reg(x.to_string()); program.emit_result_row(register, 1); } - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_constant_insns(); - program.emit_goto(start_offset); + program.epilogue(crate::translate::emitter::TransactionMode::None); } pub fn translate_pragma( @@ -41,7 +33,7 @@ pub fn translate_pragma( body: Option, database_header: Arc>, pager: Rc, - program: Option, + mut program: ProgramBuilder, ) -> crate::Result { let opts = ProgramBuilderOpts { query_mode, @@ -49,18 +41,11 @@ pub fn translate_pragma( approx_num_insns: 20, approx_num_labels: 0, }; - let mut program = if let Some(mut program) = program { - program.extend(&opts); - program - } else { - ProgramBuilder::new(opts) - }; - let init_label = program.emit_init(); - let start_offset = program.offset(); + program.extend(&opts); let mut write = false; if name.name.0.to_lowercase() == "pragma_list" { - list_pragmas(&mut program, init_label, start_offset); + list_pragmas(&mut program); return Ok(program); } @@ -110,11 +95,10 @@ pub fn translate_pragma( } }, }; - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_transaction(write); - program.emit_constant_insns(); - program.emit_goto(start_offset); + program.epilogue(match write { + false => super::emitter::TransactionMode::Read, + true => super::emitter::TransactionMode::Write, + }); Ok(program) } diff --git a/core/translate/schema.rs b/core/translate/schema.rs index bf0867214..5e2c2b2b4 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -34,7 +34,7 @@ pub fn translate_create_table( body: ast::CreateTableBody, if_not_exists: bool, schema: &Schema, - program: Option, + mut program: ProgramBuilder, ) -> Result { if temporary { bail_parse_error!("TEMPORARY table not supported yet"); @@ -45,21 +45,10 @@ pub fn translate_create_table( approx_num_insns: 30, approx_num_labels: 1, }; - let mut program = if let Some(mut program) = program { - program.extend(&opts); - program - } else { - ProgramBuilder::new(opts) - }; + program.extend(&opts); if schema.get_table(tbl_name.name.0.as_str()).is_some() { if if_not_exists { - let init_label = program.emit_init(); - let start_offset = program.offset(); - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_transaction(true); - program.emit_constant_insns(); - program.emit_goto(start_offset); + program.epilogue(crate::translate::emitter::TransactionMode::Write); return Ok(program); } @@ -69,8 +58,6 @@ pub fn translate_create_table( let sql = create_table_body_to_str(&tbl_name, &body); let parse_schema_label = program.allocate_label(); - let init_label = program.emit_init(); - let start_offset = program.offset(); // TODO: ReadCookie // TODO: If // TODO: SetCookie @@ -173,11 +160,7 @@ pub fn translate_create_table( }); // TODO: SqlExec - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_transaction(true); - program.emit_constant_insns(); - program.emit_goto(start_offset); + program.epilogue(super::emitter::TransactionMode::Write); Ok(program) } @@ -540,7 +523,7 @@ pub fn translate_create_virtual_table( schema: &Schema, query_mode: QueryMode, syms: &SymbolTable, - program: Option, + mut program: ProgramBuilder, ) -> Result { let ast::CreateVirtualTable { if_not_exists, @@ -560,19 +543,7 @@ pub fn translate_create_virtual_table( }; if schema.get_table(&table_name).is_some() { if *if_not_exists { - let mut program = ProgramBuilder::new(ProgramBuilderOpts { - query_mode, - num_cursors: 1, - approx_num_insns: 5, - approx_num_labels: 1, - }); - let init_label = program.emit_init(); - let start_offset = program.offset(); - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_transaction(true); - program.emit_constant_insns(); - program.emit_goto(start_offset); + program.epilogue(crate::translate::emitter::TransactionMode::Write); return Ok(program); } bail_parse_error!("Table {} already exists", tbl_name); @@ -584,14 +555,7 @@ pub fn translate_create_virtual_table( approx_num_insns: 40, approx_num_labels: 2, }; - let mut program = if let Some(mut program) = program { - program.extend(&opts); - program - } else { - ProgramBuilder::new(opts) - }; - let init_label = program.emit_init(); - let start_offset = program.offset(); + program.extend(&opts); let module_name_reg = program.emit_string8_new_reg(module_name_str.clone()); let table_name_reg = program.emit_string8_new_reg(table_name.clone()); let args_reg = if !args_vec.is_empty() { @@ -648,11 +612,7 @@ pub fn translate_create_virtual_table( where_clause: Some(parse_schema_where_clause), }); - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_transaction(true); - program.emit_constant_insns(); - program.emit_goto(start_offset); + program.epilogue(super::emitter::TransactionMode::Write); Ok(program) } @@ -662,7 +622,7 @@ pub fn translate_drop_table( tbl_name: ast::QualifiedName, if_exists: bool, schema: &Schema, - program: Option, + mut program: ProgramBuilder, ) -> Result { let opts = ProgramBuilderOpts { query_mode, @@ -670,22 +630,11 @@ pub fn translate_drop_table( approx_num_insns: 30, approx_num_labels: 1, }; - let mut program = if let Some(mut program) = program { - program.extend(&opts); - program - } else { - ProgramBuilder::new(opts) - }; + program.extend(&opts); let table = schema.get_table(tbl_name.name.0.as_str()); if table.is_none() { if if_exists { - let init_label = program.emit_init(); - let start_offset = program.offset(); - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_transaction(true); - program.emit_constant_insns(); - program.emit_goto(start_offset); + program.epilogue(crate::translate::emitter::TransactionMode::Write); return Ok(program); } @@ -694,9 +643,6 @@ pub fn translate_drop_table( let table = table.unwrap(); // safe since we just checked for None - let init_label = program.emit_init(); - let start_offset = program.offset(); - let null_reg = program.alloc_register(); // r1 program.emit_null(null_reg, None); let tbl_name_reg = program.alloc_register(); // r2 @@ -836,12 +782,7 @@ pub fn translate_drop_table( }); // end of the program - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_transaction(true); - program.emit_constant_insns(); - - program.emit_goto(start_offset); + program.epilogue(super::emitter::TransactionMode::Write); Ok(program) } diff --git a/core/translate/select.rs b/core/translate/select.rs index da808ecdf..46b1152a8 100644 --- a/core/translate/select.rs +++ b/core/translate/select.rs @@ -24,7 +24,7 @@ pub fn translate_select( schema: &Schema, select: ast::Select, syms: &SymbolTable, - program: Option, + mut program: ProgramBuilder, ) -> Result { let mut select_plan = prepare_select_plan(schema, select, syms, None)?; optimize_plan(&mut select_plan, schema)?; @@ -38,12 +38,7 @@ pub fn translate_select( approx_num_insns: estimate_num_instructions(select), approx_num_labels: estimate_num_labels(select), }; - let mut program = if let Some(mut program) = program { - program.extend(&opts); - program - } else { - ProgramBuilder::new(opts) - }; + program.extend(&opts); emit_program(&mut program, select_plan, syms)?; Ok(program) } diff --git a/core/translate/transaction.rs b/core/translate/transaction.rs index 11c0a8a10..0997be9b8 100644 --- a/core/translate/transaction.rs +++ b/core/translate/transaction.rs @@ -6,15 +6,14 @@ use limbo_sqlite3_parser::ast::{Name, TransactionType}; pub fn translate_tx_begin( tx_type: Option, _tx_name: Option, + mut program: ProgramBuilder, ) -> Result { - let mut program = ProgramBuilder::new(ProgramBuilderOpts { + program.extend(&ProgramBuilderOpts { query_mode: QueryMode::Normal, num_cursors: 0, approx_num_insns: 0, approx_num_labels: 0, }); - let init_label = program.emit_init(); - let start_offset = program.offset(); let tx_type = tx_type.unwrap_or(TransactionType::Deferred); match tx_type { TransactionType::Deferred => { @@ -32,27 +31,24 @@ pub fn translate_tx_begin( }); } } - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_goto(start_offset); + program.epilogue(super::emitter::TransactionMode::None); Ok(program) } -pub fn translate_tx_commit(_tx_name: Option) -> Result { - let mut program = ProgramBuilder::new(ProgramBuilderOpts { +pub fn translate_tx_commit( + _tx_name: Option, + mut program: ProgramBuilder, +) -> Result { + program.extend(&ProgramBuilderOpts { query_mode: QueryMode::Normal, num_cursors: 0, approx_num_insns: 0, approx_num_labels: 0, }); - let init_label = program.emit_init(); - let start_offset = program.offset(); program.emit_insn(Insn::AutoCommit { auto_commit: true, rollback: false, }); - program.emit_halt(); - program.preassign_label_to_next_insn(init_label); - program.emit_goto(start_offset); + program.epilogue(super::emitter::TransactionMode::None); Ok(program) } diff --git a/core/translate/update.rs b/core/translate/update.rs index aafa007ce..6c7b4c2f5 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -52,7 +52,7 @@ pub fn translate_update( body: &mut Update, syms: &SymbolTable, parse_schema: ParseSchema, - program: Option, + mut program: ProgramBuilder, ) -> crate::Result { let mut plan = prepare_update_plan(schema, body, parse_schema)?; optimize_plan(&mut plan, schema)?; @@ -63,12 +63,7 @@ pub fn translate_update( approx_num_insns: 20, approx_num_labels: 4, }; - let mut program = if let Some(mut program) = program { - program.extend(&opts); - program - } else { - ProgramBuilder::new(opts) - }; + program.extend(&opts); emit_program(&mut program, plan, syms)?; Ok(program) } diff --git a/core/vdbe/builder.rs b/core/vdbe/builder.rs index 9f4f8464b..1f84a5399 100644 --- a/core/vdbe/builder.rs +++ b/core/vdbe/builder.rs @@ -234,7 +234,7 @@ impl ProgramBuilder { self.emit_insn(Insn::ResultRow { start_reg, count }); } - pub fn emit_halt(&mut self) { + fn emit_halt(&mut self) { self.emit_insn(Insn::Halt { err_code: 0, description: String::new(), @@ -252,20 +252,6 @@ impl ProgramBuilder { }); } - pub fn emit_init(&mut self) -> BranchOffset { - let target_pc = self.allocate_label(); - self.emit_insn(Insn::Init { target_pc }); - target_pc - } - - pub fn emit_transaction(&mut self, write: bool) { - self.emit_insn(Insn::Transaction { write }); - } - - pub fn emit_goto(&mut self, target_pc: BranchOffset) { - self.emit_insn(Insn::Goto { target_pc }); - } - pub fn add_comment(&mut self, insn_index: BranchOffset, comment: &'static str) { if let Some(comments) = &mut self.comments { comments.push((insn_index.to_offset_int(), comment)); @@ -282,7 +268,7 @@ impl ProgramBuilder { self.constant_spans.push((prev, prev)); } - pub fn emit_constant_insns(&mut self) { + fn emit_constant_insns(&mut self) { // move compile-time constant instructions to the end of the program, where they are executed once after Init jumps to it. // any label_to_resolved_offset that points to an instruction within any moved constant span should be updated to point to the new location. @@ -661,10 +647,7 @@ impl ProgramBuilder { /// query will jump to the Transaction instruction via init_label. pub fn epilogue(&mut self, txn_mode: TransactionMode) { if self.nested_level == 0 { - self.emit_insn(Insn::Halt { - err_code: 0, - description: String::new(), - }); + self.emit_halt(); self.preassign_label_to_next_insn(self.init_label); match txn_mode {