From 107830715a616ea476da9de637f47a8268177840 Mon Sep 17 00:00:00 2001 From: Evgeny Fomin Date: Fri, 27 Dec 2024 16:38:46 +0100 Subject: [PATCH] revert possible cost mismatch for reference insertion --- grovedb/src/operations/insert/mod.rs | 106 +++++++++++++++++++++------ grovedb/src/tests/mod.rs | 2 + 2 files changed, 84 insertions(+), 24 deletions(-) diff --git a/grovedb/src/operations/insert/mod.rs b/grovedb/src/operations/insert/mod.rs index 0c5f03e6..6eb726ad 100644 --- a/grovedb/src/operations/insert/mod.rs +++ b/grovedb/src/operations/insert/mod.rs @@ -12,8 +12,9 @@ use grovedb_version::{check_grovedb_v0_with_cost, version::GroveVersion}; use crate::{ merk_cache::{MerkCache, MerkHandle}, - util::{self, TxRef}, - Element, Error, GroveDb, TransactionArg, + reference_path::path_from_reference_path_type, + util::TxRef, + Element, Error, GroveDb, Transaction, TransactionArg, }; #[derive(Clone)] /// Insert options @@ -81,6 +82,7 @@ impl GroveDb { element, options.unwrap_or_default(), &merk_cache, + tx.as_ref(), grove_version ) ); @@ -106,6 +108,9 @@ impl GroveDb { element: Element, options: InsertOptions, merk_cache: &'c MerkCache<'db, 'b, B>, + // TODO: only required for costs compatiblity for references insertion, shall be split + // using versioning + transaction: &Transaction, grove_version: &GroveVersion, ) -> CostResult, Error> { check_grovedb_v0_with_cost!( @@ -163,35 +168,41 @@ impl GroveDb { match element { Element::Reference(ref reference_path, ..) => { - let resolved_reference = cost_return_on_error!( + // TODO: this is an old version used to maintain the same costs as before, + // however, using reference resolution over merk cache may optimize the process + // and total cost of it, but those improvements with `util::follow_reference` + // shall go to the next version of `insert` function + let path = path.to_vec(); + let reference_path = cost_return_on_error!( &mut cost, - util::follow_reference( - merk_cache, - path.derive_owned(), - key, - reference_path.clone() + path_from_reference_path_type(reference_path.clone(), &path, Some(key)) + .wrap_with_cost(OperationCost::default()) + ); + + let referenced_item = cost_return_on_error!( + &mut cost, + self.follow_reference( + reference_path.as_slice().into(), + false, + transaction, + grove_version ) ); - if matches!( - resolved_reference.target_element, - Element::Tree(_, _) | Element::SumTree(_, _, _) - ) { - return Err(Error::NotSupported( - "References cannot point to subtrees".to_owned(), - )) - .wrap_with_cost(cost); - } + let referenced_element_value_hash = + cost_return_on_error!(&mut cost, referenced_item.value_hash(grove_version)); cost_return_on_error!( &mut cost, - subtree_to_insert_into.for_merk(|m| element.insert_reference( - m, - key, - resolved_reference.target_node_value_hash, - Some(options.as_merk_options()), - grove_version, - )) + subtree_to_insert_into.for_merk(|m| { + element.insert_reference( + m, + key, + referenced_element_value_hash, + Some(options.as_merk_options()), + grove_version, + ) + }) ); } @@ -393,6 +404,7 @@ mod tests { use crate::{ operations::insert::InsertOptions, + reference_path::ReferencePathType, tests::{common::EMPTY_PATH, make_empty_grovedb, make_test_grovedb, TEST_LEAF}, Element, Error, }; @@ -1940,6 +1952,52 @@ mod tests { ); } + #[test] + fn test_reference_insertion_cost() { + let grove_version = GroveVersion::latest(); + let db = make_empty_grovedb(); + let tx = db.start_transaction(); + + db.insert( + EMPTY_PATH, + b"key1", + Element::new_item(b"value".to_vec()), + None, + Some(&tx), + grove_version, + ) + .unwrap() + .unwrap(); + + let cost = db + .insert( + EMPTY_PATH, + b"key2", + Element::new_reference(ReferencePathType::AbsolutePathReference(vec![ + b"key1".to_vec() + ])), + None, + Some(&tx), + grove_version, + ) + .cost_as_result() + .expect("expected to insert"); + + assert_eq!( + cost, + OperationCost { + seek_count: 8, // todo: verify this + storage_cost: StorageCost { + added_bytes: 153, + replaced_bytes: 114, // todo: verify this + removed_bytes: NoStorageRemoval + }, + storage_loaded_bytes: 233, + hash_node_calls: 6, + } + ); + } + #[test] fn test_one_update_tree_bigger_cost_with_flags() { let grove_version = GroveVersion::latest(); diff --git a/grovedb/src/tests/mod.rs b/grovedb/src/tests/mod.rs index 126e3654..624b6abf 100644 --- a/grovedb/src/tests/mod.rs +++ b/grovedb/src/tests/mod.rs @@ -4143,7 +4143,9 @@ mod tests { } } +// TODO: shall go to the next version #[test] +#[ignore] fn subtrees_cant_be_referenced() { let db = make_deep_tree(&GroveVersion::latest()); assert!(db