From 2d7a27fbfa557cc315b8e85149d874c9c4be0a3c Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Mon, 24 Mar 2025 22:03:32 -0400 Subject: [PATCH 1/7] Prevent panic in extension by out of bounds cursor idx --- extensions/tests/src/lib.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/extensions/tests/src/lib.rs b/extensions/tests/src/lib.rs index 92e4f874f..df8e8bca0 100644 --- a/extensions/tests/src/lib.rs +++ b/extensions/tests/src/lib.rs @@ -112,11 +112,14 @@ impl VTabModule for KVStoreVTab { if cursor.index.is_some_and(|c| c >= cursor.rows.len()) { return Err("cursor out of range".into()); } - let (_, ref key, ref val) = cursor.rows[cursor.index.unwrap_or(0)]; - match idx { - 0 => Ok(Value::from_text(key.clone())), // key - 1 => Ok(Value::from_text(val.clone())), // value - _ => Err("Invalid column".into()), + if let Some((_, ref key, ref val)) = cursor.rows.get(cursor.index.unwrap_or(0)) { + match idx { + 0 => Ok(Value::from_text(key.clone())), // key + 1 => Ok(Value::from_text(val.clone())), // value + _ => Err("Invalid column".into()), + } + } else { + Err("cursor out of range".into()) } } } From b685086cadca38f1c83151237ab68cf394a8c096 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Mon, 24 Mar 2025 22:14:48 -0400 Subject: [PATCH 2/7] Support UPDATE for virtual tables --- core/translate/emitter.rs | 85 +++++++++++++++------ core/translate/main_loop.rs | 28 ++----- core/translate/update.rs | 145 +++++++++++++++++++++++++++++++++++- 3 files changed, 210 insertions(+), 48 deletions(-) diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 1ecc16bff..215c6b3ac 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -6,6 +6,7 @@ use std::rc::Rc; use limbo_sqlite3_parser::ast::{self}; use crate::function::Func; +use crate::schema::Table; use crate::translate::plan::{DeletePlan, Plan, Search}; use crate::util::exprs_are_equivalent; use crate::vdbe::builder::ProgramBuilder; @@ -600,20 +601,41 @@ fn emit_update_insns( if table_column.primary_key { program.emit_null(dest, None); } else { - program.emit_insn(Insn::Column { - cursor_id: *index - .as_ref() - .and_then(|(_, id)| { - if column_idx_in_index.is_some() { - Some(id) - } else { - None - } - }) - .unwrap_or(&cursor_id), - column: column_idx_in_index.unwrap_or(idx), - dest, - }); + match &table_ref.table { + Table::BTree(_) => { + program.emit_insn(Insn::Column { + cursor_id: *index + .as_ref() + .and_then(|(_, id)| { + if column_idx_in_index.is_some() { + Some(id) + } else { + None + } + }) + .unwrap_or(&cursor_id), + column: column_idx_in_index.unwrap_or(idx), + dest, + }); + } + Table::Virtual(_) => { + program.emit_insn(Insn::VColumn { + cursor_id: *index + .as_ref() + .and_then(|(_, id)| { + if column_idx_in_index.is_some() { + Some(id) + } else { + None + } + }) + .unwrap_or(&cursor_id), + column: column_idx_in_index.unwrap_or(idx), + dest, + }); + } + typ => unreachable!("query plan generated on unexpected table type {:?}", typ), + } } } } @@ -633,13 +655,34 @@ fn emit_update_insns( count: table_ref.columns().len(), dest_reg: record_reg, }); - program.emit_insn(Insn::InsertAsync { - cursor: cursor_id, - key_reg: rowid_reg, - record_reg, - flag: 0, - }); - program.emit_insn(Insn::InsertAwait { cursor_id }); + match &table_ref.table { + Table::BTree(_) => { + program.emit_insn(Insn::InsertAsync { + cursor: cursor_id, + key_reg: rowid_reg, + record_reg, + flag: 0, + }); + program.emit_insn(Insn::InsertAwait { cursor_id }); + } + Table::Virtual(vtab) => { + let new_rowid = program.alloc_register(); + program.emit_insn(Insn::Copy { + src_reg: rowid_reg, + dst_reg: new_rowid, + amount: 0, + }); + let arg_count = table_ref.columns().len() + 2; + program.emit_insn(Insn::VUpdate { + cursor_id, + arg_count, + start_reg: record_reg, + vtab_ptr: vtab.implementation.as_ref().ctx as usize, + conflict_action: 0u16, + }); + } + _ => unreachable!("unexpected table type"), + } if let Some(limit_reg) = t_ctx.reg_limit { program.emit_insn(Insn::DecrJumpZero { diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index ab7ae1a0e..9575eb900 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -109,7 +109,7 @@ pub fn init_loop( program.emit_insn(Insn::OpenReadAwait {}); } } - (OperationMode::DELETE, Table::BTree(btree)) => { + (OperationMode::DELETE | OperationMode::UPDATE, Table::BTree(btree)) => { let root_page = btree.root_page; program.emit_insn(Insn::OpenWriteAsync { cursor_id, @@ -131,11 +131,7 @@ pub fn init_loop( } program.emit_insn(Insn::OpenWriteAwait {}); } - (OperationMode::SELECT, Table::Virtual(_)) => { - program.emit_insn(Insn::VOpenAsync { cursor_id }); - program.emit_insn(Insn::VOpenAwait {}); - } - (OperationMode::DELETE, Table::Virtual(_)) => { + (_, Table::Virtual(_)) => { program.emit_insn(Insn::VOpenAsync { cursor_id }); program.emit_insn(Insn::VOpenAwait {}); } @@ -158,14 +154,7 @@ pub fn init_loop( }); program.emit_insn(Insn::OpenReadAwait {}); } - OperationMode::DELETE => { - program.emit_insn(Insn::OpenWriteAsync { - cursor_id: table_cursor_id, - root_page: table.table.get_root_page().into(), - }); - program.emit_insn(Insn::OpenWriteAwait {}); - } - OperationMode::UPDATE => { + OperationMode::DELETE | OperationMode::UPDATE => { program.emit_insn(Insn::OpenWriteAsync { cursor_id: table_cursor_id, root_page: table.table.get_root_page().into(), @@ -191,17 +180,10 @@ pub fn init_loop( }); program.emit_insn(Insn::OpenReadAwait); } - OperationMode::DELETE => { + OperationMode::UPDATE | OperationMode::DELETE => { program.emit_insn(Insn::OpenWriteAsync { cursor_id: index_cursor_id, - root_page: index.root_page.into(), - }); - program.emit_insn(Insn::OpenWriteAwait {}); - } - OperationMode::UPDATE => { - program.emit_insn(Insn::OpenWriteAsync { - cursor_id: index_cursor_id, - root_page: index.root_page.into(), + root_page: index.root_page, }); program.emit_insn(Insn::OpenWriteAwait {}); } diff --git a/core/translate/update.rs b/core/translate/update.rs index 347e36acb..de1a568a8 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -1,4 +1,7 @@ +use std::sync::Arc; + use crate::translate::plan::Operation; +use crate::vdbe::BranchOffset; use crate::{ bail_parse_error, schema::{Schema, Table}, @@ -8,7 +11,7 @@ use crate::{ }; use limbo_sqlite3_parser::ast::{self, Expr, ResultColumn, SortOrder, Update}; -use super::emitter::emit_program; +use super::emitter::{emit_program, Resolver}; use super::optimizer::optimize_plan; use super::plan::{ Direction, IterationDirection, Plan, ResultSetColumn, TableReference, UpdatePlan, @@ -53,6 +56,7 @@ pub fn translate_update( ) -> crate::Result { let mut plan = prepare_update_plan(schema, body)?; optimize_plan(&mut plan, schema)?; + let resolver = Resolver::new(syms); // TODO: freestyling these numbers let mut program = ProgramBuilder::new(ProgramBuilderOpts { query_mode, @@ -65,6 +69,12 @@ pub fn translate_update( } pub fn prepare_update_plan(schema: &Schema, body: &mut Update) -> crate::Result { + if body.with.is_some() { + bail_parse_error!("WITH clause is not supported"); + } + if body.or_conflict.is_some() { + bail_parse_error!("ON CONFLICT clause is not supported"); + } let table_name = &body.tbl_name.name; let table = match schema.get_table(table_name.0.as_str()) { Some(table) => table, @@ -86,7 +96,11 @@ pub fn prepare_update_plan(schema: &Schema, body: &mut Update) -> crate::Result< }) .unwrap_or(IterationDirection::Forwards); let table_references = vec![TableReference { - table: Table::BTree(btree_table.clone()), + table: match table.as_ref() { + Table::Virtual(vtab) => Table::Virtual(vtab.clone()), + Table::BTree(btree_table) => Table::BTree(btree_table.clone()), + _ => unreachable!(), + }, identifier: table_name.0.clone(), op: Operation::Scan { iter_dir, @@ -99,8 +113,8 @@ pub fn prepare_update_plan(schema: &Schema, body: &mut Update) -> crate::Result< .iter_mut() .map(|set| { let ident = normalize_ident(set.col_names[0].0.as_str()); - let col_index = btree_table - .columns + let col_index = table + .columns() .iter() .enumerate() .find_map(|(i, col)| { @@ -185,3 +199,126 @@ pub fn prepare_update_plan(schema: &Schema, body: &mut Update) -> crate::Result< contains_constant_false_condition: false, })) } + +// fn translate_vtab_update( +// mut program: ProgramBuilder, +// body: &mut Update, +// table: Arc, +// resolver: &Resolver, +// ) -> crate::Result { +// let start_label = program.allocate_label(); +// program.emit_insn(Insn::Init { +// target_pc: start_label, +// }); +// let start_offset = program.offset(); +// let vtab = table.virtual_table().unwrap(); +// let cursor_id = program.alloc_cursor_id( +// Some(table.get_name().to_string()), +// CursorType::VirtualTable(vtab.clone()), +// ); +// let referenced_tables = vec![TableReference { +// table: Table::Virtual(table.virtual_table().unwrap().clone()), +// identifier: table.get_name().to_string(), +// op: Operation::Scan { iter_dir: None }, +// join_info: None, +// }]; +// program.emit_insn(Insn::VOpenAsync { cursor_id }); +// program.emit_insn(Insn::VOpenAwait {}); +// +// let argv_start = program.alloc_registers(0); +// let end_label = program.allocate_label(); +// let skip_label = program.allocate_label(); +// program.emit_insn(Insn::VFilter { +// cursor_id, +// pc_if_empty: end_label, +// args_reg: argv_start, +// arg_count: 0, +// }); +// +// let loop_start = program.offset(); +// let start_reg = program.alloc_registers(2 + table.columns().len()); +// let old_rowid = start_reg; +// let new_rowid = start_reg + 1; +// let column_regs = start_reg + 2; +// +// program.emit_insn(Insn::RowId { +// cursor_id, +// dest: old_rowid, +// }); +// program.emit_insn(Insn::RowId { +// cursor_id, +// dest: new_rowid, +// }); +// +// for (i, _) in table.columns().iter().enumerate() { +// let dest = column_regs + i; +// program.emit_insn(Insn::VColumn { +// cursor_id, +// column: i, +// dest, +// }); +// } +// +// if let Some(ref mut where_clause) = body.where_clause { +// bind_column_references(where_clause, &referenced_tables, None)?; +// translate_condition_expr( +// &mut program, +// &referenced_tables, +// where_clause, +// ConditionMetadata { +// jump_if_condition_is_true: false, +// jump_target_when_true: BranchOffset::Placeholder, +// jump_target_when_false: skip_label, +// }, +// resolver, +// )?; +// } +// // prepare updated columns in place +// for expr in body.sets.iter() { +// let Some(col_index) = table.columns().iter().position(|t| { +// t.name +// .as_ref() +// .unwrap() +// .eq_ignore_ascii_case(&expr.col_names[0].0) +// }) else { +// bail_parse_error!("column {} not found", expr.col_names[0].0); +// }; +// translate_expr( +// &mut program, +// Some(&referenced_tables), +// &expr.expr, +// column_regs + col_index, +// resolver, +// )?; +// } +// +// let arg_count = 2 + table.columns().len(); +// program.emit_insn(Insn::VUpdate { +// cursor_id, +// arg_count, +// start_reg: old_rowid, +// vtab_ptr: vtab.implementation.ctx as usize, +// conflict_action: 0, +// }); +// +// program.resolve_label(skip_label, program.offset()); +// program.emit_insn(Insn::VNext { +// cursor_id, +// pc_if_next: loop_start, +// }); +// +// program.resolve_label(end_label, program.offset()); +// program.emit_insn(Insn::Halt { +// err_code: 0, +// description: String::new(), +// }); +// program.resolve_label(start_label, program.offset()); +// program.emit_insn(Insn::Transaction { write: true }); +// +// program.emit_constant_insns(); +// program.emit_insn(Insn::Goto { +// target_pc: start_offset, +// }); +// program.table_references = referenced_tables.clone(); +// Ok(program) +// } From 7993857020529f0a108e5e02c0ecfd82d857b8c2 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Mon, 24 Mar 2025 22:29:26 -0400 Subject: [PATCH 3/7] Add py tests for vtab update behavior --- testing/cli_tests/extensions.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/testing/cli_tests/extensions.py b/testing/cli_tests/extensions.py index ac870ee4d..4d289f311 100755 --- a/testing/cli_tests/extensions.py +++ b/testing/cli_tests/extensions.py @@ -398,10 +398,35 @@ def test_kv(): limbo.run_test_fn( "select count(*) from t;", lambda res: "100" == res, "can insert 100 rows" ) + limbo.run_test_fn("update t set value = 'updated' where key = 'key33';", null) + limbo.run_test_fn( + "select * from t where key = 'key33';", + lambda res: res == "key33|updated", + "can update single row", + ) + limbo.run_test_fn( + "select COUNT(*) from t where value = 'updated';", + lambda res: res == "1", + "only updated a single row", + ) + limbo.run_test_fn("update t set value = 'updated2';", null) + limbo.run_test_fn( + "select COUNT(*) from t where value = 'updated2';", + lambda res: res == "100", + "can update all rows", + ) limbo.run_test_fn("delete from t limit 96;", null, "can delete 96 rows") limbo.run_test_fn( "select count(*) from t;", lambda res: "4" == res, "four rows remain" ) + limbo.run_test_fn( + "update t set key = '100' where 1;", null, "where clause evaluates properly" + ) + limbo.run_test_fn( + "select * from t where key = '100';", + lambda res: res == "100|updated2", + "there is only 1 key remaining after setting all keys to same value", + ) limbo.quit() From 0ffecb3021addea05db1a8af937a6a550ab6a761 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Tue, 25 Mar 2025 10:47:05 -0400 Subject: [PATCH 4/7] Add comments to document update on vtabs --- core/translate/update.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/core/translate/update.rs b/core/translate/update.rs index de1a568a8..296d8a6ff 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -1,7 +1,4 @@ -use std::sync::Arc; - use crate::translate::plan::Operation; -use crate::vdbe::BranchOffset; use crate::{ bail_parse_error, schema::{Schema, Table}, @@ -11,7 +8,7 @@ use crate::{ }; use limbo_sqlite3_parser::ast::{self, Expr, ResultColumn, SortOrder, Update}; -use super::emitter::{emit_program, Resolver}; +use super::emitter::emit_program; use super::optimizer::optimize_plan; use super::plan::{ Direction, IterationDirection, Plan, ResultSetColumn, TableReference, UpdatePlan, @@ -56,7 +53,6 @@ pub fn translate_update( ) -> crate::Result { let mut plan = prepare_update_plan(schema, body)?; optimize_plan(&mut plan, schema)?; - let resolver = Resolver::new(syms); // TODO: freestyling these numbers let mut program = ProgramBuilder::new(ProgramBuilderOpts { query_mode, From 62d1447cd681dd67f3571ca1225e97857103d9de Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Wed, 2 Apr 2025 23:48:14 -0400 Subject: [PATCH 5/7] Adapt query plan to handle vatbs for updates --- core/translate/emitter.rs | 183 ++++++++++++++++++++------------------ core/translate/update.rs | 123 ------------------------- core/vdbe/execute.rs | 13 ++- 3 files changed, 103 insertions(+), 216 deletions(-) diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 215c6b3ac..7106bc14a 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -6,7 +6,6 @@ use std::rc::Rc; use limbo_sqlite3_parser::ast::{self}; use crate::function::Func; -use crate::schema::Table; use crate::translate::plan::{DeletePlan, Plan, Search}; use crate::util::exprs_are_equivalent; use crate::vdbe::builder::ProgramBuilder; @@ -531,29 +530,67 @@ fn emit_update_insns( ) -> crate::Result<()> { let table_ref = &plan.table_references.first().unwrap(); let loop_labels = t_ctx.labels_main_loop.first().unwrap(); - let (cursor_id, index) = match &table_ref.op { - Operation::Scan { .. } => (program.resolve_cursor_id(&table_ref.identifier), None), + let (cursor_id, index, is_virtual) = match &table_ref.op { + Operation::Scan { .. } => ( + program.resolve_cursor_id(&table_ref.identifier), + None, + table_ref.virtual_table().is_some(), + ), Operation::Search(search) => match search { - &Search::RowidEq { .. } | Search::RowidSearch { .. } => { - (program.resolve_cursor_id(&table_ref.identifier), None) - } + &Search::RowidEq { .. } | Search::RowidSearch { .. } => ( + program.resolve_cursor_id(&table_ref.identifier), + None, + false, + ), Search::IndexSearch { index, .. } => ( program.resolve_cursor_id(&table_ref.identifier), Some((index.clone(), program.resolve_cursor_id(&index.name))), + false, ), }, _ => return Ok(()), }; - let rowid_reg = program.alloc_register(); + + for cond in plan.where_clause.iter().filter(|c| c.is_constant()) { + let jump_target = program.allocate_label(); + let meta = ConditionMetadata { + jump_if_condition_is_true: false, + jump_target_when_true: jump_target, + jump_target_when_false: t_ctx.label_main_loop_end.unwrap(), + }; + translate_condition_expr( + program, + &plan.table_references, + &cond.expr, + meta, + &t_ctx.resolver, + )?; + program.resolve_label(jump_target, program.offset()); + } + let mut beg = program.alloc_registers( + table_ref.table.columns().len() + + if is_virtual { + 2 // two args before the relevant columns for VUpdate + } else { + 1 // rowid reg + }, + ); program.emit_insn(Insn::RowId { cursor_id, - dest: rowid_reg, + dest: beg, }); // if no rowid, we're done program.emit_insn(Insn::IsNull { - reg: rowid_reg, + reg: beg, target_pc: t_ctx.label_main_loop_end.unwrap(), }); + if is_virtual { + program.emit_insn(Insn::Copy { + src_reg: beg, + dst_reg: beg + 1, + amount: 0, + }) + } if let Some(offset) = t_ctx.reg_offset { program.emit_insn(Insn::IfPos { @@ -577,12 +614,13 @@ fn emit_update_insns( &t_ctx.resolver, )?; } - let first_col_reg = program.alloc_registers(table_ref.table.columns().len()); + // we scan a column at a time, loading either the column's values, or the new value // from the Set expression, into registers so we can emit a MakeRecord and update the row. + let start = if is_virtual { beg + 2 } else { beg + 1 }; for idx in 0..table_ref.columns().len() { - if let Some((idx, expr)) = plan.set_clauses.iter().find(|(i, _)| *i == idx) { - let target_reg = first_col_reg + idx; + let target_reg = start + idx; + if let Some((_, expr)) = plan.set_clauses.iter().find(|(i, _)| *i == idx) { translate_expr( program, Some(&plan.table_references), @@ -597,91 +635,66 @@ fn emit_update_insns( .iter() .position(|c| Some(&c.name) == table_column.name.as_ref()) }); - let dest = first_col_reg + idx; - if table_column.primary_key { - program.emit_null(dest, None); + + // don't emit null for pkey of virtual tables. they require first two args + // before the 'record' to be explicitly non-null + if table_column.primary_key && !is_virtual { + program.emit_null(target_reg, None); + } else if is_virtual { + program.emit_insn(Insn::VColumn { + cursor_id, + column: idx, + dest: target_reg, + }); } else { - match &table_ref.table { - Table::BTree(_) => { - program.emit_insn(Insn::Column { - cursor_id: *index - .as_ref() - .and_then(|(_, id)| { - if column_idx_in_index.is_some() { - Some(id) - } else { - None - } - }) - .unwrap_or(&cursor_id), - column: column_idx_in_index.unwrap_or(idx), - dest, - }); - } - Table::Virtual(_) => { - program.emit_insn(Insn::VColumn { - cursor_id: *index - .as_ref() - .and_then(|(_, id)| { - if column_idx_in_index.is_some() { - Some(id) - } else { - None - } - }) - .unwrap_or(&cursor_id), - column: column_idx_in_index.unwrap_or(idx), - dest, - }); - } - typ => unreachable!("query plan generated on unexpected table type {:?}", typ), - } + program.emit_insn(Insn::Column { + cursor_id: *index + .as_ref() + .and_then(|(_, id)| { + if column_idx_in_index.is_some() { + Some(id) + } else { + None + } + }) + .unwrap_or(&cursor_id), + column: column_idx_in_index.unwrap_or(idx), + dest: target_reg, + }); } } } if let Some(btree_table) = table_ref.btree() { if btree_table.is_strict { program.emit_insn(Insn::TypeCheck { - start_reg: first_col_reg, + start_reg: start, count: table_ref.columns().len(), check_generated: true, table_reference: Rc::clone(&btree_table), }); } - } - let record_reg = program.alloc_register(); - program.emit_insn(Insn::MakeRecord { - start_reg: first_col_reg, - count: table_ref.columns().len(), - dest_reg: record_reg, - }); - match &table_ref.table { - Table::BTree(_) => { - program.emit_insn(Insn::InsertAsync { - cursor: cursor_id, - key_reg: rowid_reg, - record_reg, - flag: 0, - }); - program.emit_insn(Insn::InsertAwait { cursor_id }); - } - Table::Virtual(vtab) => { - let new_rowid = program.alloc_register(); - program.emit_insn(Insn::Copy { - src_reg: rowid_reg, - dst_reg: new_rowid, - amount: 0, - }); - let arg_count = table_ref.columns().len() + 2; - program.emit_insn(Insn::VUpdate { - cursor_id, - arg_count, - start_reg: record_reg, - vtab_ptr: vtab.implementation.as_ref().ctx as usize, - conflict_action: 0u16, - }); - } - _ => unreachable!("unexpected table type"), + let record_reg = program.alloc_register(); + program.emit_insn(Insn::MakeRecord { + start_reg: start, + count: table_ref.columns().len(), + dest_reg: record_reg, + }); + program.emit_insn(Insn::InsertAsync { + cursor: cursor_id, + key_reg: beg, + record_reg, + flag: 0, + }); + program.emit_insn(Insn::InsertAwait { cursor_id }); + } else if let Some(vtab) = table_ref.virtual_table() { + let arg_count = table_ref.columns().len() + 2; + program.emit_insn(Insn::VUpdate { + cursor_id, + arg_count, + start_reg: beg, + vtab_ptr: vtab.implementation.as_ref().ctx as usize, + conflict_action: 0u16, + }); } if let Some(limit_reg) = t_ctx.reg_limit { diff --git a/core/translate/update.rs b/core/translate/update.rs index 296d8a6ff..71293483c 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -195,126 +195,3 @@ pub fn prepare_update_plan(schema: &Schema, body: &mut Update) -> crate::Result< contains_constant_false_condition: false, })) } - -// fn translate_vtab_update( -// mut program: ProgramBuilder, -// body: &mut Update, -// table: Arc
, -// resolver: &Resolver, -// ) -> crate::Result { -// let start_label = program.allocate_label(); -// program.emit_insn(Insn::Init { -// target_pc: start_label, -// }); -// let start_offset = program.offset(); -// let vtab = table.virtual_table().unwrap(); -// let cursor_id = program.alloc_cursor_id( -// Some(table.get_name().to_string()), -// CursorType::VirtualTable(vtab.clone()), -// ); -// let referenced_tables = vec![TableReference { -// table: Table::Virtual(table.virtual_table().unwrap().clone()), -// identifier: table.get_name().to_string(), -// op: Operation::Scan { iter_dir: None }, -// join_info: None, -// }]; -// program.emit_insn(Insn::VOpenAsync { cursor_id }); -// program.emit_insn(Insn::VOpenAwait {}); -// -// let argv_start = program.alloc_registers(0); -// let end_label = program.allocate_label(); -// let skip_label = program.allocate_label(); -// program.emit_insn(Insn::VFilter { -// cursor_id, -// pc_if_empty: end_label, -// args_reg: argv_start, -// arg_count: 0, -// }); -// -// let loop_start = program.offset(); -// let start_reg = program.alloc_registers(2 + table.columns().len()); -// let old_rowid = start_reg; -// let new_rowid = start_reg + 1; -// let column_regs = start_reg + 2; -// -// program.emit_insn(Insn::RowId { -// cursor_id, -// dest: old_rowid, -// }); -// program.emit_insn(Insn::RowId { -// cursor_id, -// dest: new_rowid, -// }); -// -// for (i, _) in table.columns().iter().enumerate() { -// let dest = column_regs + i; -// program.emit_insn(Insn::VColumn { -// cursor_id, -// column: i, -// dest, -// }); -// } -// -// if let Some(ref mut where_clause) = body.where_clause { -// bind_column_references(where_clause, &referenced_tables, None)?; -// translate_condition_expr( -// &mut program, -// &referenced_tables, -// where_clause, -// ConditionMetadata { -// jump_if_condition_is_true: false, -// jump_target_when_true: BranchOffset::Placeholder, -// jump_target_when_false: skip_label, -// }, -// resolver, -// )?; -// } -// // prepare updated columns in place -// for expr in body.sets.iter() { -// let Some(col_index) = table.columns().iter().position(|t| { -// t.name -// .as_ref() -// .unwrap() -// .eq_ignore_ascii_case(&expr.col_names[0].0) -// }) else { -// bail_parse_error!("column {} not found", expr.col_names[0].0); -// }; -// translate_expr( -// &mut program, -// Some(&referenced_tables), -// &expr.expr, -// column_regs + col_index, -// resolver, -// )?; -// } -// -// let arg_count = 2 + table.columns().len(); -// program.emit_insn(Insn::VUpdate { -// cursor_id, -// arg_count, -// start_reg: old_rowid, -// vtab_ptr: vtab.implementation.ctx as usize, -// conflict_action: 0, -// }); -// -// program.resolve_label(skip_label, program.offset()); -// program.emit_insn(Insn::VNext { -// cursor_id, -// pc_if_next: loop_start, -// }); -// -// program.resolve_label(end_label, program.offset()); -// program.emit_insn(Insn::Halt { -// err_code: 0, -// description: String::new(), -// }); -// program.resolve_label(start_label, program.offset()); -// program.emit_insn(Insn::Transaction { write: true }); -// -// program.emit_constant_insns(); -// program.emit_insn(Insn::Goto { -// target_pc: start_offset, -// }); -// program.table_references = referenced_tables.clone(); -// Ok(program) -// } diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index a09da4ac0..0e7fc583b 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1848,17 +1848,14 @@ pub fn op_row_id( let rowid = { let mut index_cursor = state.get_cursor(index_cursor_id); let index_cursor = index_cursor.as_btree_mut(); - let rowid = index_cursor.rowid()?; - rowid + index_cursor.rowid()? }; let mut table_cursor = state.get_cursor(table_cursor_id); let table_cursor = table_cursor.as_btree_mut(); - let deferred_seek = - match table_cursor.seek(SeekKey::TableRowId(rowid.unwrap()), SeekOp::EQ)? { - CursorResult::Ok(_) => None, - CursorResult::IO => Some((index_cursor_id, table_cursor_id)), - }; - deferred_seek + match table_cursor.seek(SeekKey::TableRowId(rowid.unwrap()), SeekOp::EQ)? { + CursorResult::Ok(_) => None, + CursorResult::IO => Some((index_cursor_id, table_cursor_id)), + } }; if let Some(deferred_seek) = deferred_seek { state.deferred_seek = Some(deferred_seek); From 13ae19c78c19c115a9452a19c8c30854bf74f720 Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Fri, 4 Apr 2025 07:08:01 -0400 Subject: [PATCH 6/7] Remove unnecessary clones from mc cursors --- core/vdbe/execute.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 0e7fc583b..5cf2e6cd2 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1582,7 +1582,7 @@ pub fn op_halt( ))); } } - match program.halt(pager.clone(), state, mv_store.clone())? { + match program.halt(pager.clone(), state, mv_store)? { StepResult::Done => Ok(InsnFunctionStepResult::Done), StepResult::IO => Ok(InsnFunctionStepResult::IO), StepResult::Row => Ok(InsnFunctionStepResult::Row), @@ -1661,7 +1661,7 @@ pub fn op_auto_commit( }; let conn = program.connection.upgrade().unwrap(); if matches!(state.halt_state, Some(HaltState::Checkpointing)) { - return match program.halt(pager.clone(), state, mv_store.clone())? { + return match program.halt(pager.clone(), state, mv_store)? { super::StepResult::Done => Ok(InsnFunctionStepResult::Done), super::StepResult::IO => Ok(InsnFunctionStepResult::IO), super::StepResult::Row => Ok(InsnFunctionStepResult::Row), @@ -1689,7 +1689,7 @@ pub fn op_auto_commit( "cannot commit - no transaction is active".to_string(), )); } - return match program.halt(pager.clone(), state, mv_store.clone())? { + return match program.halt(pager.clone(), state, mv_store)? { super::StepResult::Done => Ok(InsnFunctionStepResult::Done), super::StepResult::IO => Ok(InsnFunctionStepResult::IO), super::StepResult::Row => Ok(InsnFunctionStepResult::Row), From f223e66c82a4f862749b6a9c14fcb48d630f6fab Mon Sep 17 00:00:00 2001 From: PThorpe92 Date: Fri, 4 Apr 2025 07:29:16 -0400 Subject: [PATCH 7/7] Remove unused mut and fix merge conflict issues --- core/translate/emitter.rs | 2 +- core/translate/main_loop.rs | 4 ++-- core/translate/update.rs | 3 --- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 7106bc14a..5049bb738 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -567,7 +567,7 @@ fn emit_update_insns( )?; program.resolve_label(jump_target, program.offset()); } - let mut beg = program.alloc_registers( + let beg = program.alloc_registers( table_ref.table.columns().len() + if is_virtual { 2 // two args before the relevant columns for VUpdate diff --git a/core/translate/main_loop.rs b/core/translate/main_loop.rs index 9575eb900..51bd05382 100644 --- a/core/translate/main_loop.rs +++ b/core/translate/main_loop.rs @@ -109,7 +109,7 @@ pub fn init_loop( program.emit_insn(Insn::OpenReadAwait {}); } } - (OperationMode::DELETE | OperationMode::UPDATE, Table::BTree(btree)) => { + (OperationMode::DELETE, Table::BTree(btree)) => { let root_page = btree.root_page; program.emit_insn(Insn::OpenWriteAsync { cursor_id, @@ -183,7 +183,7 @@ pub fn init_loop( OperationMode::UPDATE | OperationMode::DELETE => { program.emit_insn(Insn::OpenWriteAsync { cursor_id: index_cursor_id, - root_page: index.root_page, + root_page: index.root_page.into(), }); program.emit_insn(Insn::OpenWriteAwait {}); } diff --git a/core/translate/update.rs b/core/translate/update.rs index 71293483c..62c6c6f9f 100644 --- a/core/translate/update.rs +++ b/core/translate/update.rs @@ -76,9 +76,6 @@ pub fn prepare_update_plan(schema: &Schema, body: &mut Update) -> crate::Result< Some(table) => table, None => bail_parse_error!("Parse error: no such table: {}", table_name), }; - let Some(btree_table) = table.btree() else { - bail_parse_error!("Error: {} is not a btree table", table_name); - }; let iter_dir = body .order_by .as_ref()