From 8e02855c984ba68e955c9032d7fbf58ea53becba Mon Sep 17 00:00:00 2001 From: Pavan-Nambi Date: Tue, 23 Sep 2025 10:52:30 +0530 Subject: [PATCH] update seq table onconflict nothing too and refactor logic into seperate function add testcase for failed fuzz more remove autoincrement if col aint integer fmt --- core/translate/insert.rs | 149 +++++++++++++++++++++------------- testing/autoincr.test | 14 +++- tests/integration/fuzz/mod.rs | 7 +- 3 files changed, 109 insertions(+), 61 deletions(-) diff --git a/core/translate/insert.rs b/core/translate/insert.rs index a2806eb63..550ce31d1 100644 --- a/core/translate/insert.rs +++ b/core/translate/insert.rs @@ -547,6 +547,17 @@ pub fn translate_insert( register: insertion.key_register(), value: 1, }); + + if let Some((seq_cursor_id, _, r_seq_rowid, table_name_reg)) = autoincrement_meta { + emit_update_sqlite_sequence( + &mut program, + schema, + seq_cursor_id, + r_seq_rowid, + table_name_reg, + insertion.key_register(), + )?; + } } else { program.emit_insn(Insn::NewRowid { cursor: cursor_id, @@ -962,65 +973,15 @@ pub fn translate_insert( collation: None, }); - let record_reg = program.alloc_register(); - let record_start_reg = program.alloc_registers(2); - program.emit_insn(Insn::Copy { - src_reg: table_name_reg, - dst_reg: record_start_reg, - extra_amount: 0, - }); - program.emit_insn(Insn::Copy { - src_reg: insertion.key_register(), - dst_reg: record_start_reg + 1, - extra_amount: 0, - }); - let seq_table = schema.get_btree_table("sqlite_sequence").unwrap(); - let affinity_str = seq_table - .columns - .iter() - .map(|col| col.affinity().aff_mask()) - .collect::(); - program.emit_insn(Insn::MakeRecord { - start_reg: record_start_reg, - count: 2, - dest_reg: record_reg, - index_name: None, - affinity_str: Some(affinity_str), - }); + emit_update_sqlite_sequence( + &mut program, + schema, + seq_cursor_id, + r_seq_rowid, + table_name_reg, + insertion.key_register(), + )?; - let update_existing_label = program.allocate_label(); - let end_update_label = program.allocate_label(); - program.emit_insn(Insn::NotNull { - reg: r_seq_rowid, - target_pc: update_existing_label, - }); - - program.emit_insn(Insn::NewRowid { - cursor: seq_cursor_id, - rowid_reg: r_seq_rowid, - prev_largest_reg: 0, - }); - program.emit_insn(Insn::Insert { - cursor: seq_cursor_id, - key_reg: r_seq_rowid, - record_reg, - flag: InsertFlags::new(), - table_name: "sqlite_sequence".to_string(), - }); - program.emit_insn(Insn::Goto { - target_pc: end_update_label, - }); - - program.preassign_label_to_next_insn(update_existing_label); - program.emit_insn(Insn::Insert { - cursor: seq_cursor_id, - key_reg: r_seq_rowid, - record_reg, - flag: InsertFlags(turso_parser::ast::ResolveType::Replace.bit_value() as u8), - table_name: "sqlite_sequence".to_string(), - }); - - program.preassign_label_to_next_insn(end_update_label); program.preassign_label_to_next_insn(no_update_needed_label); program.emit_insn(Insn::Close { cursor_id: seq_cursor_id, @@ -1745,3 +1706,75 @@ pub fn rewrite_partial_index_where( }, ) } + +fn emit_update_sqlite_sequence( + program: &mut ProgramBuilder, + schema: &Schema, + seq_cursor_id: usize, + r_seq_rowid: usize, + table_name_reg: usize, + new_key_reg: usize, +) -> Result<()> { + let record_reg = program.alloc_register(); + let record_start_reg = program.alloc_registers(2); + program.emit_insn(Insn::Copy { + src_reg: table_name_reg, + dst_reg: record_start_reg, + extra_amount: 0, + }); + program.emit_insn(Insn::Copy { + src_reg: new_key_reg, + dst_reg: record_start_reg + 1, + extra_amount: 0, + }); + + let seq_table = schema.get_btree_table("sqlite_sequence").unwrap(); + let affinity_str = seq_table + .columns + .iter() + .map(|col| col.affinity().aff_mask()) + .collect::(); + program.emit_insn(Insn::MakeRecord { + start_reg: record_start_reg, + count: 2, + dest_reg: record_reg, + index_name: None, + affinity_str: Some(affinity_str), + }); + + let update_existing_label = program.allocate_label(); + let end_update_label = program.allocate_label(); + program.emit_insn(Insn::NotNull { + reg: r_seq_rowid, + target_pc: update_existing_label, + }); + + program.emit_insn(Insn::NewRowid { + cursor: seq_cursor_id, + rowid_reg: r_seq_rowid, + prev_largest_reg: 0, + }); + program.emit_insn(Insn::Insert { + cursor: seq_cursor_id, + key_reg: r_seq_rowid, + record_reg, + flag: InsertFlags::new(), + table_name: "sqlite_sequence".to_string(), + }); + program.emit_insn(Insn::Goto { + target_pc: end_update_label, + }); + + program.preassign_label_to_next_insn(update_existing_label); + program.emit_insn(Insn::Insert { + cursor: seq_cursor_id, + key_reg: r_seq_rowid, + record_reg, + flag: InsertFlags(turso_parser::ast::ResolveType::Replace.bit_value() as u8), + table_name: "sqlite_sequence".to_string(), + }); + + program.preassign_label_to_next_insn(end_update_label); + + Ok(()) +} diff --git a/testing/autoincr.test b/testing/autoincr.test index bc89d49a3..cb28e8c5d 100755 --- a/testing/autoincr.test +++ b/testing/autoincr.test @@ -162,4 +162,16 @@ do_execsql_test_on_specific_db {:memory:} autoinc-with-upsert { INSERT INTO t11(a,b) VALUES(2,3),(5,6),(4,3),(1,2) ON CONFLICT(b) DO UPDATE SET a=a+1000; SELECT seq FROM sqlite_sequence WHERE name='t11'; -} {5} \ No newline at end of file +} {5} + + +# refer https://github.com/tursodatabase/turso/pull/2983#issuecomment-3322404270 was discovered while adding autoincr to fuzz tests +do_execsql_test_on_specific_db {:memory:} autoinc-conflict-on-nothing { + CREATE TABLE t (id INTEGER PRIMARY KEY AUTOINCREMENT, k TEXT); + CREATE UNIQUE INDEX idx_k_partial ON t(k) WHERE id > 1; + INSERT INTO t (k) VALUES ('a'); + INSERT INTO t (k) VALUES ('a'); + INSERT INTO t (k) VALUES ('a') ON CONFLICT DO NOTHING; + INSERT INTO t (k) VALUES ('b'); + SELECT * FROM t ORDER BY id; +} {1|a 2|a 4|b} \ No newline at end of file diff --git a/tests/integration/fuzz/mod.rs b/tests/integration/fuzz/mod.rs index 01c72c288..f5e96dee2 100644 --- a/tests/integration/fuzz/mod.rs +++ b/tests/integration/fuzz/mod.rs @@ -55,7 +55,10 @@ mod tests { #[test] pub fn rowid_seek_fuzz() { - let db = TempDatabase::new_with_rusqlite("CREATE TABLE t (x INTEGER PRIMARY KEY)", false); // INTEGER PRIMARY KEY is a rowid alias, so an index is not created + let db = TempDatabase::new_with_rusqlite( + "CREATE TABLE t (x INTEGER PRIMARY KEY autoincrement)", + false, + ); // INTEGER PRIMARY KEY is a rowid alias, so an index is not created let sqlite_conn = rusqlite::Connection::open(db.path.clone()).unwrap(); let (mut rng, _seed) = rng_from_time_or_env(); @@ -576,7 +579,7 @@ mod tests { // Track executed statements in case we fail let mut dml_statements = Vec::new(); let col_names = (0..num_cols) - .map(|i| format!("c{}", i)) + .map(|i| format!("c{i}")) .collect::>() .join(", "); let insert = format!(