Simplify program epilogue by tracking the transaction mode and rollback status in the ProgramBuilder and then calling epilogue just once

This commit is contained in:
pedrocarlo
2025-07-21 22:50:17 -03:00
parent c567636deb
commit 736748cdf7
11 changed files with 69 additions and 70 deletions

View File

@@ -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<crate::Connection>,
) -> Result<ProgramBuilder> {
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
}
})

View File

@@ -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);

View File

@@ -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(())

View File

@@ -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)
}

View File

@@ -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)
}

View File

@@ -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<Connection>,
) -> Result<ProgramBuilder> {
// 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 {

View File

@@ -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)
}

View File

@@ -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)
}

View File

@@ -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)
}

View File

@@ -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)
}

View File

@@ -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();