diff --git a/core/translate/alter.rs b/core/translate/alter.rs index bf2b96522..1319195ba 100644 --- a/core/translate/alter.rs +++ b/core/translate/alter.rs @@ -13,9 +13,7 @@ use crate::{ LimboError, Result, SymbolTable, }; -use super::{ - emitter::TransactionMode, schema::SQLITE_TABLEID, update::translate_update_with_after, -}; +use super::{schema::SQLITE_TABLEID, update::translate_update_with_after}; pub fn translate_alter_table( alter: (ast::QualifiedName, ast::AlterTableBody), @@ -24,6 +22,7 @@ pub fn translate_alter_table( mut program: ProgramBuilder, connection: &Arc, ) -> Result { + program.begin_write_operation(); let (table_name, alter_table) = alter; let table_name = table_name.name.as_str(); if schema.table_has_indexes(table_name) && !schema.indexes_enabled() { @@ -315,8 +314,6 @@ pub fn translate_alter_table( where_clause: None, }); - program.epilogue(TransactionMode::Write); - program } ast::AlterTableBody::RenameTo(new_name) => { @@ -398,8 +395,6 @@ pub fn translate_alter_table( to: new_name.to_owned(), }); - program.epilogue(TransactionMode::Write); - program } }) diff --git a/core/translate/compound_select.rs b/core/translate/compound_select.rs index 973c5f27f..9cc28b65c 100644 --- a/core/translate/compound_select.rs +++ b/core/translate/compound_select.rs @@ -1,5 +1,5 @@ use crate::schema::{Index, IndexColumn, Schema}; -use crate::translate::emitter::{emit_query, LimitCtx, TransactionMode, TranslateCtx}; +use crate::translate::emitter::{emit_query, LimitCtx, TranslateCtx}; use crate::translate::plan::{Plan, QueryDestination, SelectPlan}; use crate::vdbe::builder::{CursorType, ProgramBuilder}; use crate::vdbe::insn::Insn; @@ -33,7 +33,6 @@ pub fn emit_program_for_compound_select( // Trivial exit on LIMIT 0 if let Some(limit) = limit { if *limit == 0 { - program.epilogue(TransactionMode::Read); program.result_columns = right_plan.result_columns; program.table_references.extend(right_plan.table_references); return Ok(()); @@ -89,7 +88,6 @@ pub fn emit_program_for_compound_select( reg_result_cols_start, )?; - program.epilogue(TransactionMode::Read); program.result_columns = right_plan.result_columns; program.table_references.extend(right_plan.table_references); diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 6a2d90661..68de235e6 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -194,6 +194,7 @@ pub enum OperationMode { } #[derive(Clone, Copy, Debug)] +/// Sqlite always considers Read transactions implicit pub enum TransactionMode { None, Read, @@ -238,7 +239,6 @@ fn emit_program_for_select( // Trivial exit on LIMIT 0 if let Some(limit) = plan.limit { if limit == 0 { - program.epilogue(TransactionMode::Read); program.result_columns = plan.result_columns; program.table_references.extend(plan.table_references); return Ok(()); @@ -247,13 +247,6 @@ fn emit_program_for_select( // Emit main parts of query emit_query(program, &mut plan, &mut t_ctx)?; - // Finalize program - if plan.table_references.joined_tables().is_empty() { - program.epilogue(TransactionMode::None); - } else { - program.epilogue(TransactionMode::Read); - } - program.result_columns = plan.result_columns; program.table_references.extend(plan.table_references); Ok(()) @@ -416,7 +409,6 @@ fn emit_program_for_delete( // exit early if LIMIT 0 if let Some(0) = plan.limit { - program.epilogue(TransactionMode::Write); program.result_columns = plan.result_columns; program.table_references.extend(plan.table_references); return Ok(()); @@ -472,7 +464,6 @@ fn emit_program_for_delete( program.preassign_label_to_next_insn(after_main_loop_label); // Finalize program - program.epilogue(TransactionMode::Write); program.result_columns = plan.result_columns; program.table_references.extend(plan.table_references); Ok(()) @@ -673,7 +664,6 @@ fn emit_program_for_update( // Exit on LIMIT 0 if let Some(0) = plan.limit { - program.epilogue(TransactionMode::None); program.result_columns = plan.returning.unwrap_or_default(); program.table_references.extend(plan.table_references); return Ok(()); @@ -767,8 +757,6 @@ fn emit_program_for_update( after(program); - // Finalize program - program.epilogue(TransactionMode::Write); program.result_columns = plan.returning.unwrap_or_default(); program.table_references.extend(plan.table_references); Ok(()) diff --git a/core/translate/index.rs b/core/translate/index.rs index e6359f672..3041e431f 100644 --- a/core/translate/index.rs +++ b/core/translate/index.rs @@ -237,9 +237,6 @@ pub fn translate_create_index( cursor_id: sqlite_schema_cursor_id, }); - // Epilogue: - program.epilogue(super::emitter::TransactionMode::Write); - Ok(program) } @@ -338,7 +335,6 @@ pub fn translate_drop_index( // then return normaly, otherwise show an error. if maybe_index.is_none() { if if_exists { - program.epilogue(super::emitter::TransactionMode::Write); return Ok(program); } else { return Err(crate::error::LimboError::InvalidArgument(format!( @@ -453,8 +449,5 @@ pub fn translate_drop_index( }); } - // Epilogue: - program.epilogue(super::emitter::TransactionMode::Write); - Ok(program) } diff --git a/core/translate/insert.rs b/core/translate/insert.rs index 6448d8d21..f89ffdad3 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -87,7 +87,6 @@ pub fn translate_insert( on_conflict, &resolver, )?; - program.epilogue(super::emitter::TransactionMode::Write); return Ok(program); } @@ -631,7 +630,6 @@ pub fn translate_insert( } program.resolve_label(halt_label, program.offset()); - program.epilogue(super::emitter::TransactionMode::Write); Ok(program) } diff --git a/core/translate/mod.rs b/core/translate/mod.rs index e3a2eb940..9a273fe83 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -100,7 +100,7 @@ pub fn translate( stmt => translate_inner(schema, stmt, syms, program, &connection)?, }; - // TODO: bring epilogue here when I can sort out what instructions correspond to a Write or a Read transaction + program.epilogue(); Ok(program.build(connection, change_cnt_on, input)) } @@ -112,9 +112,32 @@ pub fn translate_inner( schema: &Schema, stmt: ast::Stmt, syms: &SymbolTable, - program: ProgramBuilder, + mut program: ProgramBuilder, connection: &Arc, ) -> Result { + // Indicate write operations so that in the epilogue we can emit the correct type of transaction + if matches!( + stmt, + ast::Stmt::AlterTable(..) + | ast::Stmt::CreateIndex { .. } + | ast::Stmt::CreateTable { .. } + | ast::Stmt::CreateTrigger { .. } + | ast::Stmt::CreateView { .. } + | ast::Stmt::CreateVirtualTable(..) + | ast::Stmt::Delete(..) + | ast::Stmt::DropIndex { .. } + | ast::Stmt::DropTable { .. } + | ast::Stmt::Reindex { .. } + | ast::Stmt::Update(..) + | ast::Stmt::Insert(..) + ) { + program.begin_write_operation(); + } + // Indicate read operations so that in the epilogue we can emit the correct type of transaction + if matches!(stmt, ast::Stmt::Select { .. }) { + program.begin_read_operation(); + } + let program = match stmt { ast::Stmt::AlterTable(alter) => { translate_alter_table(*alter, syms, schema, program, connection)? @@ -124,7 +147,7 @@ pub fn translate_inner( attach::translate_attach(&expr, &db_name, &key, schema, syms, program)? } ast::Stmt::Begin(tx_type, tx_name) => { - translate_tx_begin(tx_type, tx_name, &schema, program)? + translate_tx_begin(tx_type, tx_name, schema, program)? } ast::Stmt::Commit(tx_name) => translate_tx_commit(tx_name, program)?, ast::Stmt::CreateIndex { diff --git a/core/translate/pragma.rs b/core/translate/pragma.rs index a9f1b3729..748436ddc 100644 --- a/core/translate/pragma.rs +++ b/core/translate/pragma.rs @@ -30,7 +30,6 @@ fn list_pragmas(program: &mut ProgramBuilder) { program.emit_result_row(register, 1); } program.add_pragma_result_column("pragma_list".into()); - program.epilogue(TransactionMode::None); } #[allow(clippy::too_many_arguments)] @@ -68,7 +67,15 @@ pub fn translate_pragma( _ => update_pragma(pragma, schema, value, pager, connection, program)?, }, }; - program.epilogue(mode); + match mode { + TransactionMode::None => {} + TransactionMode::Read => { + program.begin_read_operation(); + } + TransactionMode::Write => { + program.begin_write_operation(); + } + } Ok(program) } diff --git a/core/translate/rollback.rs b/core/translate/rollback.rs index c175786ed..271146552 100644 --- a/core/translate/rollback.rs +++ b/core/translate/rollback.rs @@ -2,7 +2,6 @@ use turso_sqlite3_parser::ast::Name; use crate::{ schema::Schema, - translate::emitter::TransactionMode, vdbe::{builder::ProgramBuilder, insn::Insn}, Result, SymbolTable, }; @@ -22,6 +21,6 @@ pub fn translate_rollback( auto_commit: true, rollback: true, }); - program.epilogue_maybe_rollback(TransactionMode::None, true); + program.rollback(); Ok(program) } diff --git a/core/translate/schema.rs b/core/translate/schema.rs index ef3739644..24f79aab5 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -45,8 +45,6 @@ pub fn translate_create_table( let normalized_tbl_name = normalize_ident(tbl_name.name.as_str()); if schema.get_table(&normalized_tbl_name).is_some() { if if_not_exists { - program.epilogue(crate::translate::emitter::TransactionMode::Write); - return Ok(program); } bail_parse_error!("Table {} already exists", normalized_tbl_name); @@ -164,7 +162,6 @@ pub fn translate_create_table( }); // TODO: SqlExec - program.epilogue(super::emitter::TransactionMode::Write); Ok(program) } @@ -542,7 +539,6 @@ pub fn translate_create_virtual_table( }; if schema.get_table(&table_name).is_some() { if *if_not_exists { - program.epilogue(crate::translate::emitter::TransactionMode::Write); return Ok(program); } bail_parse_error!("Table {} already exists", tbl_name); @@ -613,8 +609,6 @@ pub fn translate_create_virtual_table( where_clause: Some(parse_schema_where_clause), }); - program.epilogue(super::emitter::TransactionMode::Write); - Ok(program) } @@ -638,8 +632,6 @@ pub fn translate_drop_table( let table = schema.get_table(tbl_name.name.as_str()); if table.is_none() { if if_exists { - program.epilogue(crate::translate::emitter::TransactionMode::Write); - return Ok(program); } bail_parse_error!("No such table: {}", tbl_name.name.as_str()); @@ -941,8 +933,5 @@ pub fn translate_drop_table( p5: 0, }); - // end of the program - program.epilogue(super::emitter::TransactionMode::Write); - Ok(program) } diff --git a/core/translate/transaction.rs b/core/translate/transaction.rs index 8a4e6ab21..6ecb113a9 100644 --- a/core/translate/transaction.rs +++ b/core/translate/transaction.rs @@ -36,7 +36,6 @@ pub fn translate_tx_begin( }); } } - program.epilogue(super::emitter::TransactionMode::None); Ok(program) } @@ -53,6 +52,5 @@ pub fn translate_tx_commit( auto_commit: true, rollback: false, }); - program.epilogue(super::emitter::TransactionMode::None); Ok(program) } diff --git a/core/vdbe/builder.rs b/core/vdbe/builder.rs index a71c01ceb..a945b3ce9 100644 --- a/core/vdbe/builder.rs +++ b/core/vdbe/builder.rs @@ -111,6 +111,9 @@ pub struct ProgramBuilder { init_label: BranchOffset, start_offset: BranchOffset, capture_data_changes_mode: CaptureDataChangesMode, + // TODO: when we support multiple dbs, this should be a write mask to track which DBs need to be written + txn_mode: TransactionMode, + rollback: bool, } #[derive(Debug, Clone)] @@ -178,6 +181,8 @@ impl ProgramBuilder { init_label: BranchOffset::Placeholder, start_offset: BranchOffset::Placeholder, capture_data_changes_mode, + txn_mode: TransactionMode::None, + rollback: false, } } @@ -751,34 +756,40 @@ impl ProgramBuilder { } } - /// Clean up and finalize the program, resolving any remaining labels - /// Note that although these are the final instructions, typically an SQLite - /// query will jump to the Transaction instruction via init_label. - pub fn epilogue(&mut self, txn_mode: TransactionMode) { - self.epilogue_maybe_rollback(txn_mode, false); + /// Tries to mirror: https://github.com/sqlite/sqlite/blob/e77e589a35862f6ac9c4141cfd1beb2844b84c61/src/build.c#L5379 + /// TODO: as we currently do not support multiple dbs + /// this function just sets a write operation/transaction for the current db + pub fn begin_write_operation(&mut self) { + self.txn_mode = TransactionMode::Write; + } + + pub fn begin_read_operation(&mut self) { + // Just override the transaction mode when it is None + if matches!(self.txn_mode, TransactionMode::None) { + self.txn_mode = TransactionMode::Read; + } + } + + /// Indicates the rollback behvaiour for the halt instruction in epilogue + pub fn rollback(&mut self) { + self.rollback = true; } /// Clean up and finalize the program, resolving any remaining labels /// Note that although these are the final instructions, typically an SQLite /// query will jump to the Transaction instruction via init_label. - /// "rollback" flag is used to determine if halt should rollback the transaction. - pub fn epilogue_maybe_rollback(&mut self, txn_mode: TransactionMode, rollback: bool) { + pub fn epilogue(&mut self) { if self.nested_level == 0 { - self.emit_halt(rollback); + // "rollback" flag is used to determine if halt should rollback the transaction. + self.emit_halt(self.rollback); self.preassign_label_to_next_insn(self.init_label); - match txn_mode { - TransactionMode::Read => self.emit_insn(Insn::Transaction { + if !matches!(self.txn_mode, TransactionMode::None) { + self.emit_insn(Insn::Transaction { db: 0, - write: false, + write: matches!(self.txn_mode, TransactionMode::Write), schema_cookie: 0, // TODO: placeholder until we have epilogue being called only in one place - }), - TransactionMode::Write => self.emit_insn(Insn::Transaction { - db: 0, - write: true, - schema_cookie: 0, // TODO: placeholder until we have epilogue being called only in one place - }), - TransactionMode::None => {} + }); } self.emit_constant_insns();