Skip to content

Commit

Permalink
revert possible cost mismatch for reference insertion
Browse files Browse the repository at this point in the history
  • Loading branch information
fominok committed Dec 27, 2024
1 parent bcdfbbd commit 1078307
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 24 deletions.
106 changes: 82 additions & 24 deletions grovedb/src/operations/insert/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -81,6 +82,7 @@ impl GroveDb {
element,
options.unwrap_or_default(),
&merk_cache,
tx.as_ref(),
grove_version
)
);
Expand All @@ -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<MerkHandle<'db, 'c>, Error> {

Check warning on line 115 in grovedb/src/operations/insert/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

this function has too many arguments (8/7)

warning: this function has too many arguments (8/7) --> grovedb/src/operations/insert/mod.rs:104:5 | 104 | / fn add_element_on_transaction<'db, 'b, 'c, B: AsRef<[u8]>>( 105 | | &'db self, 106 | | path: SubtreePath<'b, B>, 107 | | key: &[u8], ... | 114 | | grove_version: &GroveVersion, 115 | | ) -> CostResult<MerkHandle<'db, 'c>, Error> { | |_______________________________________________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
check_grovedb_v0_with_cost!(
Expand Down Expand Up @@ -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,
)
})
);
}

Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions grovedb/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1078307

Please sign in to comment.