From eff5de50c598a5679745d1285256a0ece1d65bf9 Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Mon, 13 Jan 2025 19:53:06 -0300 Subject: [PATCH 1/2] refactor: make `translate_*` functions accept ProgramBuilder simplifies function signatures and allows attaching more context to ProgramStatus on `translate::translate`, useful for query parameters. --- core/translate/delete.rs | 13 +++--- core/translate/emitter.rs | 57 +++++++++++-------------- core/translate/insert.rs | 25 ++++------- core/translate/mod.rs | 89 ++++++++++++++++----------------------- core/translate/select.rs | 15 +++---- 5 files changed, 81 insertions(+), 118 deletions(-) diff --git a/core/translate/delete.rs b/core/translate/delete.rs index dcaed53e6..373a74024 100644 --- a/core/translate/delete.rs +++ b/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, limit: Option>, - database_header: Rc>, - connection: Weak, syms: &SymbolTable, -) -> Result { +) -> 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( diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 8fdff8cd1..cc4df6d8c 100644 --- a/core/translate/emitter.rs +++ b/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 @@ -154,40 +151,37 @@ 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>, + program: &mut ProgramBuilder, plan: Plan, - connection: Weak, syms: &SymbolTable, -) -> Result { +) -> 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>, + program: &mut ProgramBuilder, mut plan: SelectPlan, - connection: Weak, syms: &SymbolTable, -) -> Result { - 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 +257,11 @@ pub fn emit_query<'a>( } fn emit_program_for_delete( - database_header: Rc>, + program: &mut ProgramBuilder, mut plan: DeletePlan, - connection: Weak, syms: &SymbolTable, -) -> Result { - 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(); @@ -280,7 +273,7 @@ fn emit_program_for_delete( // Initialize cursors and other resources needed for query execution init_loop( - &mut program, + program, &mut t_ctx, &plan.source, &OperationMode::DELETE, @@ -288,23 +281,23 @@ fn emit_program_for_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>( diff --git a/core/translate/insert.rs b/core/translate/insert.rs index 2dec74248..1b1669ddb 100644 --- a/core/translate/insert.rs +++ b/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, @@ -11,21 +10,17 @@ use crate::util::normalize_ident; use crate::vdbe::BranchOffset; use crate::{ schema::{Column, Schema}, - storage::sqlite3_ondisk::DatabaseHeader, translate::expr::translate_expr, - vdbe::{ - builder::{CursorType, ProgramBuilder}, - insn::Insn, - Program, - }, + vdbe::{builder::{CursorType, ProgramBuilder}, insn::Insn}, SymbolTable, }; -use crate::{Connection, Result}; +use crate::Result; use super::emitter::Resolver; #[allow(clippy::too_many_arguments)] pub fn translate_insert( + program: &mut ProgramBuilder, schema: &Schema, with: &Option, on_conflict: &Option, @@ -33,17 +28,14 @@ pub fn translate_insert( columns: &Option, body: &InsertBody, _returning: &Option>, - database_header: Rc>, - connection: Weak, syms: &SymbolTable, -) -> Result { +) -> 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 +110,7 @@ pub fn translate_insert( for value in values { populate_column_registers( - &mut program, + program, value, &column_mappings, column_registers_start, @@ -157,7 +149,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 +254,8 @@ pub fn translate_insert( program.emit_insn(Insn::Goto { target_pc: start_offset, }); - Ok(program.build(database_header, connection)) + + Ok(()) } #[derive(Debug)] diff --git a/core/translate/mod.rs b/core/translate/mod.rs index fdbbc47e0..9a990eb36 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -48,6 +48,7 @@ pub fn translate( connection: Weak, syms: &SymbolTable, ) -> Result { + 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>, - connection: Weak, schema: &Schema, -) -> Result { - 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, database_header: Rc>, pager: Rc, - connection: Weak, -) -> Result { - 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( diff --git a/core/translate/select.rs b/core/translate/select.rs index 44dcb5288..b1be01169 100644 --- a/core/translate/select.rs +++ b/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>, - connection: Weak, syms: &SymbolTable, -) -> Result { +) -> 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 { From 2f2c96fa2c90ef9f6bf7cf2f2645010006f1bc98 Mon Sep 17 00:00:00 2001 From: "Levy A." Date: Mon, 13 Jan 2025 21:31:33 -0300 Subject: [PATCH 2/2] chore: cargo fmt --- core/translate/emitter.rs | 13 ++----------- core/translate/insert.rs | 7 +++++-- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index cc4df6d8c..e376da160 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -150,11 +150,7 @@ 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( - program: &mut ProgramBuilder, - plan: Plan, - syms: &SymbolTable, -) -> Result<()> { +pub fn emit_program(program: &mut ProgramBuilder, plan: Plan, syms: &SymbolTable) -> Result<()> { match plan { Plan::Select(plan) => emit_program_for_select(program, plan, syms), Plan::Delete(plan) => emit_program_for_delete(program, plan, syms), @@ -272,12 +268,7 @@ fn emit_program_for_delete( } // Initialize cursors and other resources needed for query execution - init_loop( - 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( diff --git a/core/translate/insert.rs b/core/translate/insert.rs index 1b1669ddb..d8a6b4149 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -8,13 +8,16 @@ 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}, translate::expr::translate_expr, - vdbe::{builder::{CursorType, ProgramBuilder}, insn::Insn}, + vdbe::{ + builder::{CursorType, ProgramBuilder}, + insn::Insn, + }, SymbolTable, }; -use crate::Result; use super::emitter::Resolver;