From 762b61ce55b7fff4090616aee8f4506df569431a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Sat, 7 Jun 2025 17:22:41 +0900 Subject: [PATCH 1/6] Add insn::InsertFlags --- core/vdbe/insn.rs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/core/vdbe/insn.rs b/core/vdbe/insn.rs index 976f6aba4..93a731188 100644 --- a/core/vdbe/insn.rs +++ b/core/vdbe/insn.rs @@ -82,6 +82,30 @@ impl IdxInsertFlags { } } +#[derive(Clone, Copy, Debug, Default)] +pub struct InsertFlags(pub u8); + +impl InsertFlags { + pub const UPDATE: u8 = 0x01; // Flag indicating this is part of an UPDATE statement + + pub fn new() -> Self { + InsertFlags(0) + } + + pub fn has(&self, flag: u8) -> bool { + (self.0 & flag) != 0 + } + + pub fn update(mut self, is_update: bool) -> Self { + if is_update { + self.0 |= InsertFlags::UPDATE; + } else { + self.0 &= !InsertFlags::UPDATE; + } + self + } +} + #[derive(Clone, Copy, Debug)] pub enum RegisterOrLiteral { Register(usize), @@ -665,7 +689,7 @@ pub enum Insn { cursor: CursorID, key_reg: usize, // Must be int. record_reg: usize, // Blob of record data. - flag: usize, // Flags used by insert, for now not used. + flag: InsertFlags, // Flags used by insert, for now not used. table_name: String, }, From a9c096bb0113b80a5a04ec306c1c601fd6deeddd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Sat, 7 Jun 2025 17:23:23 +0900 Subject: [PATCH 2/6] Skip increasing n_changes for insn::Insert when it's UPDATE query --- core/translate/emitter.rs | 4 ++-- core/translate/insert.rs | 6 +++--- core/translate/mod.rs | 5 ++++- core/translate/schema.rs | 8 ++++---- core/vdbe/execute.rs | 13 ++++++++++--- core/vdbe/explain.rs | 2 +- 6 files changed, 24 insertions(+), 14 deletions(-) diff --git a/core/translate/emitter.rs b/core/translate/emitter.rs index 9754ebc63..80279ef85 100644 --- a/core/translate/emitter.rs +++ b/core/translate/emitter.rs @@ -29,7 +29,7 @@ use crate::translate::plan::{DeletePlan, Plan, Search}; use crate::translate::values::emit_values; use crate::util::exprs_are_equivalent; use crate::vdbe::builder::{CursorKey, CursorType, ProgramBuilder}; -use crate::vdbe::insn::{CmpInsFlags, IdxInsertFlags, RegisterOrLiteral}; +use crate::vdbe::insn::{CmpInsFlags, IdxInsertFlags, InsertFlags, RegisterOrLiteral}; use crate::vdbe::{insn::Insn, BranchOffset}; use crate::{Result, SymbolTable}; @@ -1319,7 +1319,7 @@ fn emit_update_insns( cursor: cursor_id, key_reg: rowid_set_clause_reg.unwrap_or(beg), record_reg, - flag: 0, + flag: InsertFlags::new().update(true), table_name: table_ref.identifier.clone(), }); } else if let Some(_) = table_ref.virtual_table() { diff --git a/core/translate/insert.rs b/core/translate/insert.rs index d664aca6c..b852a3344 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -8,7 +8,7 @@ use crate::error::SQLITE_CONSTRAINT_PRIMARYKEY; use crate::schema::{IndexColumn, Table}; use crate::util::normalize_ident; use crate::vdbe::builder::{ProgramBuilderOpts, QueryMode}; -use crate::vdbe::insn::{IdxInsertFlags, RegisterOrLiteral}; +use crate::vdbe::insn::{IdxInsertFlags, InsertFlags, RegisterOrLiteral}; use crate::vdbe::BranchOffset; use crate::{ schema::{Column, Schema}, @@ -212,7 +212,7 @@ pub fn translate_insert( cursor: temp_cursor_id, key_reg: rowid_reg, record_reg, - flag: 0, + flag: InsertFlags::new(), table_name: "".to_string(), }); @@ -520,7 +520,7 @@ pub fn translate_insert( cursor: cursor_id, key_reg: rowid_reg, record_reg: record_register, - flag: 0, + flag: InsertFlags::new(), table_name: table_name.to_string(), }); diff --git a/core/translate/mod.rs b/core/translate/mod.rs index c796ec8ba..a357e869d 100644 --- a/core/translate/mod.rs +++ b/core/translate/mod.rs @@ -66,7 +66,10 @@ pub fn translate( ) -> Result { let change_cnt_on = matches!( stmt, - ast::Stmt::CreateIndex { .. } | ast::Stmt::Delete(..) | ast::Stmt::Insert(..) + ast::Stmt::CreateIndex { .. } + | ast::Stmt::Delete(..) + | ast::Stmt::Insert(..) + | ast::Stmt::Update(..) ); // These options will be extended whithin each translate program diff --git a/core/translate/schema.rs b/core/translate/schema.rs index 1b1f6c040..7f6a623e0 100644 --- a/core/translate/schema.rs +++ b/core/translate/schema.rs @@ -16,7 +16,7 @@ use crate::translate::ProgramBuilderOpts; use crate::translate::QueryMode; use crate::util::PRIMARY_KEY_AUTOMATIC_INDEX_NAME_PREFIX; use crate::vdbe::builder::CursorType; -use crate::vdbe::insn::{CmpInsFlags, Insn}; +use crate::vdbe::insn::{CmpInsFlags, InsertFlags, Insn}; use crate::LimboError; use crate::SymbolTable; use crate::{bail_parse_error, Result}; @@ -234,7 +234,7 @@ pub fn emit_schema_entry( cursor: sqlite_schema_cursor_id, key_reg: rowid_reg, record_reg, - flag: 0, + flag: InsertFlags::new(), table_name: tbl_name.to_string(), }); } @@ -843,7 +843,7 @@ pub fn translate_drop_table( cursor: ephemeral_cursor_id, key_reg: schema_row_id_register, record_reg: schema_data_register, - flag: 0, + flag: InsertFlags::new(), table_name: "scratch_table".to_string(), }); @@ -925,7 +925,7 @@ pub fn translate_drop_table( cursor: sqlite_schema_cursor_id_1, key_reg: schema_row_id_register, record_reg: new_record_register, - flag: 0, + flag: InsertFlags::new(), table_name: SQLITE_TABLEID.to_string(), }); diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 0db530c5e..35a9170f3 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -74,6 +74,7 @@ use crate::{ }; use super::{get_new_rowid, make_record, Program, ProgramState, Register}; +use crate::vdbe::insn::InsertFlags; use crate::{ bail_constraint_error, must_be_btree_cursor, resolve_ext_path, MvStore, Pager, Result, DATABASE_VERSION, @@ -3809,7 +3810,7 @@ pub fn op_insert( cursor, key_reg, record_reg, - flag: _, + flag, table_name: _, } = insn else { @@ -3836,8 +3837,12 @@ pub fn op_insert( if let Some(conn) = program.connection.upgrade() { conn.update_last_rowid(rowid); } - let prev_changes = program.n_change.get(); - program.n_change.set(prev_changes + 1); + + // n_changes is increased when Insn::Delete is executed, so we can skip for Insn::Insert + if !flag.has(InsertFlags::UPDATE) { + let prev_changes = program.n_change.get(); + program.n_change.set(prev_changes + 1); + } } } } @@ -3866,6 +3871,7 @@ pub fn op_delete( return_if_io!(cursor.delete()); } let prev_changes = program.n_change.get(); + println!("[Insn::Delete] set n_changes to {}", prev_changes + 1); program.n_change.set(prev_changes + 1); state.pc += 1; Ok(InsnFunctionStepResult::Step) @@ -3925,6 +3931,7 @@ pub fn op_idx_delete( return_if_io!(cursor.delete()); } let n_change = program.n_change.get(); + println!("[Insn::IdxDelete] set n_changes to {}", n_change + 1); program.n_change.set(n_change + 1); state.pc += 1; state.op_idx_delete_state = None; diff --git a/core/vdbe/explain.rs b/core/vdbe/explain.rs index 69bb886c1..1200d9dc3 100644 --- a/core/vdbe/explain.rs +++ b/core/vdbe/explain.rs @@ -1091,7 +1091,7 @@ pub fn insn_to_str( *record_reg as i32, *key_reg as i32, Value::build_text(&table_name), - *flag as u16, + flag.0 as u16, format!("intkey=r[{}] data=r[{}]", key_reg, record_reg), ), Insn::Delete { cursor_id } => ( From 3b671302502c7d0990a4b5e308d08cfe1487a694 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Sat, 7 Jun 2025 17:37:36 +0900 Subject: [PATCH 3/6] Add total_changes test --- testing/total-changes.test | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/testing/total-changes.test b/testing/total-changes.test index 488f99b79..5f155a1ab 100644 --- a/testing/total-changes.test +++ b/testing/total-changes.test @@ -21,3 +21,24 @@ do_execsql_test_on_specific_db {:memory:} total-changes-on-multiple-inserts { insert into temp values (4), (5), (6), (7); select total_changes(); } {7} + +do_execsql_test_on_specific_db {:memory:} total-changes-on-update-single-row { + create table temp (t1 integer primary key, t2 text); + insert into temp values (1, 'a'), (2, 'b'), (3, 'c'); + update temp set t2 = 'z' where t1 = 2; + select total_changes(); +} {4} + +do_execsql_test_on_specific_db {:memory:} total-changes-on-update-multiple-rows { + create table temp (t1 integer primary key, t2 text); + insert into temp values (1, 'a'), (2, 'b'), (3, 'c'); + update temp set t2 = 'x' where t1 > 1; + select total_changes(); +} {5} + +do_execsql_test_on_specific_db {:memory:} total-changes-on-update-no-match { + create table temp (t1 integer primary key, t2 text); + insert into temp values (1, 'a'), (2, 'b'); + update temp set t2 = 'y' where t1 = 99; + select total_changes(); +} {2} From f52f881bc839a018d827b76183e8ff29e376e168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Sat, 7 Jun 2025 17:44:47 +0900 Subject: [PATCH 4/6] Remove unnecessary comment --- core/vdbe/execute.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 35a9170f3..d6b30a38a 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -3871,7 +3871,6 @@ pub fn op_delete( return_if_io!(cursor.delete()); } let prev_changes = program.n_change.get(); - println!("[Insn::Delete] set n_changes to {}", prev_changes + 1); program.n_change.set(prev_changes + 1); state.pc += 1; Ok(InsnFunctionStepResult::Step) @@ -3931,7 +3930,7 @@ pub fn op_idx_delete( return_if_io!(cursor.delete()); } let n_change = program.n_change.get(); - println!("[Insn::IdxDelete] set n_changes to {}", n_change + 1); + tracing::debug!("[Insn::IdxDelete] set n_changes to {}", n_change + 1); program.n_change.set(n_change + 1); state.pc += 1; state.op_idx_delete_state = None; From 0193e1525b907161895360f49dccfa2a019b91bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Sat, 7 Jun 2025 17:51:24 +0900 Subject: [PATCH 5/6] Nit --- core/vdbe/execute.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index d6b30a38a..c338a586b 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -3838,7 +3838,7 @@ pub fn op_insert( conn.update_last_rowid(rowid); } - // n_changes is increased when Insn::Delete is executed, so we can skip for Insn::Insert + // n_change is increased when Insn::Delete is executed, so we can skip for Insn::Insert if !flag.has(InsertFlags::UPDATE) { let prev_changes = program.n_change.get(); program.n_change.set(prev_changes + 1); @@ -3930,7 +3930,6 @@ pub fn op_idx_delete( return_if_io!(cursor.delete()); } let n_change = program.n_change.get(); - tracing::debug!("[Insn::IdxDelete] set n_changes to {}", n_change + 1); program.n_change.set(n_change + 1); state.pc += 1; state.op_idx_delete_state = None; From ac1fbdc9cebcc602f899204ce65689e58f4de8ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=84=A0=EC=9A=B0?= Date: Sun, 8 Jun 2025 13:36:15 +0900 Subject: [PATCH 6/6] Remove @Disabled from bindings/java test --- .../src/test/java/tech/turso/jdbc4/JDBC4StatementTest.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/bindings/java/src/test/java/tech/turso/jdbc4/JDBC4StatementTest.java b/bindings/java/src/test/java/tech/turso/jdbc4/JDBC4StatementTest.java index 70abce902..1f75725bc 100644 --- a/bindings/java/src/test/java/tech/turso/jdbc4/JDBC4StatementTest.java +++ b/bindings/java/src/test/java/tech/turso/jdbc4/JDBC4StatementTest.java @@ -11,7 +11,6 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.Properties; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import tech.turso.TestUtils; @@ -83,11 +82,9 @@ class JDBC4StatementTest { } @Test - @Disabled("limbo's total_changes() works differently from sqlite's total_changes()") void execute_update_should_return_number_of_updated_elements() throws Exception { assertThat(stmt.executeUpdate("CREATE TABLE s1 (c1 INT);")).isEqualTo(0); assertThat(stmt.executeUpdate("INSERT INTO s1 VALUES (1), (2), (3);")).isEqualTo(3); - assertThat(stmt.executeUpdate("UPDATE s1 SET c1 = 0;")).isEqualTo(3); }