From fcfee24c5074fb92fc7947a915c1f60c265e2f0e Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Tue, 14 Jan 2025 16:06:42 +0200 Subject: [PATCH 1/2] Remove mark_last_insn_constant() from places where it is not safe to do so --- core/translate/expr.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/translate/expr.rs b/core/translate/expr.rs index d13902433..eadbc40e1 100644 --- a/core/translate/expr.rs +++ b/core/translate/expr.rs @@ -1651,7 +1651,6 @@ pub fn translate_expr( dest: target_register, }); } - program.mark_last_insn_constant(); Ok(target_register) } (UnaryOperator::Negative | UnaryOperator::Positive, _) => { @@ -1690,7 +1689,6 @@ pub fn translate_expr( dest: target_register, }); } - program.mark_last_insn_constant(); Ok(target_register) } (UnaryOperator::BitwiseNot, ast::Expr::Literal(ast::Literal::Null)) => { @@ -1698,7 +1696,6 @@ pub fn translate_expr( dest: target_register, dest_end: None, }); - program.mark_last_insn_constant(); Ok(target_register) } (UnaryOperator::BitwiseNot, _) => { From 3cbb2d2d7c5f43d2b245d5051d80f2ca5a91b4e3 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Tue, 14 Jan 2025 16:22:16 +0200 Subject: [PATCH 2/2] Add regression test for multi insert with unary operator --- test/src/lib.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/test/src/lib.rs b/test/src/lib.rs index 931c9b1bf..9aa9116d5 100644 --- a/test/src/lib.rs +++ b/test/src/lib.rs @@ -110,6 +110,73 @@ mod tests { Ok(()) } + #[test] + /// There was a regression with inserting multiple rows with a column containing an unary operator :) + /// https://github.com/tursodatabase/limbo/pull/679 + fn test_regression_multi_row_insert() -> anyhow::Result<()> { + let _ = env_logger::try_init(); + let tmp_db = TempDatabase::new("CREATE TABLE test (x REAL);"); + let conn = tmp_db.connect_limbo(); + + let insert_query = "INSERT INTO test VALUES (-2), (-3), (-1)"; + let list_query = "SELECT * FROM test"; + + match conn.query(insert_query) { + Ok(Some(ref mut rows)) => loop { + match rows.next_row()? { + StepResult::IO => { + tmp_db.io.run_once()?; + } + StepResult::Done => break, + _ => unreachable!(), + } + }, + Ok(None) => {} + Err(err) => { + eprintln!("{}", err); + } + }; + + do_flush(&conn, &tmp_db)?; + + let mut current_read_index = 1; + let expected_ids = vec![-3, -2, -1]; + let mut actual_ids = Vec::new(); + match conn.query(list_query) { + Ok(Some(ref mut rows)) => loop { + match rows.next_row()? { + StepResult::Row(row) => { + let first_value = row.values.first().expect("missing id"); + let id = match first_value { + Value::Float(f) => *f as i32, + _ => panic!("expected float"), + }; + actual_ids.push(id); + current_read_index += 1; + } + StepResult::IO => { + tmp_db.io.run_once()?; + } + StepResult::Interrupt => break, + StepResult::Done => break, + StepResult::Busy => { + panic!("Database is busy"); + } + } + }, + Ok(None) => {} + Err(err) => { + eprintln!("{}", err); + } + } + + assert_eq!(current_read_index, 4); // Verify we read all rows + // sort ids + actual_ids.sort(); + assert_eq!(actual_ids, expected_ids); + Ok(()) + } + #[test] fn test_simple_overflow_page() -> anyhow::Result<()> { let _ = env_logger::try_init();