From 5bf6fa78f0200ef8923ee0828ce2dfa11333dee3 Mon Sep 17 00:00:00 2001 From: michaelvlach Date: Sun, 13 Aug 2023 22:41:31 +0200 Subject: [PATCH 1/2] add insert or replace --- agdb/src/collections/map.rs | 10 +-- agdb/src/collections/multi_map.rs | 112 +++++++++++++------------- agdb/src/command.rs | 1 + agdb/src/db.rs | 34 ++++++++ agdb/src/query/insert_values_query.rs | 9 ++- agdb/tests/insert_values_test.rs | 59 ++++++++++++++ 6 files changed, 158 insertions(+), 67 deletions(-) diff --git a/agdb/src/collections/map.rs b/agdb/src/collections/map.rs index 600de068c..25a2abbac 100644 --- a/agdb/src/collections/map.rs +++ b/agdb/src/collections/map.rs @@ -340,15 +340,7 @@ where } pub fn insert(&mut self, storage: &mut S, key: &K, value: &T) -> Result, DbError> { - let old_value = self.value(key)?; - - if let Some(old) = &old_value { - self.multi_map.replace(storage, key, old, value)?; - } else { - self.multi_map.insert(storage, key, value)?; - } - - Ok(old_value) + self.multi_map.insert_replace(storage, key, |_| true, value) } #[allow(dead_code)] diff --git a/agdb/src/collections/multi_map.rs b/agdb/src/collections/multi_map.rs index 343f82f01..80e8fc713 100644 --- a/agdb/src/collections/multi_map.rs +++ b/agdb/src/collections/multi_map.rs @@ -114,13 +114,51 @@ where pub fn insert(&mut self, storage: &mut S, key: &K, value: &T) -> Result<(), DbError> { let id = self.data.transaction(storage); let index = self.free_index(storage, key)?; - self.data.set_state(storage, index, MapValueState::Valid)?; - self.data.set_key(storage, index, key)?; - self.data.set_value(storage, index, value)?; - self.data.set_len(storage, self.len() + 1)?; + self.do_insert(storage, index, key, value)?; self.data.commit(storage, id) } + pub fn insert_replace bool>( + &mut self, + storage: &mut S, + key: &K, + predicate: P, + new_value: &T, + ) -> Result, DbError> { + let id = self.data.transaction(storage); + + if self.len() >= self.max_len() { + self.rehash(storage, self.capacity() * 2)?; + } + + let hash = key.stable_hash(); + let mut pos = hash % self.capacity(); + let mut ret = None; + + loop { + match self.data.state(pos)? { + MapValueState::Empty => { + self.do_insert(storage, pos, key, new_value)?; + break; + } + MapValueState::Valid if self.data.key(pos)? == *key => { + let old_value = self.data.value(pos)?; + if predicate(&old_value) { + self.data.set_value(storage, pos, new_value)?; + ret = Some(old_value); + break; + } else { + pos = self.next_pos(pos) + } + } + MapValueState::Valid | MapValueState::Deleted => pos = self.next_pos(pos), + } + } + + self.data.commit(storage, id)?; + Ok(ret) + } + pub fn is_empty(&self) -> bool { self.len() == 0 } @@ -213,38 +251,6 @@ where self.data.commit(storage, id) } - pub fn replace( - &mut self, - storage: &mut S, - key: &K, - value: &T, - new_value: &T, - ) -> Result<(), DbError> { - if self.capacity() == 0 { - return Ok(()); - } - - let hash = key.stable_hash(); - let mut pos = hash % self.capacity(); - - let id = self.data.transaction(storage); - - loop { - match self.data.state(pos)? { - MapValueState::Empty => break, - MapValueState::Valid - if self.data.key(pos)? == *key && self.data.value(pos)? == *value => - { - self.data.set_value(storage, pos, new_value)?; - break; - } - MapValueState::Valid | MapValueState::Deleted => pos = self.next_pos(pos), - } - } - - self.data.commit(storage, id) - } - pub fn reserve(&mut self, storage: &mut S, capacity: u64) -> Result<(), DbError> { if self.capacity() < capacity { self.rehash(storage, capacity)?; @@ -322,6 +328,19 @@ where Ok(result) } + fn do_insert( + &mut self, + storage: &mut S, + index: u64, + key: &K, + value: &T, + ) -> Result<(), DbError> { + self.data.set_state(storage, index, MapValueState::Valid)?; + self.data.set_key(storage, index, key)?; + self.data.set_value(storage, index, value)?; + self.data.set_len(storage, self.len() + 1) + } + fn drop_value(&mut self, storage: &mut S, pos: u64) -> Result<(), DbError> { self.data.set_state(storage, pos, MapValueState::Deleted)?; self.data.set_key(storage, pos, &K::default())?; @@ -630,12 +649,7 @@ mod tests { let mut map = MultiMapStorage::::new(&mut storage).unwrap(); assert!(map - .replace( - &mut storage, - &10, - &"Hello".to_string(), - &"World".to_string() - ) + .insert_replace(&mut storage, &10, |v| v == "Hello", &"World".to_string()) .is_ok()); } @@ -647,12 +661,7 @@ mod tests { map.insert(&mut storage, &11, &"Hello".to_string()).unwrap(); assert!(map - .replace( - &mut storage, - &10, - &"Hello".to_string(), - &"World".to_string() - ) + .insert_replace(&mut storage, &10, |v| v == "Hello", &"World".to_string()) .is_ok()); } @@ -665,12 +674,7 @@ mod tests { map.remove_key(&mut storage, &10).unwrap(); assert!(map - .replace( - &mut storage, - &10, - &"Hello".to_string(), - &"World".to_string() - ) + .insert_replace(&mut storage, &10, |v| v == "Hello", &"World".to_string()) .is_ok()); } diff --git a/agdb/src/command.rs b/agdb/src/command.rs index e2dd185cf..f565ef532 100644 --- a/agdb/src/command.rs +++ b/agdb/src/command.rs @@ -11,4 +11,5 @@ pub(crate) enum Command { RemoveEdge { index: GraphIndex }, RemoveKeyValue { id: DbId, key_value: DbKeyValue }, RemoveNode { index: GraphIndex }, + ReplaceKeyValue { id: DbId, key_value: DbKeyValue }, } diff --git a/agdb/src/db.rs b/agdb/src/db.rs index 9d380ff5a..91b2274fb 100644 --- a/agdb/src/db.rs +++ b/agdb/src/db.rs @@ -310,6 +310,15 @@ impl Db { Command::RemoveNode { index } => { self.graph.remove_node(&mut self.storage, *index)? } + Command::ReplaceKeyValue { id, key_value } => { + self.values.insert_replace( + &mut self.storage, + id, + |v| v.key == key_value.key, + key_value, + )?; + return Ok(()); + } } } @@ -401,6 +410,31 @@ impl Db { Ok(()) } + pub(crate) fn insert_or_replace_key_value( + &mut self, + db_id: DbId, + key_value: &DbKeyValue, + ) -> Result<(), QueryError> { + if let Some(old) = self.values.insert_replace( + &mut self.storage, + &db_id, + |kv| kv.key == key_value.key, + key_value, + )? { + self.undo_stack.push(Command::ReplaceKeyValue { + id: db_id, + key_value: old, + }); + } else { + self.undo_stack.push(Command::RemoveKeyValue { + id: db_id, + key_value: key_value.clone(), + }); + } + + Ok(()) + } + pub(crate) fn keys(&self, db_id: DbId) -> Result, DbError> { Ok(self .values diff --git a/agdb/src/query/insert_values_query.rs b/agdb/src/query/insert_values_query.rs index b72da37da..c9fde3d1e 100644 --- a/agdb/src/query/insert_values_query.rs +++ b/agdb/src/query/insert_values_query.rs @@ -31,8 +31,9 @@ impl QueryMut for InsertValuesQuery { QueryValues::Single(values) => { for id in ids { let db_id = db.db_id(id)?; + for key_value in values { - db.insert_key_value(db_id, key_value)?; + db.insert_or_replace_key_value(db_id, key_value)?; result.result += 1; } } @@ -45,7 +46,7 @@ impl QueryMut for InsertValuesQuery { for (id, values) in ids.iter().zip(values) { let db_id = db.db_id(id)?; for key_value in values { - db.insert_key_value(db_id, key_value)?; + db.insert_or_replace_key_value(db_id, key_value)?; result.result += 1; } } @@ -58,7 +59,7 @@ impl QueryMut for InsertValuesQuery { QueryValues::Single(values) => { for db_id in db_ids { for key_value in values { - db.insert_key_value(db_id, key_value)?; + db.insert_or_replace_key_value(db_id, key_value)?; result.result += 1; } } @@ -70,7 +71,7 @@ impl QueryMut for InsertValuesQuery { for (db_id, values) in db_ids.iter().zip(values) { for key_value in values { - db.insert_key_value(*db_id, key_value)?; + db.insert_or_replace_key_value(*db_id, key_value)?; result.result += 1; } } diff --git a/agdb/tests/insert_values_test.rs b/agdb/tests/insert_values_test.rs index ac6ab5d0f..b1b8fe9fb 100644 --- a/agdb/tests/insert_values_test.rs +++ b/agdb/tests/insert_values_test.rs @@ -237,3 +237,62 @@ fn insert_values_search_invalid_length() { "Ids and values length do not match", ); } + +#[test] +fn insert_values_overwrite() { + let mut db = TestDb::new(); + db.exec_mut( + QueryBuilder::insert() + .nodes() + .values(vec![vec![("key", 10).into()]]) + .query(), + 1, + ); + db.exec_mut( + QueryBuilder::insert() + .values_uniform(vec![("key", 20).into(), ("key2", 30).into()]) + .ids(1) + .query(), + 2, + ); + db.exec_elements( + QueryBuilder::select().ids(1).query(), + &[DbElement { + id: DbId(1), + values: vec![("key", 20).into(), ("key2", 30).into()], + }], + ) +} + +#[test] +fn insert_values_overwrite_transaction() { + let mut db = TestDb::new(); + db.exec_mut( + QueryBuilder::insert() + .nodes() + .values(vec![vec![("key", 10).into()]]) + .query(), + 1, + ); + + db.transaction_mut_error( + |t| -> Result<(), QueryError> { + t.exec_mut( + &QueryBuilder::insert() + .values_uniform(vec![("key", 20).into(), ("key2", 30).into()]) + .ids(1) + .query(), + )?; + Err(QueryError::from("error")) + }, + QueryError::from("error"), + ); + + db.exec_elements( + QueryBuilder::select().ids(1).query(), + &[DbElement { + id: DbId(1), + values: vec![("key", 10).into()], + }], + ) +} From 89822fe0df3672c4958ad98039fd11ffa77b6a72 Mon Sep 17 00:00:00 2001 From: michaelvlach Date: Sun, 13 Aug 2023 23:07:01 +0200 Subject: [PATCH 2/2] fix coverage --- agdb/src/collections/map.rs | 3 ++- agdb/src/collections/multi_map.rs | 16 ++++++++++------ agdb/src/db.rs | 4 ++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/agdb/src/collections/map.rs b/agdb/src/collections/map.rs index 25a2abbac..fe8323c8b 100644 --- a/agdb/src/collections/map.rs +++ b/agdb/src/collections/map.rs @@ -340,7 +340,8 @@ where } pub fn insert(&mut self, storage: &mut S, key: &K, value: &T) -> Result, DbError> { - self.multi_map.insert_replace(storage, key, |_| true, value) + self.multi_map + .insert_or_replace(storage, key, |_| true, value) } #[allow(dead_code)] diff --git a/agdb/src/collections/multi_map.rs b/agdb/src/collections/multi_map.rs index 80e8fc713..c8ff4773b 100644 --- a/agdb/src/collections/multi_map.rs +++ b/agdb/src/collections/multi_map.rs @@ -118,7 +118,7 @@ where self.data.commit(storage, id) } - pub fn insert_replace bool>( + pub fn insert_or_replace bool>( &mut self, storage: &mut S, key: &K, @@ -647,10 +647,11 @@ mod tests { let test_file = TestFile::new(); let mut storage = FileStorage::new(test_file.file_name()).unwrap(); let mut map = MultiMapStorage::::new(&mut storage).unwrap(); - + let p = |v: &String| v == "Hello"; assert!(map - .insert_replace(&mut storage, &10, |v| v == "Hello", &"World".to_string()) + .insert_or_replace(&mut storage, &10, p, &"World".to_string()) .is_ok()); + p(&"".to_string()); } #[test] @@ -658,10 +659,11 @@ mod tests { let test_file = TestFile::new(); let mut storage = FileStorage::new(test_file.file_name()).unwrap(); let mut map = MultiMapStorage::::new(&mut storage).unwrap(); + map.insert(&mut storage, &10, &"World".to_string()).unwrap(); map.insert(&mut storage, &11, &"Hello".to_string()).unwrap(); assert!(map - .insert_replace(&mut storage, &10, |v| v == "Hello", &"World".to_string()) + .insert_or_replace(&mut storage, &10, |v| v == "Hello", &"World".to_string()) .is_ok()); } @@ -671,10 +673,12 @@ mod tests { let mut storage = FileStorage::new(test_file.file_name()).unwrap(); let mut map = MultiMapStorage::::new(&mut storage).unwrap(); map.insert(&mut storage, &10, &"Hello".to_string()).unwrap(); - map.remove_key(&mut storage, &10).unwrap(); + map.insert(&mut storage, &10, &"World".to_string()).unwrap(); + map.remove_value(&mut storage, &10, &"Hello".to_string()) + .unwrap(); assert!(map - .insert_replace(&mut storage, &10, |v| v == "Hello", &"World".to_string()) + .insert_or_replace(&mut storage, &10, |v| v == "Hello", &"World".to_string()) .is_ok()); } diff --git a/agdb/src/db.rs b/agdb/src/db.rs index 91b2274fb..92e01d11c 100644 --- a/agdb/src/db.rs +++ b/agdb/src/db.rs @@ -311,7 +311,7 @@ impl Db { self.graph.remove_node(&mut self.storage, *index)? } Command::ReplaceKeyValue { id, key_value } => { - self.values.insert_replace( + self.values.insert_or_replace( &mut self.storage, id, |v| v.key == key_value.key, @@ -415,7 +415,7 @@ impl Db { db_id: DbId, key_value: &DbKeyValue, ) -> Result<(), QueryError> { - if let Some(old) = self.values.insert_replace( + if let Some(old) = self.values.insert_or_replace( &mut self.storage, &db_id, |kv| kv.key == key_value.key,