diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 6fe8c655c..6c0683cb6 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -3589,12 +3589,15 @@ impl BTreeCursor { Some(rowid) => rowid, None => { self.state = CursorState::None; + // I don't think its necessary to invalidate here, but its safer to do so if I'm wrong + self.invalidate_record(); return Ok(CursorResult::Ok(())); } }; } else { if self.reusable_immutable_record.borrow().is_none() { self.state = CursorState::None; + // Record is None here so no need to invalidate return Ok(CursorResult::Ok(())); } } @@ -3778,6 +3781,7 @@ impl BTreeCursor { } else { self.stack.retreat(); self.state = CursorState::None; + self.invalidate_record(); return Ok(CursorResult::Ok(())); } // Only reaches this function call if state = DeleteState::WaitForBalancingToComplete @@ -3834,12 +3838,18 @@ impl BTreeCursor { return_if_io!(self.seek(key, SeekOp::EQ)); self.state = CursorState::None; + self.invalidate_record(); return Ok(CursorResult::Ok(())); } } } } + fn invalidate_record(&mut self) { + let mut record = self.get_immutable_record(); + record.as_mut().map(|record| record.invalidate()); + } + /// In outer joins, whenever the right-side table has no matching row, the query must still return a row /// for each left-side row. In order to achieve this, we set the null flag on the right-side table cursor /// so that it returns NULL for all columns until cleared. @@ -3863,21 +3873,24 @@ impl BTreeCursor { // Existing record found — compare prefix let existing_key = &record.get_values()[..record.count().saturating_sub(1)]; let inserted_key_vals = &key.get_values(); - if existing_key - .iter() - .zip(inserted_key_vals.iter()) - .all(|(a, b)| a == b) - { - return Ok(CursorResult::Ok(true)); // duplicate + // Need this check because .all returns True on an empty iterator, + // So when record_opt is invalidated, it would always indicate show up as a duplicate key + if existing_key.len() != inserted_key_vals.len() { + return Ok(CursorResult::Ok(false)); } + + Ok(CursorResult::Ok( + existing_key + .iter() + .zip(inserted_key_vals.iter()) + .all(|(a, b)| a == b), + )) } None => { // Cursor not pointing at a record — table is empty or past last - return Ok(CursorResult::Ok(false)); + Ok(CursorResult::Ok(false)) } } - - Ok(CursorResult::Ok(false)) // not a duplicate } pub fn exists(&mut self, key: &Value) -> Result> { diff --git a/testing/cli_tests/constraint.py b/testing/cli_tests/constraint.py index 36181fa67..d1244344b 100644 --- a/testing/cli_tests/constraint.py +++ b/testing/cli_tests/constraint.py @@ -347,6 +347,15 @@ def custom_test_2(limbo: TestLimboShell): ) +# Issue #1482 +def regression_test_update_single_key(limbo: TestLimboShell): + create = "CREATE TABLE t(a unique);" + first_insert = "INSERT INTO t VALUES (1);" + limbo.run_test("Create simple table with 1 unique value", create + first_insert, "") + update_single = "UPDATE t SET a=1 WHERE a=1;" + limbo.run_test("Update one single key to the same value", update_single, "") + + def all_tests() -> list[ConstraintTest]: tests: list[ConstraintTest] = [] max_cols = 10 @@ -390,15 +399,17 @@ def main(): cleanup(db_path) db_path = "testing/constraint.db" - try: - with TestLimboShell("") as limbo: - limbo.execute_dot(f".open {db_path}") - custom_test_2(limbo) - except Exception as e: - console.error(f"Test FAILED: {e}") + tests = [custom_test_2, regression_test_update_single_key] + for test in tests: + try: + with TestLimboShell("") as limbo: + limbo.execute_dot(f".open {db_path}") + test(limbo) + except Exception as e: + console.error(f"Test FAILED: {e}") + cleanup(db_path) + exit(1) cleanup(db_path) - exit(1) - cleanup(db_path) console.info("All tests passed successfully.")