From 5ee44c4c81180903628e0a65b21842b0775258e5 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 9 Sep 2026 08:04:16 +0700 Subject: [PATCH 1/5] feat(batch): typed backward-references deletes; rename flag to propagate_backward_references_when_unsure Rename the `propagate_backward_references` field of `InsertOptions`, `DeleteOptions` and `BatchApplyOptions` to `propagate_backward_references_when_unsure` (semantics unchanged). The name says what the flag buys: when the caller does not know whether the element it displaces carries backward references, GroveDB reads it and finds out. Callers that do know can now say so per batch op: - `GroveOp::DeleteWithCascade` (sort tag 21) always reads the deleted element and runs the flagged delete's bookkeeping: every referrer chain cascades (consent via `cascade_on_update` required) and a deleted `BidirectionalReference` is de-registered from its target. - `GroveOp::DeleteWithNoBackwardsReferenceCheck` (sort tag 22) never reads it; registered references are left dangling, exactly as an unflagged live delete leaves them. Plain `Delete` keeps following the batch flag. The backward-references preprocessor gains a `flag_on` parameter: an unflagged batch carrying a cascade op runs in per-op mode, where only the cascade ops are read and planned, the other ops pay nothing extra, and only their certain effects are staged so the cascade resolves against the batch's outcome. The M4 conflict rules apply unchanged. Estimators charge the fan-out per op rather than per flag. Both ops require GROVE_V4 full batches: pre-V4 versions, partial batches and partial-batch add-on ops refuse them with `NotSupported` instead of silently degrading to a plain delete; `apply_operations_without_batching` gets the same gate. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 25 +- adr/bidirectional_references.md | 30 +- docs/book/src/batch-operations.md | 13 + docs/crates/grovedb.md | 2 + .../src/version/grovedb_versions.rs | 2 +- grovedb-version/src/version/v4.rs | 2 +- grovedb/src/batch/backward_references.rs | 143 ++++-- grovedb/src/batch/batch_structure.rs | 13 +- .../estimated_costs/average_case_costs.rs | 36 +- grovedb/src/batch/estimated_costs/mod.rs | 2 +- .../batch/estimated_costs/worst_case_costs.rs | 36 +- grovedb/src/batch/indexed_tree/pre_state.rs | 2 + .../src/batch/initial_segment_footprint.rs | 10 +- grovedb/src/batch/mod.rs | 226 +++++++-- grovedb/src/batch/options.rs | 25 +- grovedb/src/debugger.rs | 2 +- .../delete_internal_on_transaction/mod.rs | 2 +- .../delete_internal_on_transaction/v2.rs | 20 +- .../src/operations/delete/delete_up_tree.rs | 2 +- grovedb/src/operations/delete/mod.rs | 22 +- .../insert/add_element_on_transaction/v1.rs | 2 +- .../insert/add_element_on_transaction/v2.rs | 2 +- .../insert/insert_on_transaction/mod.rs | 2 +- .../insert/insert_on_transaction/v1.rs | 8 +- grovedb/src/operations/insert/mod.rs | 14 +- .../batch_backward_references_cost_tests.rs | 89 +++- .../tests/batch_backward_references_tests.rs | 473 +++++++++++++++++- .../tests/bidirectional_references_tests.rs | 30 +- .../src/tests/delete_indexed_tree_tests.rs | 2 +- .../src/tests/direct_insert_indexed_tests.rs | 2 +- grovedb/src/tests/mod.rs | 8 +- .../src/tests/operations_coverage_tests.rs | 10 +- .../tests/ordinary_replacement_cost_tests.rs | 2 +- .../provable_count_indexed_tree_tests.rs | 6 +- ...e_count_provable_sum_indexed_tree_tests.rs | 6 +- .../tests/provable_sum_indexed_tree_tests.rs | 4 +- .../src/tests/verify_grovedb_indexed_tests.rs | 2 +- 37 files changed, 1092 insertions(+), 185 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c3c898b6..5870fb44b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 updating a referenced element propagates the new hash along every chain, and deleting/overwriting it cascades the chains away (each affected reference must opt in via `cascade_on_update`). Opt-in per call through - the new `propagate_backward_references` flag on `InsertOptions` / + the new `propagate_backward_references_when_unsure` flag on `InsertOptions` / `DeleteOptions`. The referrer list is stored on the element itself under a two-layer hash (`combine(inner, backrefs)`), so registering a referrer never re-hashes what existing referrers committed to; public reads return @@ -26,7 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 verifier recomputes. Requires `GROVE_V4`; earlier versions, V0 proofs, and `Provable*` aggregate parents reject the new variants (fail closed). `apply_batch` supports the whole family when the batch opts in via - `BatchApplyOptions::propagate_backward_references`: a preprocessing pass + `BatchApplyOptions::propagate_backward_references_when_unsure`: a preprocessing pass expands the batch into the derived registration/propagation/cascade operations the live flagged flow performs (shared semantic core, so batch and non-batch execution produce byte-identical root hashes), including @@ -44,6 +44,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 displace, ≤10-hop chains, 1 referrer per reference) while pre-V4 estimation stays byte-stable for replay. See `adr/bidirectional_references.md`. +- Per-op backward-references deletes for batches: `GroveOp::DeleteWithCascade` + reads the deleted element and cascades away every bidirectional reference + registered on it (each affected reference must allow `cascade_on_update`; a + deleted `BidirectionalReference` is de-registered from its target), and + `GroveOp::DeleteWithNoBackwardsReferenceCheck` deletes without the read, + leaving any registered references dangling — each whatever the batch's + `propagate_backward_references_when_unsure` says, which plain `Delete` keeps + following. An unflagged batch carrying a cascading delete runs the + backward-references expansion in per-op mode: the other ops are not read + and pay nothing extra. Constructors `delete_with_cascade_op` / + `delete_with_no_backwards_reference_check_op` (plus `_estimated_op` + twins); sort tags 21 / 22; the estimators charge the fan-out per op rather + than per flag. `GROVE_V4`+ full batches only: pre-V4 and partial batches + refuse both with `NotSupported`. - **BREAKING**: Added `add_parent_tree_on_subquery` feature to PathQuery (#379) - New field in `Query` struct: `add_parent_tree_on_subquery: bool` - When set to `true`, parent tree elements (like CountTree or SumTree) are included in query results when performing subqueries @@ -52,6 +66,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Updated proof verification logic to handle parent tree inclusion ### Changed +- **BREAKING**: renamed the `propagate_backward_references` field of + `InsertOptions`, `DeleteOptions` and `BatchApplyOptions` to + `propagate_backward_references_when_unsure`. The name says what the flag + buys: when the caller does not know whether the element it displaces + carries backward references, GroveDB reads it and finds out (and + propagates or cascades accordingly). Callers that do know say so per op + through the typed batch deletes above. Semantics are unchanged. - Bumped the GroveDB workspace crates and their internal dependency requirements to **6.0.0** for the public API changes since 5.0.1. This package version is independent of the existing `GroveVersion` runtime compatibility versions. diff --git a/adr/bidirectional_references.md b/adr/bidirectional_references.md index 5cb73020b..9bc9f869b 100644 --- a/adr/bidirectional_references.md +++ b/adr/bidirectional_references.md @@ -51,10 +51,13 @@ chain origin. When such behavior is required, a different type of element should Moreover, these types are incompatible, which will be discussed in the "Rules" section. Additionally, a new flag was added to `InsertOptions` and `DeleteOptions` -called `propagate_backward_references` (`ClearOptions` support is deferred — +called `propagate_backward_references_when_unsure` (`ClearOptions` support is deferred — see the limitations below). Since propagation incurs a cost, starting with the checks required to determine whether it should be performed, bidirectional references are -optional and must be explicitly enabled. +optional and must be explicitly enabled. The name says what the flag buys: when the +caller does not know whether the element it displaces carries backward references, +GroveDB reads it and finds out. A caller that does know can say so per batch op +instead — see the typed deletes under Batching below. Even when a user inserts something unrelated to the bidirectional references feature, a check must still be performed to determine whether the insertion overwrites an item @@ -78,7 +81,7 @@ Current limitations (fail closed, lift as needed): - The four variants may not be wrapped in the aggregation wrappers (`NonCounted` / `NotSummed` / `NotCountedOrSummed`). - `apply_batch` supports the family when the batch opts in via - `BatchApplyOptions::propagate_backward_references` (see the batching + `BatchApplyOptions::propagate_backward_references_when_unsure` (see the batching section under Implementation); batches without the flag — and partial batches, which have no expansion support — reject ops carrying the family. A flagged batch also refuses to delete a NON-EMPTY subtree: @@ -93,7 +96,7 @@ Current limitations (fail closed, lift as needed): propagations and cascades skip it and lazily clear its slot — but `verify_grovedb` reports the affected references until the chain is rewritten through flagged operations. -- `clear_subtree` has no `propagate_backward_references` option yet; use +- `clear_subtree` has no `propagate_backward_references_when_unsure` option yet; use `delete` with the flag for cascade-aware removal. - Under the flag, insert supports items, references, and empty plain-Merk trees; delete supports plain Merk subtrees. The specialized data trees @@ -106,7 +109,7 @@ Current limitations (fail closed, lift as needed): Next, we’ll go over the rules and limitations for using bidirectional references. -Note that for the rules to apply, the `propagate_backward_references` flag needs to be +Note that for the rules to apply, the `propagate_backward_references_when_unsure` flag needs to be set. An 'Element with backward references' refers to `ItemWithBackwardsReferences`, @@ -165,7 +168,7 @@ preventing the operation from completing successfully. ### Batching `apply_batch` supports the whole family when the batch sets -`BatchApplyOptions::propagate_backward_references` (GROVE_V4+, riding the +`BatchApplyOptions::propagate_backward_references_when_unsure` (GROVE_V4+, riding the same activation as the live flagged flow). A preprocessing pass (`batch::backward_references`) expands the user's operations into the derived operations the live flow would perform, planned by the SAME @@ -194,6 +197,21 @@ same batch that deletes its target; a cascade deleting a position another op touches; a propagation rewrite hitting a user delete; and `RefreshReference` on a position holding a bidirectional reference. +Two typed deletes pin the decision per op, whatever the batch flag says +(GROVE_V4+, full batches only): `GroveOp::DeleteWithCascade` reads the +element and runs exactly the flagged delete's bookkeeping (cascade of every +referrer chain, consent required, de-registration of a deleted reference), +and `GroveOp::DeleteWithNoBackwardsReferenceCheck` deletes without the read, +leaving whatever was registered on the element dangling — exactly the +unflagged delete. Plain `Delete` keeps following the flag. An unflagged +batch carrying a cascading delete runs the expansion in per-op mode: only +the cascading deletes are read and planned, the other ops stay ordinary +unflagged ops (no read, no bookkeeping) and only their certain effects are +staged into the overlay, so the cascade resolves against the batch's +outcome and the M4 conflict rules apply unchanged. Partial batches and +pre-V4 versions refuse both typed ops with `NotSupported` rather than +silently degrading them to a plain delete. + Estimated costs (average and worst case) model the derived fan-out on GROVE_V4+ under the batch flag, bounded by the budgets above (a written item's DECLARED referrer capacity, the 256 ceiling for writes that cannot diff --git a/docs/book/src/batch-operations.md b/docs/book/src/batch-operations.md index b33327f89..2a97adfe5 100644 --- a/docs/book/src/batch-operations.md +++ b/docs/book/src/batch-operations.md @@ -13,6 +13,8 @@ pub enum GroveOp { Patch { element: Element, change_in_bytes: i32 }, RefreshReference { reference_path_type, max_reference_hop, flags, trust_refresh_reference }, Delete, + DeleteWithCascade, // Read + cascade bidirectional references, whatever the batch flag says (GROVE_V4+) + DeleteWithNoBackwardsReferenceCheck, // Never read for backward references, whatever the batch flag says (GROVE_V4+) DeleteTree(TreeType, SubelementsDeletionBehavior), // Per-op deletion policy // Non-Merk tree append operations (user-facing): @@ -40,6 +42,17 @@ pub enum NonMerkTreeMeta { } ``` +**Typed deletes.** A plain `Delete` is read for backward-references +bookkeeping only when the batch sets +`BatchApplyOptions::propagate_backward_references_when_unsure`. The two +typed variants pin that decision for one op: `DeleteWithCascade` always +reads the element and cascades away every bidirectional reference registered +on it (each must allow `cascade_on_update`, and a deleted reference is +de-registered from its target), while `DeleteWithNoBackwardsReferenceCheck` +never reads it and leaves registered references dangling. Both require +`GROVE_V4`+ and a full (non-partial) batch. See +`adr/bidirectional_references.md`. + **SubelementsDeletionBehavior** controls how a `DeleteTree` handles non-empty subtrees: ```rust diff --git a/docs/crates/grovedb.md b/docs/crates/grovedb.md index 1251fcda0..40f748d9f 100644 --- a/docs/crates/grovedb.md +++ b/docs/crates/grovedb.md @@ -196,6 +196,8 @@ pub enum GroveDbOp { InsertOrReplace { element: Element }, Replace { element: Element }, Delete, + DeleteWithCascade, // GROVE_V4+: read + cascade bidirectional references, whatever the batch flag says + DeleteWithNoBackwardsReferenceCheck, // GROVE_V4+: never read for backward references DeleteTree, DeleteUpTree { stop_path_height: Option }, TransientInsertTreeWithRootHash { hash: [u8; 32], .. }, diff --git a/grovedb-version/src/version/grovedb_versions.rs b/grovedb-version/src/version/grovedb_versions.rs index c34dea9c9..cee560773 100644 --- a/grovedb-version/src/version/grovedb_versions.rs +++ b/grovedb-version/src/version/grovedb_versions.rs @@ -537,7 +537,7 @@ pub struct GroveDBOperationsAverageCaseVersions { /// family in batches, so historical admission decisions replay /// byte-identically. /// - `1` (V4+): family-carrying ops and (under - /// `BatchApplyOptions::propagate_backward_references`) deletes charge + /// `BatchApplyOptions::propagate_backward_references_when_unsure`) deletes charge /// the derived registration / propagation / cascade fan-out, bounded /// by the apply path's budgets (≤32 referrers per item, ≤10-hop /// chains, 1 referrer per reference), and the derived op itself gets diff --git a/grovedb-version/src/version/v4.rs b/grovedb-version/src/version/v4.rs index a38838bb5..f2bb6196c 100644 --- a/grovedb-version/src/version/v4.rs +++ b/grovedb-version/src/version/v4.rs @@ -433,7 +433,7 @@ pub const GROVE_V4: GroveVersion = GroveVersion { insert: 0, // v1: backward-references router. Calls that neither insert a // BidirectionalReference nor set - // propagate_backward_references run the exact v0 body. + // propagate_backward_references_when_unsure run the exact v0 body. insert_on_transaction: 1, // v2: a directly inserted Reference binds the value hash of its // terminal's STORED bytes (wrapper included for a NonCounted diff --git a/grovedb/src/batch/backward_references.rs b/grovedb/src/batch/backward_references.rs index fa5008ef4..632af4e70 100644 --- a/grovedb/src/batch/backward_references.rs +++ b/grovedb/src/batch/backward_references.rs @@ -1,6 +1,6 @@ //! The backward-references batch preprocessor (batching milestones M2–M4). //! -//! When [`super::BatchApplyOptions::propagate_backward_references`] is set, +//! When [`super::BatchApplyOptions::propagate_backward_references_when_unsure`] is set, //! user operations touching the backward-references family — the three ITEM //! variants and `BidirectionalReference` itself — expand into the derived //! operations the live flagged flow would perform. The decisions come from @@ -28,6 +28,19 @@ //! Planners read through [`OverlayChainStore`]: staged pending state first, //! the transaction's pre-batch DB state otherwise. //! +//! # Per-op mode +//! +//! `GroveOp::DeleteWithCascade` forces the bookkeeping for one delete, and +//! `GroveOp::DeleteWithNoBackwardsReferenceCheck` forbids it, whatever the +//! batch flag says. An unflagged batch carrying a cascading delete runs +//! this pass in per-op mode: only the cascading deletes are read and +//! planned; every other op stays an ordinary unflagged op (no read, no +//! bookkeeping) and only its certain effect is staged, so the cascade +//! resolves against the batch's outcome. A no-check delete in a flagged +//! batch is likewise not read: its position is staged as gone and whatever +//! was registered on it dangles, exactly as an unflagged live delete would +//! leave it. +//! //! Derived writes carry their final node value hash (the two-layer //! combine), computed here exactly as the live applier computes it, and //! execute through [`super::GroveOp::ReplaceBackwardReferenceFamilyMember`]. @@ -600,10 +613,18 @@ impl<'db, 'g> Expansion<'db, 'g> { /// Expand `ops` with the derived operations the backward-references rules /// require, per the module documentation. +/// +/// `flag_on` is the batch's `propagate_backward_references_when_unsure`. +/// With it set every op gets the bookkeeping; without it the pass runs in +/// per-op mode for the `DeleteWithCascade` ops the batch carries — the +/// other ops are not read (they pay nothing extra) and only their certain +/// effects are staged into the overlay, so a cascade resolves against the +/// batch's outcome. pub(super) fn expand_backward_references_ops( db: &GroveDb, tx: &TxRef<'_, '_>, ops: Vec, + flag_on: bool, validate_insertion_does_not_override: bool, grove_version: &GroveVersion, ) -> CostResult, Error> { @@ -633,7 +654,13 @@ pub(super) fn expand_backward_references_ops( )) .wrap_with_cost(cost); } - if matches!(op.op, GroveOp::Delete | GroveOp::DeleteTree(..)) { + if matches!( + op.op, + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + | GroveOp::DeleteTree(..) + ) { expansion.user_deleted_positions.insert(position); } } @@ -646,37 +673,44 @@ pub(super) fn expand_backward_references_ops( // first, so a nested new tree sees its parent already marked — because // the batch is unordered: an op under such a subtree may appear before // the op creating it, and its previous-state read must not touch - // committed storage (the parent does not exist there). - let mut tree_write_positions: Vec = expansion - .ops - .iter() - .flatten() - .filter_map(|op| match &op.op { - GroveOp::InsertOrReplace { element } - | GroveOp::Replace { element } - | GroveOp::Patch { element, .. } - | GroveOp::InsertIfNotExists { element, .. } - | GroveOp::InsertWithKnownToNotAlreadyExist { element } - if element.is_any_tree() => - { - Expansion::op_position(op) + // committed storage (the parent does not exist there). Flagged batches + // only: the scan reads every tree-writing op's previous state, which + // an unflagged batch must not pay for. (In per-op mode a + // `DeleteWithCascade` under a subtree the same batch creates has + // nothing to delete; its previous-state read fails the batch, which is + // the fail-closed outcome.) + if flag_on { + let mut tree_write_positions: Vec = expansion + .ops + .iter() + .flatten() + .filter_map(|op| match &op.op { + GroveOp::InsertOrReplace { element } + | GroveOp::Replace { element } + | GroveOp::Patch { element, .. } + | GroveOp::InsertIfNotExists { element, .. } + | GroveOp::InsertWithKnownToNotAlreadyExist { element } + if element.is_any_tree() => + { + Expansion::op_position(op) + } + _ => None, + }) + .collect(); + tree_write_positions.sort_by_key(|(path, _)| path.len()); + for (path, key) in tree_write_positions { + let previous_is_tree = if expansion.store.under_fresh_subtree(&path) { + false + } else { + cost_return_on_error!(&mut cost, expansion.store.element_at(&path, &key)) + .map(|p| p.is_any_tree()) + .unwrap_or(false) + }; + if !previous_is_tree { + let mut qualified = path; + qualified.push(key); + expansion.store.stage_fresh_subtree(qualified); } - _ => None, - }) - .collect(); - tree_write_positions.sort_by_key(|(path, _)| path.len()); - for (path, key) in tree_write_positions { - let previous_is_tree = if expansion.store.under_fresh_subtree(&path) { - false - } else { - cost_return_on_error!(&mut cost, expansion.store.element_at(&path, &key)) - .map(|p| p.is_any_tree()) - .unwrap_or(false) - }; - if !previous_is_tree { - let mut qualified = path; - qualified.push(key); - expansion.store.stage_fresh_subtree(qualified); } } @@ -702,6 +736,18 @@ pub(super) fn expand_backward_references_ops( | GroveOp::Patch { element, .. } | GroveOp::InsertIfNotExists { element, .. } | GroveOp::InsertWithKnownToNotAlreadyExist { element } => { + if !flag_on { + // Per-op mode: writes carry no bookkeeping (family + // payloads were rejected upstream) and are not read. + // Stage the ones that certainly land so a cascade + // resolving a referrer here sees the batch's outcome; + // a conditional insert may write nothing, and its + // stored state stays authoritative. + if !matches!(op_kind, GroveOp::InsertIfNotExists { .. }) { + expansion.store.stage(position, Some(element.clone())); + } + continue; + } if let Element::BidirectionalReference(reference, _) = element { // A conditional insert whose gate will SKIP it must not // advertise a pending edge: `InsertIfNotExists` over an @@ -852,7 +898,24 @@ pub(super) fn expand_backward_references_ops( expansion.store.stage(position, Some(element)); } } - GroveOp::Delete | GroveOp::DeleteTree(..) => { + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + | GroveOp::DeleteTree(..) => { + let check = match op_kind { + GroveOp::DeleteWithCascade => true, + GroveOp::DeleteWithNoBackwardsReferenceCheck => false, + _ => flag_on, + }; + if !check { + // No read, no bookkeeping: the position simply goes + // away. Staged so a cascade resolving a referrer here + // sees it gone (and skips it as dangling); whatever was + // registered on the deleted element is left dangling — + // the batch, or this op, opted out of the check. + expansion.store.stage(position, None); + continue; + } let previous = cost_return_on_error!(&mut cost, expansion.store.element_at(&path, &key)); // Deleting a NON-EMPTY subtree is refused under the flag: @@ -871,10 +934,10 @@ pub(super) fn expand_backward_references_ops( ); if non_empty { return Err(Error::NotSupported( - "deleting a non-empty subtree in a batch with \ - propagate_backward_references is not supported; delete it through \ - the live flagged flow (which cascades descendants) or empty it \ - first" + "deleting a non-empty subtree with backward-references bookkeeping \ + (a batch with propagate_backward_references_when_unsure, or a \ + DeleteWithCascade) is not supported; delete it through the live \ + flagged flow (which cascades descendants) or empty it first" .to_owned(), )) .wrap_with_cost(cost); @@ -904,6 +967,12 @@ pub(super) fn expand_backward_references_ops( flags, .. } => { + if !flag_on { + // Per-op mode: a refresh is an ordinary unflagged write + // (not read, not staged — the rebuilt shape needs the + // stored one). + continue; + } let previous = cost_return_on_error!(&mut cost, expansion.store.element_at(&path, &key)); if matches!(previous, Some(Element::BidirectionalReference(..))) { diff --git a/grovedb/src/batch/batch_structure.rs b/grovedb/src/batch/batch_structure.rs index 9ea066a61..39ffeead1 100644 --- a/grovedb/src/batch/batch_structure.rs +++ b/grovedb/src/batch/batch_structure.rs @@ -280,9 +280,11 @@ where } Ok(()) } - GroveOp::RefreshReference { .. } | GroveOp::Delete | GroveOp::DeleteTree(..) => { - Ok(()) - } + GroveOp::RefreshReference { .. } + | GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + | GroveOp::DeleteTree(..) => Ok(()), GroveOp::CommitmentTreeInsert { .. } | GroveOp::MmrTreeAppend { .. } | GroveOp::BulkAppend { .. } @@ -463,7 +465,10 @@ pub(super) fn merge_add_on_op_over_pending( axes, ) } - GroveOp::Delete | GroveOp::DeleteTree(..) => { + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + | GroveOp::DeleteTree(..) => { if root_key.is_some() { Err(Error::InvalidBatchOperation( "modification of tree when it will be deleted", diff --git a/grovedb/src/batch/estimated_costs/average_case_costs.rs b/grovedb/src/batch/estimated_costs/average_case_costs.rs index 6825e8047..90a31f040 100644 --- a/grovedb/src/batch/estimated_costs/average_case_costs.rs +++ b/grovedb/src/batch/estimated_costs/average_case_costs.rs @@ -64,7 +64,7 @@ impl GroveOp { // not carry. Ignored by every other op type. append_tree_chunk_power: Option, // Whether the batch opts into backward-references bookkeeping - // (`BatchApplyOptions::propagate_backward_references`): family ops + // (`BatchApplyOptions::propagate_backward_references_when_unsure`): family ops // and deletes then charge the derived fan-out on GROVE_V4+. backward_references_enabled: bool, propagate: bool, @@ -144,9 +144,10 @@ impl GroveOp { // The flagged apply path probes a deleted tree's child subtree for // emptiness (a merk open and its root read) before admitting the // deletion — charged whenever the fan-out is active. - let flagged_delete_probe = || { + let fan_out_active = backward_references_enabled && fan_out_version != 0; + let delete_probe = |active: bool| { let mut probe = OperationCost::default(); - if backward_references_enabled && fan_out_version != 0 { + if active { let key_width = GroveDb::average_case_layer_key_size( &layer_element_estimates.estimated_layer_sizes, ); @@ -347,7 +348,7 @@ impl GroveOp { ), backward_references_fan_out(None), ) - .add_cost(flagged_delete_probe()), + .add_cost(delete_probe(fan_out_active)), GroveOp::DeleteTree(tree_type, _) => with_fan_out( GroveDb::average_case_merk_delete_tree( key, @@ -358,7 +359,30 @@ impl GroveOp { ), backward_references_fan_out(None), ) - .add_cost(flagged_delete_probe()), + .add_cost(delete_probe(fan_out_active)), + // Forces the bookkeeping whatever the batch flag says: the + // displaced-state fan-out and the probe are charged whenever the + // version models them. + GroveOp::DeleteWithCascade => with_fan_out( + GroveDb::average_case_merk_delete_element( + key, + layer_element_estimates, + propagate, + grove_version, + ), + (fan_out_version != 0).then(super::BackwardReferencesFanOut::average_item), + ) + .add_cost(delete_probe(fan_out_version != 0)), + // Opts out of the bookkeeping whatever the batch flag says: the + // plain delete model, nothing derived. + GroveOp::DeleteWithNoBackwardsReferenceCheck => { + GroveDb::average_case_merk_delete_element( + key, + layer_element_estimates, + propagate, + grove_version, + ) + } GroveOp::CommitmentTreeInsert { payload, .. } => { Self::average_case_commitment_tree_insert( payload, @@ -1135,7 +1159,7 @@ impl TreeCache for AverageCaseTreeCacheKnownPaths { &key, layer_element_estimates, append_tree_chunk_power, - batch_apply_options.propagate_backward_references, + batch_apply_options.propagate_backward_references_when_unsure, false, grove_version ) diff --git a/grovedb/src/batch/estimated_costs/mod.rs b/grovedb/src/batch/estimated_costs/mod.rs index a7092d49d..673d78ee5 100644 --- a/grovedb/src/batch/estimated_costs/mod.rs +++ b/grovedb/src/batch/estimated_costs/mod.rs @@ -43,7 +43,7 @@ pub(in crate::batch) fn wrapper_overhead_for( // ── Backward-references fan-out estimation model ──────────────────────── // -// Under `BatchApplyOptions::propagate_backward_references` (GROVE_V4+), a +// Under `BatchApplyOptions::propagate_backward_references_when_unsure` (GROVE_V4+), a // single op can expand into derived operations in OTHER subtrees: // registering on a target, rewriting every referrer chain with a new end // hash, cascading deletions through referrer chains. The estimator cannot diff --git a/grovedb/src/batch/estimated_costs/worst_case_costs.rs b/grovedb/src/batch/estimated_costs/worst_case_costs.rs index 3f7041dc0..d9bd6b8de 100644 --- a/grovedb/src/batch/estimated_costs/worst_case_costs.rs +++ b/grovedb/src/batch/estimated_costs/worst_case_costs.rs @@ -53,7 +53,7 @@ impl GroveOp { in_parent_tree_type: TreeType, worst_case_layer_element_estimates: &WorstCaseLayerInformation, // Whether the batch opts into backward-references bookkeeping - // (`BatchApplyOptions::propagate_backward_references`): family ops + // (`BatchApplyOptions::propagate_backward_references_when_unsure`): family ops // and deletes then charge the derived fan-out on GROVE_V4+. backward_references_enabled: bool, propagate: bool, @@ -132,9 +132,10 @@ impl GroveOp { // The flagged apply path probes a deleted tree's child subtree for // emptiness (a merk open and its root read) before admitting the // deletion — charged whenever the fan-out is active. - let flagged_delete_probe = || { + let fan_out_active = backward_references_enabled && fan_out_version != 0; + let delete_probe = |active: bool| { let mut probe = OperationCost::default(); - if backward_references_enabled && fan_out_version != 0 { + if active { for _ in 0..2 { let _ = add_worst_case_get_merk_node( &mut probe, @@ -315,7 +316,7 @@ impl GroveOp { ), backward_references_fan_out(None), ) - .add_cost(flagged_delete_probe()), + .add_cost(delete_probe(fan_out_active)), GroveOp::DeleteTree(tree_type, _) => with_fan_out( GroveDb::worst_case_merk_delete_tree( key, @@ -326,7 +327,30 @@ impl GroveOp { ), backward_references_fan_out(None), ) - .add_cost(flagged_delete_probe()), + .add_cost(delete_probe(fan_out_active)), + // Forces the bookkeeping whatever the batch flag says: the + // displaced-state fan-out and the probe are charged whenever the + // version models them. + GroveOp::DeleteWithCascade => with_fan_out( + GroveDb::worst_case_merk_delete_element( + key, + worst_case_layer_element_estimates, + propagate, + grove_version, + ), + (fan_out_version != 0).then(super::BackwardReferencesFanOut::worst_item), + ) + .add_cost(delete_probe(fan_out_version != 0)), + // Opts out of the bookkeeping whatever the batch flag says: the + // plain delete model, nothing derived. + GroveOp::DeleteWithNoBackwardsReferenceCheck => { + GroveDb::worst_case_merk_delete_element( + key, + worst_case_layer_element_estimates, + propagate, + grove_version, + ) + } GroveOp::CommitmentTreeInsert { payload, .. } => { Self::worst_case_commitment_tree_insert( payload, @@ -889,7 +913,7 @@ impl TreeCache for WorstCaseTreeCacheKnownPaths { &key, TreeType::NormalTree, worst_case_layer_element_estimates, - batch_apply_options.propagate_backward_references, + batch_apply_options.propagate_backward_references_when_unsure, false, grove_version ) diff --git a/grovedb/src/batch/indexed_tree/pre_state.rs b/grovedb/src/batch/indexed_tree/pre_state.rs index f4387f804..7337883c7 100644 --- a/grovedb/src/batch/indexed_tree/pre_state.rs +++ b/grovedb/src/batch/indexed_tree/pre_state.rs @@ -86,6 +86,8 @@ fn validate_indexed_child_ops( // Ops that carry no caller-supplied element, or whose element is // internally derived rather than caller-claimed. GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck | GroveOp::DeleteTree(..) | GroveOp::ReplaceBackwardReferenceFamilyMember { .. } | GroveOp::ReplaceTreeRootKey { .. } diff --git a/grovedb/src/batch/initial_segment_footprint.rs b/grovedb/src/batch/initial_segment_footprint.rs index 7a50c7573..4e8e1323a 100644 --- a/grovedb/src/batch/initial_segment_footprint.rs +++ b/grovedb/src/batch/initial_segment_footprint.rs @@ -62,7 +62,13 @@ impl InitialSegmentFootprint { } let mut path = qualified.clone(); path.pop(); - if matches!(op, GroveOp::Delete | GroveOp::DeleteTree(..)) { + if matches!( + op, + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + | GroveOp::DeleteTree(..) + ) { deleted.push(qualified.clone()); } // Typed appends were rewritten into ReplaceNonMerkTreeRoot by @@ -135,6 +141,8 @@ impl InitialSegmentFootprint { let replaces_or_deletes_subtree = matches!( op.op, GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck | GroveOp::DeleteTree(..) | GroveOp::InsertOrReplace { .. } | GroveOp::InsertWithKnownToNotAlreadyExist { .. } diff --git a/grovedb/src/batch/mod.rs b/grovedb/src/batch/mod.rs index a0de1b832..bc3808f92 100644 --- a/grovedb/src/batch/mod.rs +++ b/grovedb/src/batch/mod.rs @@ -300,8 +300,8 @@ impl NonMerkTreeMeta { /// /// User-facing variants: `InsertWithKnownToNotAlreadyExist`, `InsertIfNotExists`, /// `InsertOrReplace`, `Replace`, `Patch`, `RefreshReference`, `Delete`, -/// `DeleteTree`, `CommitmentTreeInsert`, `MmrTreeAppend`, `BulkAppend`, -/// `DenseTreeInsert`. +/// `DeleteWithCascade`, `DeleteWithNoBackwardsReferenceCheck`, `DeleteTree`, +/// `CommitmentTreeInsert`, `MmrTreeAppend`, `BulkAppend`, `DenseTreeInsert`. /// /// Internal variants (`ReplaceTreeRootKey`, `InsertTreeWithRootHash`, /// `ReplaceNonMerkTreeRoot`, `InsertNonMerkTree`) are marked @@ -566,8 +566,26 @@ pub enum GroveOp { /// untrusted cross-check against on-disk. non_counted: bool, }, - /// Delete + /// Delete. Whether the deleted element is first read for + /// backward-references bookkeeping follows the batch's + /// `BatchApplyOptions::propagate_backward_references_when_unsure`; the + /// two typed variants below pin that decision per op. Delete, + /// Delete an element and cascade away every bidirectional reference + /// registered on it, whatever the batch's + /// `propagate_backward_references_when_unsure` setting (`GROVE_V4`+). + /// The element is read before the deletion; each referrer chain is + /// deleted (every affected reference must allow `cascade_on_update`, + /// otherwise the batch errors) and a deleted `BidirectionalReference` + /// is de-registered from its target. Exactly what `Delete` does in a + /// batch with the flag set. + DeleteWithCascade, + /// Delete an element without reading it for backward-references + /// bookkeeping, whatever the batch's + /// `propagate_backward_references_when_unsure` setting (`GROVE_V4`+). + /// Bidirectional references registered on it are left dangling. + /// Exactly what `Delete` does in a batch without the flag. + DeleteWithNoBackwardsReferenceCheck, /// Delete tree DeleteTree(TreeType, SubelementsDeletionBehavior), /// Insert a note commitment + payload into a CommitmentTree @@ -635,6 +653,8 @@ impl GroveOp { GroveOp::InsertAggregateIndexedTreeRootKeys { .. } => 18, GroveOp::PrivateDocumentStoreInsert { .. } => 19, GroveOp::ReplaceBackwardReferenceFamilyMember { .. } => 20, + GroveOp::DeleteWithCascade => 21, + GroveOp::DeleteWithNoBackwardsReferenceCheck => 22, } } @@ -671,6 +691,8 @@ impl GroveOp { | GroveOp::Replace { .. } | GroveOp::Patch { .. } | GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck | GroveOp::DeleteTree(..) | GroveOp::RefreshReference { .. } => true, @@ -750,6 +772,8 @@ impl GroveOp { | GroveOp::Replace { .. } | GroveOp::Patch { .. } | GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck | GroveOp::DeleteTree(..) | GroveOp::RefreshReference { .. } | GroveOp::ReplaceTreeRootKey { .. } @@ -1048,6 +1072,10 @@ impl fmt::Debug for QualifiedGroveDbOp { hex::encode(node_value_hash) ), GroveOp::Delete => "Delete".to_string(), + GroveOp::DeleteWithCascade => "Delete With Cascade".to_string(), + GroveOp::DeleteWithNoBackwardsReferenceCheck => { + "Delete With No Backwards Reference Check".to_string() + } GroveOp::DeleteTree(tree_type, check) => { format!("Delete Tree {} ({:?})", tree_type, check) } @@ -1395,6 +1423,49 @@ impl QualifiedGroveDbOp { } } + /// A delete op that cascades away the bidirectional references registered + /// on the deleted element, whatever the batch flag says (`GROVE_V4`+). + pub fn delete_with_cascade_op(path: Vec>, key: Vec) -> Self { + let path = KeyInfoPath::from_known_owned_path(path); + Self { + path, + key: Some(KnownKey(key)), + op: GroveOp::DeleteWithCascade, + } + } + + /// A delete op that skips the backward-references check, whatever the + /// batch flag says (`GROVE_V4`+). + pub fn delete_with_no_backwards_reference_check_op(path: Vec>, key: Vec) -> Self { + let path = KeyInfoPath::from_known_owned_path(path); + Self { + path, + key: Some(KnownKey(key)), + op: GroveOp::DeleteWithNoBackwardsReferenceCheck, + } + } + + /// A delete-with-cascade op for estimation + pub fn delete_with_cascade_estimated_op(path: KeyInfoPath, key: KeyInfo) -> Self { + Self { + path, + key: Some(key), + op: GroveOp::DeleteWithCascade, + } + } + + /// A delete-with-no-backwards-reference-check op for estimation + pub fn delete_with_no_backwards_reference_check_estimated_op( + path: KeyInfoPath, + key: KeyInfo, + ) -> Self { + Self { + path, + key: Some(key), + op: GroveOp::DeleteWithNoBackwardsReferenceCheck, + } + } + /// A commitment tree insert op. `path` includes the tree key as its last /// segment (e.g. `vec![b"pool".to_vec()]` for a tree at key `b"pool"` in /// the root subtree). @@ -1566,7 +1637,13 @@ impl QualifiedGroveDbOp { // Build a map of deleted_qualified_path -> indices of delete ops let mut deleted_path_to_op_indices: HashMap> = HashMap::new(); for (idx, op) in ops.iter().enumerate() { - if matches!(op.op, GroveOp::Delete | GroveOp::DeleteTree(..)) { + if matches!( + op.op, + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + | GroveOp::DeleteTree(..) + ) { let Some(ref key) = op.key else { continue; }; @@ -2845,7 +2922,10 @@ where grove_version, ) } - GroveOp::Delete | GroveOp::DeleteTree(..) => Err(Error::InvalidBatchOperation( + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + | GroveOp::DeleteTree(..) => Err(Error::InvalidBatchOperation( "references can not point to something currently being deleted", )) .wrap_with_cost(cost), @@ -4112,7 +4192,9 @@ where ) ); } - GroveOp::Delete => { + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck => { cost_return_on_error_into!( &mut cost, Element::delete_into_batch_operations( @@ -5160,7 +5242,10 @@ impl GroveDb { )) .wrap_with_cost(cost); } - GroveOp::Delete | GroveOp::DeleteTree(..) => { + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + | GroveOp::DeleteTree(..) => { if calculated_root_key.is_some() { return Err(Error::InvalidBatchOperation( "modification of tree when it will be \ @@ -5572,7 +5657,9 @@ impl GroveDb { ); } } - GroveOp::Delete => { + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck => { let path_slices: Vec<&[u8]> = op.path.iterator().map(|p| p.as_slice()).collect(); let key = cost_return_on_error_no_add!( @@ -5581,12 +5668,47 @@ impl GroveDb { .as_ref() .ok_or(Error::InvalidBatchOperation("delete op is missing a key")) ); + // The typed variants pin the backward-references check on + // or off for this op; plain `Delete` follows the batch's + // `propagate_backward_references_when_unsure`. Pre-V4 the + // live delete ignores the flag, so the typed ops fail + // closed there instead of silently degrading. + let mut delete_options = options.clone().map(|o| o.as_delete_options()); + match &op.op { + GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + if grove_version + .grovedb_versions + .operations + .insert + .insert_on_transaction + < 1 => + { + return Err(Error::NotSupported( + "DeleteWithCascade and DeleteWithNoBackwardsReferenceCheck \ + require GROVE_V4+" + .to_owned(), + )) + .wrap_with_cost(cost); + } + GroveOp::DeleteWithCascade => { + delete_options + .get_or_insert_with(DeleteOptions::default) + .propagate_backward_references_when_unsure = true; + } + GroveOp::DeleteWithNoBackwardsReferenceCheck => { + if let Some(delete_options) = delete_options.as_mut() { + delete_options.propagate_backward_references_when_unsure = false; + } + } + _ => {} + } cost_return_on_error!( &mut cost, self.delete( path_slices.as_slice(), key.as_slice(), - options.clone().map(|o| o.as_delete_options()), + delete_options, transaction, grove_version ) @@ -5642,9 +5764,9 @@ impl GroveDb { validate_tree_at_path_exists: false, // Same decision as `as_delete_options`: the batch's // opt-in extends to its deletes. - propagate_backward_references: options + propagate_backward_references_when_unsure: options .as_ref() - .is_some_and(|o| o.propagate_backward_references), + .is_some_and(|o| o.propagate_backward_references_when_unsure), }; cost_return_on_error!( &mut cost, @@ -6151,7 +6273,9 @@ impl GroveDb { let batch_deleted_keys = ops .iter() .filter_map(|other_op| match &other_op.op { - GroveOp::Delete => { + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck => { if other_op.path.to_path() == child_path { Some(other_op.key.as_ref()?.as_slice().to_vec()) } else { @@ -6300,7 +6424,9 @@ impl GroveDb { let batch_deleted_keys = ops .iter() .filter_map(|other_op| match &other_op.op { - GroveOp::Delete => { + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck => { if other_op.path.to_path() == child_path { Some(other_op.key.as_ref()?.as_slice().to_vec()) } else { @@ -6391,18 +6517,37 @@ impl GroveDb { /// Backward-references family elements are only valid in batches that /// opt into the bookkeeping via - /// `BatchApplyOptions::propagate_backward_references` (GROVE_V4+), + /// `BatchApplyOptions::propagate_backward_references_when_unsure` (GROVE_V4+), /// where the preprocessor expands them into the derived operations the /// live flagged flow performs. Everywhere else (`allow_family` false: /// unflagged batches, partial batches, partial-batch add-on ops) they /// fail closed — the pipeline would otherwise silently produce /// inconsistent backward-reference state (a `BidirectionalReference` /// whose target never learns about it). + /// + /// The typed deletes (`DeleteWithCascade` / + /// `DeleteWithNoBackwardsReferenceCheck`) are valid only where + /// `allow_typed_deletes` is set: full batches on GROVE_V4+. Partial + /// batches have no expansion support, and pre-V4 the live delete ignores + /// the flag, so both would silently degrade to a plain delete. fn reject_backward_references_elements_in_batch( ops: &[QualifiedGroveDbOp], allow_family: bool, + allow_typed_deletes: bool, ) -> Result<(), Error> { for op in ops { + if !allow_typed_deletes + && matches!( + op.op, + GroveOp::DeleteWithCascade | GroveOp::DeleteWithNoBackwardsReferenceCheck + ) + { + return Err(Error::NotSupported( + "DeleteWithCascade and DeleteWithNoBackwardsReferenceCheck require GROVE_V4+ \ + and are not supported in partial batches" + .to_owned(), + )); + } // The derived write op is internal to the preprocessor; a // caller supplying one could install arbitrary value hashes. if matches!(op.op, GroveOp::ReplaceBackwardReferenceFamilyMember { .. }) { @@ -6433,7 +6578,7 @@ impl GroveDb { if !allow_family { return Err(Error::NotSupported( "backward-references family elements require \ - BatchApplyOptions::propagate_backward_references (GROVE_V4+)" + BatchApplyOptions::propagate_backward_references_when_unsure (GROVE_V4+)" .to_owned(), )); } @@ -6522,30 +6667,45 @@ impl GroveDb { } } - // Backward-references bookkeeping is a per-batch opt-in, and rides - // the same activation as the live flagged flow (`GROVE_V4`+, where - // `insert_on_transaction` dispatches to v1). + // Backward-references bookkeeping rides the same activation as the + // live flagged flow (`GROVE_V4`+, where `insert_on_transaction` + // dispatches to v1). Plain ops opt in per batch through + // `propagate_backward_references_when_unsure`; the typed deletes + // (`DeleteWithCascade` / `DeleteWithNoBackwardsReferenceCheck`) pin + // the check on or off for themselves, whatever the flag says. + let backward_references_supported = grove_version + .grovedb_versions + .operations + .insert + .insert_on_transaction + >= 1; let backward_references_enabled = batch_apply_options .as_ref() - .map(|options| options.propagate_backward_references) + .map(|options| options.propagate_backward_references_when_unsure) .unwrap_or(false) - && grove_version - .grovedb_versions - .operations - .insert - .insert_on_transaction - >= 1; + && backward_references_supported; cost_return_on_error_no_add!( cost, - Self::reject_backward_references_elements_in_batch(&ops, backward_references_enabled) + Self::reject_backward_references_elements_in_batch( + &ops, + backward_references_enabled, + backward_references_supported, + ) ); - let ops = if backward_references_enabled { + // A `DeleteWithCascade` needs the expansion even in an unflagged + // batch; the preprocessor then runs in per-op mode, touching nothing + // but the cascading deletes. + let has_cascade_deletes = ops + .iter() + .any(|op| matches!(op.op, GroveOp::DeleteWithCascade)); + let ops = if backward_references_enabled || has_cascade_deletes { let ops = cost_return_on_error!( &mut cost, backward_references::expand_backward_references_ops( self, &tx, ops, + backward_references_enabled, batch_apply_options .as_ref() .map(|options| options.validate_insertion_does_not_override) @@ -6987,7 +7147,7 @@ impl GroveDb { cost_return_on_error_no_add!( cost, - Self::reject_backward_references_elements_in_batch(&ops, false) + Self::reject_backward_references_elements_in_batch(&ops, false, false) ); cost_return_on_error!( @@ -7305,7 +7465,7 @@ impl GroveDb { // carrying the family would only fail deep inside execution. cost_return_on_error_no_add!( cost, - Self::reject_backward_references_elements_in_batch(&new_operations, false) + Self::reject_backward_references_elements_in_batch(&new_operations, false, false) ); // Add-on typed appends (CommitmentTreeInsert, MmrTreeAppend, @@ -7921,7 +8081,7 @@ mod tests { disable_operation_consistency_check: true, base_root_storage_is_free: true, batch_pause_height: None, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, grove_version @@ -8527,7 +8687,7 @@ mod tests { disable_operation_consistency_check: false, base_root_storage_is_free: true, batch_pause_height: None, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, grove_version @@ -8569,7 +8729,7 @@ mod tests { validate_insertion_does_not_override: true, base_root_storage_is_free: true, batch_pause_height: None, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, grove_version @@ -8603,7 +8763,7 @@ mod tests { disable_operation_consistency_check: false, base_root_storage_is_free: true, batch_pause_height: None, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, grove_version diff --git a/grovedb/src/batch/options.rs b/grovedb/src/batch/options.rs index 66673f998..9d7c81768 100644 --- a/grovedb/src/batch/options.rs +++ b/grovedb/src/batch/options.rs @@ -77,14 +77,19 @@ pub struct BatchApplyOptions { /// At what height do we want to pause applying batch operations /// Most of the time this should be not set pub batch_pause_height: Option, - /// Opt into backward-references bookkeeping for this batch: ops - /// carrying the backward-references family (`BidirectionalReference` - /// and the three backward-references item variants) become valid, and - /// the batch expands into the derived registration / propagation / + /// Opt into backward-references bookkeeping for this batch's plain ops: + /// every write and delete that does not say otherwise is first read to + /// see whether it displaces an element carrying backward references, + /// and the batch expands into the derived registration / propagation / /// cascade operations the live flagged flow would perform — including - /// references whose targets are created in the same batch. See + /// references whose targets are created in the same batch. Ops carrying + /// the backward-references family (`BidirectionalReference` and the + /// three backward-references item variants) are valid only with the + /// flag set. The typed deletes (`GroveOp::DeleteWithCascade` / + /// `GroveOp::DeleteWithNoBackwardsReferenceCheck`) pin the decision for + /// themselves and ignore this flag. `GROVE_V4`+; see /// `batch::backward_references` for the expansion and conflict rules. - pub propagate_backward_references: bool, + pub propagate_backward_references_when_unsure: bool, } #[cfg(feature = "minimal")] @@ -96,7 +101,7 @@ impl Default for BatchApplyOptions { disable_operation_consistency_check: false, base_root_storage_is_free: true, batch_pause_height: None, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, } } } @@ -110,7 +115,8 @@ impl BatchApplyOptions { validate_insertion_does_not_override_tree: self .validate_insertion_does_not_override_tree, base_root_storage_is_free: self.base_root_storage_is_free, - propagate_backward_references: self.propagate_backward_references, + propagate_backward_references_when_unsure: self + .propagate_backward_references_when_unsure, } } @@ -125,7 +131,8 @@ impl BatchApplyOptions { // into backward-references bookkeeping keeps its deletes // flagged too, so registered targets cascade instead of // silently dangling. - propagate_backward_references: self.propagate_backward_references, + propagate_backward_references_when_unsure: self + .propagate_backward_references_when_unsure, } } diff --git a/grovedb/src/debugger.rs b/grovedb/src/debugger.rs index a1c221802..5a541f9b7 100644 --- a/grovedb/src/debugger.rs +++ b/grovedb/src/debugger.rs @@ -1762,7 +1762,7 @@ mod tests { Some(vec![7]), ), Some(InsertOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, diff --git a/grovedb/src/operations/delete/delete_internal_on_transaction/mod.rs b/grovedb/src/operations/delete/delete_internal_on_transaction/mod.rs index 1a7ae89d4..8558202fb 100644 --- a/grovedb/src/operations/delete/delete_internal_on_transaction/mod.rs +++ b/grovedb/src/operations/delete/delete_internal_on_transaction/mod.rs @@ -34,7 +34,7 @@ //! everything else is identical. See [v0] / [v1]. //! //! * **[v2]** — `GROVE_V4`+ with backward-references support. A router: -//! calls without `DeleteOptions::propagate_backward_references` run the +//! calls without `DeleteOptions::propagate_backward_references_when_unsure` run the //! exact v1 body (identical root hashes and costs); calls with the flag //! run a `MerkCache`-based flow that cascades backward-reference chains. //! diff --git a/grovedb/src/operations/delete/delete_internal_on_transaction/v2.rs b/grovedb/src/operations/delete/delete_internal_on_transaction/v2.rs index 0d21eff75..c1c676c53 100644 --- a/grovedb/src/operations/delete/delete_internal_on_transaction/v2.rs +++ b/grovedb/src/operations/delete/delete_internal_on_transaction/v2.rs @@ -1,7 +1,7 @@ //! `delete_internal_on_transaction` — **v2** (`GROVE_V4`+). //! //! A behaviour-preserving router. A call without -//! [`DeleteOptions::propagate_backward_references`] runs the exact v1 body +//! [`DeleteOptions::propagate_backward_references_when_unsure`] runs the exact v1 body //! (`GROVE_V4`'s parent-reuse delete, issue #686) — identical root hashes //! and costs. A call WITH the flag runs the `MerkCache`-based flow below, //! which fetches the deleted element, cascades backward-reference chains @@ -60,7 +60,7 @@ impl GroveDb { batch: &StorageBatch, grove_version: &GroveVersion, ) -> CostResult { - if options.propagate_backward_references { + if options.propagate_backward_references_when_unsure { self.delete_with_backward_references( path, key, @@ -123,7 +123,7 @@ impl GroveDb { cost, crate::operations::indexed_tree::reject_generic_write_into_indexed_primary( subtree_to_delete_from_type, - "delete with propagate_backward_references", + "delete with propagate_backward_references_when_unsure", ) ); @@ -147,7 +147,7 @@ impl GroveDb { { return Err(Error::NotSupported( "specialized data trees and indexed trees cannot be deleted with \ - propagate_backward_references set; delete them without the flag" + propagate_backward_references_when_unsure set; delete them without the flag" .to_owned(), )) .wrap_with_cost(cost); @@ -185,7 +185,7 @@ impl GroveDb { transaction, DeletionVisitor::new( &cache, - options.propagate_backward_references, + options.propagate_backward_references_when_unsure, true, sectioned_removal, ), @@ -298,7 +298,7 @@ impl GroveDb { /// we're good as long as we do nothing outside of the cache, then finalize /// it, and only then merge with the final deletion batches. struct DeletionVisitor<'c, 'db, 'b, 's, B: AsRef<[u8]>> { - propagate_backward_references: bool, + propagate_backward_references_when_unsure: bool, allow_deleting_subtrees: bool, cache: &'c MerkCache<'db, 'b, B>, /// The caller's removal-accounting policy, applied to every referrer a @@ -309,12 +309,12 @@ struct DeletionVisitor<'c, 'db, 'b, 's, B: AsRef<[u8]>> { impl<'c, 'db, 'b, 's, B: AsRef<[u8]>> DeletionVisitor<'c, 'db, 'b, 's, B> { fn new( cache: &'c MerkCache<'db, 'b, B>, - propagate_backward_references: bool, + propagate_backward_references_when_unsure: bool, allow_deleting_subtrees: bool, sectioned_removal: bidirectional_references::SectionedRemovalFn<'s>, ) -> Self { Self { - propagate_backward_references, + propagate_backward_references_when_unsure, allow_deleting_subtrees, cache, sectioned_removal, @@ -360,7 +360,7 @@ impl<'b, B: AsRef<[u8]>> Visit<'b, B> for DeletionVisitor<'_, '_, 'b, '_, B> { { return Err(Error::NotSupported( "a descendant specialized data tree or indexed tree blocks deletion with \ - propagate_backward_references set; delete it without the flag first" + propagate_backward_references_when_unsure set; delete it without the flag first" .to_owned(), )) .wrap_with_cost(cost); @@ -370,7 +370,7 @@ impl<'b, B: AsRef<[u8]>> Visit<'b, B> for DeletionVisitor<'_, '_, 'b, '_, B> { // Step 2: perform backward references' deletion on top of cached // data: - if self.propagate_backward_references + if self.propagate_backward_references_when_unsure && matches!( element, Element::ItemWithBackwardsReferences(..) diff --git a/grovedb/src/operations/delete/delete_up_tree.rs b/grovedb/src/operations/delete/delete_up_tree.rs index 319cf550b..9b2101691 100644 --- a/grovedb/src/operations/delete/delete_up_tree.rs +++ b/grovedb/src/operations/delete/delete_up_tree.rs @@ -51,7 +51,7 @@ impl DeleteUpTreeOptions { deleting_non_empty_trees_returns_error: self.deleting_non_empty_trees_returns_error, base_root_storage_is_free: self.base_root_storage_is_free, validate_tree_at_path_exists: self.validate_tree_at_path_exists, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, } } } diff --git a/grovedb/src/operations/delete/mod.rs b/grovedb/src/operations/delete/mod.rs index 21fabcfda..dc853eeb0 100644 --- a/grovedb/src/operations/delete/mod.rs +++ b/grovedb/src/operations/delete/mod.rs @@ -15,7 +15,7 @@ //! //! The exception is the opt-in bidirectional-references machinery //! (`GROVE_V4`+): deleting with -//! [`DeleteOptions::propagate_backward_references`] set cascades any +//! [`DeleteOptions::propagate_backward_references_when_unsure`] set cascades any //! [`BidirectionalReference`](crate::Element::BidirectionalReference) //! chains that point at the deleted element (each affected reference must //! allow `cascade_on_update`, otherwise the delete errors instead). See @@ -113,7 +113,7 @@ pub struct DeleteOptions { /// (each affected reference must allow `cascade_on_update`, otherwise /// the operation errors). Opt-in per call because the checks require an /// extra fetch on every delete. Requires `GROVE_V4`+. - pub propagate_backward_references: bool, + pub propagate_backward_references_when_unsure: bool, } #[cfg(feature = "minimal")] @@ -124,7 +124,7 @@ impl Default for DeleteOptions { deleting_non_empty_trees_returns_error: true, base_root_storage_is_free: true, validate_tree_at_path_exists: false, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, } } } @@ -144,7 +144,7 @@ impl GroveDb { /// /// # Dangling references /// - /// Without [`DeleteOptions::propagate_backward_references`], this + /// Without [`DeleteOptions::propagate_backward_references_when_unsure`], this /// operation does **not** check for incoming references. If other /// elements hold [`Reference`](crate::Element::Reference) paths that point /// to the deleted element, those references become dangling. Following a @@ -513,7 +513,10 @@ impl GroveDb { let batch_deleted_keys = current_batch_operations .iter() .filter_map(|op| match op.op { - GroveOp::Delete | GroveOp::DeleteTree(..) => { + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + | GroveOp::DeleteTree(..) => { if op.path.eq_path_vec(&subtree_merk_path_vec) { Some(op.key.as_ref()?.as_slice()) } else { @@ -542,7 +545,10 @@ impl GroveDb { // If there is any current batch operation that is inserting something in this // tree then it is not empty either is_empty &= !current_batch_operations.iter().any(|op| match op.op { - GroveOp::Delete | GroveOp::DeleteTree(..) => false, + GroveOp::Delete + | GroveOp::DeleteWithCascade + | GroveOp::DeleteWithNoBackwardsReferenceCheck + | GroveOp::DeleteTree(..) => false, _ => op.path.eq_path_vec(&subtree_merk_path_vec), }); @@ -2140,7 +2146,7 @@ mod tests { deleting_non_empty_trees_returns_error: true, base_root_storage_is_free: true, validate_tree_at_path_exists: true, - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, }), None, version, @@ -2179,7 +2185,7 @@ mod tests { deleting_non_empty_trees_returns_error: false, base_root_storage_is_free: true, validate_tree_at_path_exists: true, - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, }), Some(&transaction), version, diff --git a/grovedb/src/operations/insert/add_element_on_transaction/v1.rs b/grovedb/src/operations/insert/add_element_on_transaction/v1.rs index fb3ca10d2..80f957017 100644 --- a/grovedb/src/operations/insert/add_element_on_transaction/v1.rs +++ b/grovedb/src/operations/insert/add_element_on_transaction/v1.rs @@ -277,7 +277,7 @@ impl GroveDb { // The backward-references item variants store exactly like // their plain counterparts; the backward-reference // bookkeeping only runs when the caller opts in via - // `propagate_backward_references` (routed before this call). + // `propagate_backward_references_when_unsure` (routed before this call). // // DELIBERATE TRADEOFF (see adr/bidirectional_references.md): // without the flag, overwriting a key that carries backward diff --git a/grovedb/src/operations/insert/add_element_on_transaction/v2.rs b/grovedb/src/operations/insert/add_element_on_transaction/v2.rs index fa2add655..193970555 100644 --- a/grovedb/src/operations/insert/add_element_on_transaction/v2.rs +++ b/grovedb/src/operations/insert/add_element_on_transaction/v2.rs @@ -329,7 +329,7 @@ impl GroveDb { // The backward-references item variants store exactly like // their plain counterparts; the backward-reference // bookkeeping only runs when the caller opts in via - // `propagate_backward_references` (routed before this call). + // `propagate_backward_references_when_unsure` (routed before this call). // // DELIBERATE TRADEOFF (see adr/bidirectional_references.md): // without the flag, overwriting a key that carries backward diff --git a/grovedb/src/operations/insert/insert_on_transaction/mod.rs b/grovedb/src/operations/insert/insert_on_transaction/mod.rs index 4814c3fda..b24704d15 100644 --- a/grovedb/src/operations/insert/insert_on_transaction/mod.rs +++ b/grovedb/src/operations/insert/insert_on_transaction/mod.rs @@ -10,7 +10,7 @@ //! activates with `GROVE_V4`. //! * **[v1]** — `GROVE_V4`+. Behaviour-preserving router: calls that neither //! insert a `BidirectionalReference` nor set -//! `InsertOptions::propagate_backward_references` run the exact v0 body +//! `InsertOptions::propagate_backward_references_when_unsure` run the exact v0 body //! (same root hashes, same costs). The remainder run through the //! `MerkCache`-based flow in [v1], which performs backward-reference //! bookkeeping and propagation (see `adr/bidirectional_references.md`). diff --git a/grovedb/src/operations/insert/insert_on_transaction/v1.rs b/grovedb/src/operations/insert/insert_on_transaction/v1.rs index 0bf61e893..3c7473aaa 100644 --- a/grovedb/src/operations/insert/insert_on_transaction/v1.rs +++ b/grovedb/src/operations/insert/insert_on_transaction/v1.rs @@ -2,7 +2,7 @@ //! //! A behaviour-preserving router. A call that neither inserts a //! [`Element::BidirectionalReference`] nor sets -//! [`InsertOptions::propagate_backward_references`] runs the exact shipped +//! [`InsertOptions::propagate_backward_references_when_unsure`] runs the exact shipped //! flow ([`super::v0::insert_on_transaction_body`]) — identical root hashes //! and costs. The rest run through the `MerkCache`-based flow below, which //! keeps every touched subtree open in one cache so backward-reference @@ -15,7 +15,7 @@ //! The `MerkCache` flow supports items, references (all three variants), //! and empty plain-Merk trees. Specialized tree types (commitment / MMR / //! bulk-append / dense / private document store / indexed trees) and the -//! aggregation wrappers are rejected when `propagate_backward_references` +//! aggregation wrappers are rejected when `propagate_backward_references_when_unsure` //! is set — their child-hash conventions live in //! `add_element_on_transaction` and have no backward-references semantics //! yet. Insert them without the flag (they cannot be targeted by @@ -72,7 +72,7 @@ pub(super) fn insert_on_transaction<'db, 'b, B: AsRef<[u8]>>( // meta storage, flag or no flag; everything else opts into the // backward-references flow via the flag. if matches!(element, Element::BidirectionalReference(..)) - || options.propagate_backward_references + || options.propagate_backward_references_when_unsure { insert_with_backward_references( db, @@ -266,7 +266,7 @@ fn insert_with_backward_references<'db, 'b, B: AsRef<[u8]>>( // targeted by bidirectional references. Insert them without the // flag. return Err(Error::NotSupported( - "this element type cannot be inserted with propagate_backward_references set; \ + "this element type cannot be inserted with propagate_backward_references_when_unsure set; \ insert it without the flag" .to_owned(), )) diff --git a/grovedb/src/operations/insert/mod.rs b/grovedb/src/operations/insert/mod.rs index 58811a289..1b22f5e10 100644 --- a/grovedb/src/operations/insert/mod.rs +++ b/grovedb/src/operations/insert/mod.rs @@ -31,7 +31,7 @@ pub struct InsertOptions { /// supports backward references. Since the checks require an extra /// fetch on every write, the feature is opt-in per call. Requires /// `GROVE_V4`+; ignored (never set) by shipped v1..v3 flows. - pub propagate_backward_references: bool, + pub propagate_backward_references_when_unsure: bool, } impl Default for InsertOptions { @@ -40,7 +40,7 @@ impl Default for InsertOptions { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: true, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, } } } @@ -474,7 +474,7 @@ mod tests { validate_insertion_does_not_override: true, validate_insertion_does_not_override_tree: true, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, gv, @@ -552,7 +552,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, } } @@ -3313,7 +3313,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), Some(&tx), grove_version, @@ -3603,7 +3603,7 @@ mod tests { b"key5", Element::new_item_allowing_bidirectional_references(b"certainly new value".to_vec()), Some(InsertOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -3633,7 +3633,7 @@ mod tests { b"key5", Element::new_item(b"hello".to_vec()), Some(InsertOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, diff --git a/grovedb/src/tests/batch_backward_references_cost_tests.rs b/grovedb/src/tests/batch_backward_references_cost_tests.rs index ac8a33f29..05b318b4f 100644 --- a/grovedb/src/tests/batch_backward_references_cost_tests.rs +++ b/grovedb/src/tests/batch_backward_references_cost_tests.rs @@ -1,5 +1,5 @@ //! Estimated-cost coverage for backward-references batch ops (batching -//! M5): under `BatchApplyOptions::propagate_backward_references`, the +//! M5): under `BatchApplyOptions::propagate_backward_references_when_unsure`, the //! GROVE_V4 estimators charge the derived fan-out (registration, chain //! propagation, cascade deletion) so `worst-case estimate >= actual` holds //! for flagged family batches, while pre-V4 estimation stays byte-stable @@ -33,7 +33,7 @@ use crate::{ fn batch_flag_on() -> Option { Some(BatchApplyOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }) } @@ -675,3 +675,88 @@ fn declared_capacity_tightens_the_worst_case_estimate() { "{tight_average:?} vs {default_average:?}" ); } + +// ─── Typed deletes ─────────────────────────────────────────────────────── + +#[test] +fn worst_case_estimate_covers_typed_cascade_delete_without_the_flag() { + let grove_version = GroveVersion::latest(); + let db = db_with_chain(grove_version); + + let ops = vec![QualifiedGroveDbOp::delete_with_cascade_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + )]; + let estimate = worst_case_estimate(ops.clone(), None, grove_version); + let actual = db + .apply_batch(ops, None, None, grove_version) + .cost_as_result() + .expect("apply succeeds"); + + assert!( + estimate.worse_or_eq_than(&actual), + "worst-case estimate {estimate:?} must cover the actual cascade {actual:?}" + ); +} + +#[test] +fn typed_delete_fan_out_follows_the_op_not_the_flag() { + let grove_version = GroveVersion::latest(); + let plain = || { + vec![QualifiedGroveDbOp::delete_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + )] + }; + let cascade = || { + vec![QualifiedGroveDbOp::delete_with_cascade_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + )] + }; + let no_check = || { + vec![ + QualifiedGroveDbOp::delete_with_no_backwards_reference_check_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + ), + ] + }; + + // A cascade delete charges the fan-out with the flag off — exactly what + // a plain delete charges with the flag on. + assert_eq!( + worst_case_estimate(cascade(), None, grove_version), + worst_case_estimate(plain(), batch_flag_on(), grove_version) + ); + assert_eq!( + average_case_estimate(cascade(), None, grove_version), + average_case_estimate(plain(), batch_flag_on(), grove_version) + ); + assert!( + worst_case_estimate(cascade(), None, grove_version).seek_count + > worst_case_estimate(plain(), None, grove_version).seek_count + ); + + // A no-check delete charges nothing derived with the flag on — exactly + // what a plain delete charges with the flag off. + assert_eq!( + worst_case_estimate(no_check(), batch_flag_on(), grove_version), + worst_case_estimate(plain(), None, grove_version) + ); + assert_eq!( + average_case_estimate(no_check(), batch_flag_on(), grove_version), + average_case_estimate(plain(), None, grove_version) + ); + + // Pre-V4 the fan-out model is inactive for every op alike. + let v3 = &grovedb_version::version::v3::GROVE_V3; + assert_eq!( + worst_case_estimate(cascade(), None, v3), + worst_case_estimate(plain(), None, v3) + ); + assert_eq!( + average_case_estimate(cascade(), batch_flag_on(), v3), + average_case_estimate(plain(), None, v3) + ); +} diff --git a/grovedb/src/tests/batch_backward_references_tests.rs b/grovedb/src/tests/batch_backward_references_tests.rs index a84306c02..65290560e 100644 --- a/grovedb/src/tests/batch_backward_references_tests.rs +++ b/grovedb/src/tests/batch_backward_references_tests.rs @@ -1,11 +1,12 @@ //! Batch support for the backward-references family (batching M2–M4): the //! master invariant is that a batch under -//! `BatchApplyOptions::propagate_backward_references` produces the exact +//! `BatchApplyOptions::propagate_backward_references_when_unsure` produces the exact //! root hash the live flagged flow produces for the same logical //! operations — including `BidirectionalReference` ops, in-batch targets //! and chains, retargets, identical-edge no-ops, and the M4 conflict //! rules. +use grovedb_path::SubtreePath; use grovedb_version::version::GroveVersion; use crate::{ @@ -19,14 +20,14 @@ use crate::{ fn flag_on() -> Option { Some(InsertOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }) } fn batch_flag_on() -> Option { Some(BatchApplyOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }) } @@ -254,7 +255,7 @@ fn batch_delete_cascades_like_live() { &[TEST_LEAF], b"value", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -802,7 +803,7 @@ fn batch_bidi_delete_matches_live() { &[TEST_LEAF], b"r1", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -2432,3 +2433,465 @@ fn batch_enforces_declared_capacity() { .unwrap() .is_empty()); } + +// ─── Typed deletes: DeleteWithCascade / DeleteWithNoBackwardsReferenceCheck ─ +// +// Plain `Delete` follows the batch's `propagate_backward_references_when_unsure`; +// the two typed variants pin the check on or off for one op, whatever the +// flag says. + +fn live_flagged_delete() -> Option { + Some(DeleteOptions { + propagate_backward_references_when_unsure: true, + ..Default::default() + }) +} + +fn assert_absent(db: &TempGroveDb, key: &[u8], grove_version: &GroveVersion) { + assert!( + matches!( + db.get(&[TEST_LEAF], key, None, grove_version).unwrap(), + Err(Error::PathKeyNotFound(_)) + ), + "{} should be gone", + String::from_utf8_lossy(key) + ); +} + +/// Raw read: a dangling bidirectional reference is still PRESENT (that is +/// the state these tests assert), even though following it fails. +fn assert_present(db: &TempGroveDb, key: &[u8], grove_version: &GroveVersion) { + db.get_raw( + SubtreePath::from([TEST_LEAF].as_ref()), + key, + None, + grove_version, + ) + .unwrap() + .unwrap_or_else(|e| panic!("{} should still exist: {e}", String::from_utf8_lossy(key))); +} + +#[test] +fn typed_cascade_delete_in_unflagged_batch_matches_live_flagged_delete() { + let grove_version = GroveVersion::latest(); + let (batch_db, live_db) = twin_dbs_with_chain(grove_version); + + batch_db + .apply_batch( + vec![QualifiedGroveDbOp::delete_with_cascade_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + )], + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + live_db + .delete( + &[TEST_LEAF], + b"value", + live_flagged_delete(), + None, + grove_version, + ) + .unwrap() + .unwrap(); + + for db in [&batch_db, &live_db] { + for key in [b"value".as_slice(), b"r1", b"r2"] { + assert_absent(db, key, grove_version); + } + } + roots_match(&batch_db, &live_db, grove_version); +} + +#[test] +fn typed_cascade_delete_of_a_reference_deregisters_like_live() { + let grove_version = GroveVersion::latest(); + let (batch_db, live_db) = twin_dbs_with_chain(grove_version); + + // Deleting `r1` (a bidirectional reference) cascades its own referrer + // `r2` away and removes its registration from `value`. + batch_db + .apply_batch( + vec![QualifiedGroveDbOp::delete_with_cascade_op( + vec![TEST_LEAF.to_vec()], + b"r1".to_vec(), + )], + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + live_db + .delete( + &[TEST_LEAF], + b"r1", + live_flagged_delete(), + None, + grove_version, + ) + .unwrap() + .unwrap(); + + for db in [&batch_db, &live_db] { + assert_absent(db, b"r1", grove_version); + assert_absent(db, b"r2", grove_version); + assert_present(db, b"value", grove_version); + } + // `roots_match` also runs `verify_grovedb`, which would report a stale + // registration left on `value`. + roots_match(&batch_db, &live_db, grove_version); +} + +#[test] +fn typed_cascade_delete_requires_consent() { + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + db.insert( + &[TEST_LEAF], + b"value", + Element::new_item_allowing_bidirectional_references(b"hello".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + db.insert( + &[TEST_LEAF], + b"r1", + sibling_bidi(b"value", false), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + assert!(matches!( + db.apply_batch( + vec![QualifiedGroveDbOp::delete_with_cascade_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + )], + None, + None, + grove_version, + ) + .unwrap(), + Err(Error::BidirectionalReferenceRule(_)) + )); + // Nothing was written. + assert_present(&db, b"value", grove_version); + assert_present(&db, b"r1", grove_version); +} + +#[test] +fn typed_no_check_delete_in_flagged_batch_matches_live_unflagged_delete() { + let grove_version = GroveVersion::latest(); + let (batch_db, live_db) = twin_dbs_with_chain(grove_version); + + let batch_cost = batch_db + .apply_batch( + vec![ + QualifiedGroveDbOp::delete_with_no_backwards_reference_check_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + ), + ], + batch_flag_on(), + None, + grove_version, + ) + .cost_as_result() + .unwrap(); + live_db + .delete(&[TEST_LEAF], b"value", None, None, grove_version) + .unwrap() + .unwrap(); + + // No cascade on either side: the chain dangles, as the caller asked. + for db in [&batch_db, &live_db] { + assert_absent(db, b"value", grove_version); + assert_present(db, b"r1", grove_version); + assert_present(db, b"r2", grove_version); + } + assert_eq!( + batch_db.root_hash(None, grove_version).unwrap().unwrap(), + live_db.root_hash(None, grove_version).unwrap().unwrap() + ); + + // The op is not read for bookkeeping even though the batch flag is on: + // it costs exactly what a plain delete costs in an unflagged batch. + let (plain_db, _) = twin_dbs_with_chain(grove_version); + let plain_cost = plain_db + .apply_batch( + vec![QualifiedGroveDbOp::delete_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + )], + None, + None, + grove_version, + ) + .cost_as_result() + .unwrap(); + assert_eq!(batch_cost, plain_cost); +} + +#[test] +fn typed_cascade_delete_leaves_the_other_ops_unflagged() { + let grove_version = GroveVersion::latest(); + // Two registered chains: `r2 -> r1 -> value` and `s1 -> other`. + let build = || { + let (db, _) = twin_dbs_with_chain(grove_version); + db.insert( + &[TEST_LEAF], + b"other", + Element::new_item_allowing_bidirectional_references(b"world".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + db.insert( + &[TEST_LEAF], + b"s1", + sibling_bidi(b"other", true), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + db + }; + let batch_db = build(); + let live_db = build(); + + // Unflagged batch: the cascade op cascades, the plain delete does not. + batch_db + .apply_batch( + vec![ + QualifiedGroveDbOp::delete_with_cascade_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + ), + QualifiedGroveDbOp::delete_op(vec![TEST_LEAF.to_vec()], b"other".to_vec()), + ], + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + live_db + .delete( + &[TEST_LEAF], + b"value", + live_flagged_delete(), + None, + grove_version, + ) + .unwrap() + .unwrap(); + live_db + .delete(&[TEST_LEAF], b"other", None, None, grove_version) + .unwrap() + .unwrap(); + + for db in [&batch_db, &live_db] { + for key in [b"value".as_slice(), b"r1", b"r2", b"other"] { + assert_absent(db, key, grove_version); + } + // The plain delete left its referrer dangling. + assert_present(db, b"s1", grove_version); + } + assert_eq!( + batch_db.root_hash(None, grove_version).unwrap().unwrap(), + live_db.root_hash(None, grove_version).unwrap().unwrap() + ); +} + +#[test] +fn typed_cascade_delete_hitting_a_user_op_fails_closed() { + let grove_version = GroveVersion::latest(); + let (db, _) = twin_dbs_with_chain(grove_version); + + // The cascade from `value` must delete `r1`, which another op deletes. + assert!(matches!( + db.apply_batch( + vec![ + QualifiedGroveDbOp::delete_with_cascade_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + ), + QualifiedGroveDbOp::delete_op(vec![TEST_LEAF.to_vec()], b"r1".to_vec()), + ], + None, + None, + grove_version, + ) + .unwrap(), + Err(Error::InvalidBatchOperation(_)) + )); + // ... or overwrites. + assert!(matches!( + db.apply_batch( + vec![ + QualifiedGroveDbOp::delete_with_cascade_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + ), + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec()], + b"r1".to_vec(), + Element::new_item(b"plain".to_vec()), + ), + ], + None, + None, + grove_version, + ) + .unwrap(), + Err(Error::InvalidBatchOperation(_)) + )); + // Nothing was written. + for key in [b"value".as_slice(), b"r1", b"r2"] { + assert_present(&db, key, grove_version); + } +} + +#[test] +fn typed_deletes_through_apply_operations_without_batching_match_live() { + let grove_version = GroveVersion::latest(); + let (batch_db, live_db) = twin_dbs_with_chain(grove_version); + + batch_db + .apply_operations_without_batching( + vec![QualifiedGroveDbOp::delete_with_cascade_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + )], + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + live_db + .delete( + &[TEST_LEAF], + b"value", + live_flagged_delete(), + None, + grove_version, + ) + .unwrap() + .unwrap(); + roots_match(&batch_db, &live_db, grove_version); + + let (batch_db, live_db) = twin_dbs_with_chain(grove_version); + batch_db + .apply_operations_without_batching( + vec![ + QualifiedGroveDbOp::delete_with_no_backwards_reference_check_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + ), + ], + batch_flag_on(), + None, + grove_version, + ) + .unwrap() + .unwrap(); + live_db + .delete(&[TEST_LEAF], b"value", None, None, grove_version) + .unwrap() + .unwrap(); + assert_present(&batch_db, b"r1", grove_version); + assert_eq!( + batch_db.root_hash(None, grove_version).unwrap().unwrap(), + live_db.root_hash(None, grove_version).unwrap().unwrap() + ); +} + +#[test] +fn typed_deletes_are_refused_pre_v4_and_in_partial_batches() { + let grove_version = GroveVersion::latest(); + let v3 = &grovedb_version::version::v3::GROVE_V3; + let (db, _) = twin_dbs_with_chain(grove_version); + + let typed_ops = || { + [ + QualifiedGroveDbOp::delete_with_cascade_op(vec![TEST_LEAF.to_vec()], b"value".to_vec()), + QualifiedGroveDbOp::delete_with_no_backwards_reference_check_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + ), + ] + }; + + for op in typed_ops() { + // Pre-V4 the live delete ignores the flag: fail closed instead of + // silently degrading, with or without the batch flag. + for options in [None, batch_flag_on()] { + assert!(matches!( + db.apply_batch(vec![op.clone()], options.clone(), None, v3) + .unwrap(), + Err(Error::NotSupported(_)) + )); + assert!(matches!( + db.apply_operations_without_batching(vec![op.clone()], options, None, v3) + .unwrap(), + Err(Error::NotSupported(_)) + )); + } + // Partial batches have no expansion support. + assert!(matches!( + db.apply_partial_batch( + vec![op.clone()], + None, + |_cost, _leftover| Ok(vec![]), + None, + grove_version, + ) + .unwrap(), + Err(Error::NotSupported(_)) + )); + // ... including their add-on ops. + assert!(matches!( + db.apply_partial_batch( + vec![QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec()], + b"fresh".to_vec(), + Element::new_item(b"x".to_vec()), + )], + None, + |_cost, _leftover| Ok(vec![op.clone()]), + None, + grove_version, + ) + .unwrap(), + Err(Error::NotSupported(_)) + )); + } + for key in [b"value".as_slice(), b"r1", b"r2"] { + assert_present(&db, key, grove_version); + } +} + +#[test] +fn typed_delete_sort_tags_are_pinned() { + use crate::batch::GroveOp; + assert_eq!(GroveOp::DeleteWithCascade.to_u8(), 21); + assert_eq!(GroveOp::DeleteWithNoBackwardsReferenceCheck.to_u8(), 22); + assert!(GroveOp::DeleteWithCascade > GroveOp::Delete); + assert!(GroveOp::DeleteWithNoBackwardsReferenceCheck > GroveOp::DeleteWithCascade); +} diff --git a/grovedb/src/tests/bidirectional_references_tests.rs b/grovedb/src/tests/bidirectional_references_tests.rs index df36c23b7..3e186a18f 100644 --- a/grovedb/src/tests/bidirectional_references_tests.rs +++ b/grovedb/src/tests/bidirectional_references_tests.rs @@ -17,7 +17,7 @@ use crate::{ fn flag_on() -> Option { Some(InsertOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }) } @@ -363,7 +363,7 @@ fn cascade_requires_opt_in() { &[TEST_LEAF], b"value", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -521,7 +521,7 @@ fn override_checks_apply_under_the_flag() { b"value", Element::new_item_allowing_bidirectional_references(b"nope".to_vec()), Some(InsertOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, validate_insertion_does_not_override: true, ..Default::default() }), @@ -578,7 +578,7 @@ fn delete_with_flag_handles_trees() { &[TEST_LEAF], b"empty", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -614,7 +614,7 @@ fn delete_with_flag_handles_trees() { &[TEST_LEAF], b"full", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, allow_deleting_non_empty_trees: false, deleting_non_empty_trees_returns_error: true, ..Default::default() @@ -629,7 +629,7 @@ fn delete_with_flag_handles_trees() { &[TEST_LEAF], b"full", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, allow_deleting_non_empty_trees: false, deleting_non_empty_trees_returns_error: false, ..Default::default() @@ -660,7 +660,7 @@ fn delete_with_flag_handles_trees() { &[TEST_LEAF], b"mmr", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -1197,7 +1197,7 @@ fn delete_with_flag_rejects_rows_of_indexed_primaries() { &[TEST_LEAF, b"pcit"], b"row", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -1678,7 +1678,7 @@ fn flagged_inserts_enforce_tree_shape_guards() { let opts = Some(InsertOptions { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: true, - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }); assert!(matches!( @@ -3186,7 +3186,7 @@ fn flagged_delete_rejects_specialized_descendants() { let options = DeleteOptions { allow_deleting_non_empty_trees: true, deleting_non_empty_trees_returns_error: false, - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }; assert!(matches!( @@ -3567,7 +3567,7 @@ fn bidi_insert_rejects_undersized_max_hop() { ), )], Some(crate::batch::BatchApplyOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -3861,7 +3861,7 @@ fn retarget_rejects_upstream_max_hop_violation() { sibling_bidi(b"d", true), )], Some(crate::batch::BatchApplyOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -3902,7 +3902,7 @@ fn cascade_removes_the_physical_referrer_record() { &[TEST_LEAF], b"value", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -4020,7 +4020,7 @@ fn cascade_forwards_the_sectioned_removal_callback() { SubtreePath::from(&[TEST_LEAF]), b"value", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, @@ -4080,7 +4080,7 @@ fn stale_nonconsenting_registration_does_not_block_target_deletion() { &[TEST_LEAF], b"value", Some(DeleteOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), None, diff --git a/grovedb/src/tests/delete_indexed_tree_tests.rs b/grovedb/src/tests/delete_indexed_tree_tests.rs index 19ac0d573..f297dd2bd 100644 --- a/grovedb/src/tests/delete_indexed_tree_tests.rs +++ b/grovedb/src/tests/delete_indexed_tree_tests.rs @@ -85,7 +85,7 @@ mod tests { deleting_non_empty_trees_returns_error: false, base_root_storage_is_free: true, validate_tree_at_path_exists: false, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, } } diff --git a/grovedb/src/tests/direct_insert_indexed_tests.rs b/grovedb/src/tests/direct_insert_indexed_tests.rs index 4702aae1a..522435196 100644 --- a/grovedb/src/tests/direct_insert_indexed_tests.rs +++ b/grovedb/src/tests/direct_insert_indexed_tests.rs @@ -519,7 +519,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, } } diff --git a/grovedb/src/tests/mod.rs b/grovedb/src/tests/mod.rs index ebed77d5e..b5a1390f9 100644 --- a/grovedb/src/tests/mod.rs +++ b/grovedb/src/tests/mod.rs @@ -4974,7 +4974,7 @@ mod general_tests { None, ), Some(InsertOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), Some(&transaction), @@ -4996,7 +4996,7 @@ mod general_tests { None, ), Some(InsertOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), Some(&transaction), @@ -5018,7 +5018,7 @@ mod general_tests { None, ), Some(InsertOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), Some(&transaction), @@ -5038,7 +5038,7 @@ mod general_tests { b"value", Element::new_item_allowing_bidirectional_references(b"not hello >:(".to_vec()), Some(InsertOptions { - propagate_backward_references: true, + propagate_backward_references_when_unsure: true, ..Default::default() }), Some(&transaction), diff --git a/grovedb/src/tests/operations_coverage_tests.rs b/grovedb/src/tests/operations_coverage_tests.rs index 9e4608a47..251ae106f 100644 --- a/grovedb/src/tests/operations_coverage_tests.rs +++ b/grovedb/src/tests/operations_coverage_tests.rs @@ -1560,7 +1560,7 @@ mod tests { validate_insertion_does_not_override: true, validate_insertion_does_not_override_tree: true, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, grove_version, @@ -1613,7 +1613,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: true, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, grove_version, @@ -3167,7 +3167,7 @@ mod tests { validate_insertion_does_not_override: true, validate_insertion_does_not_override_tree: true, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, grove_version, @@ -3210,7 +3210,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: true, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, grove_version, @@ -3243,7 +3243,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: false, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, grove_version, diff --git a/grovedb/src/tests/ordinary_replacement_cost_tests.rs b/grovedb/src/tests/ordinary_replacement_cost_tests.rs index 4d8224a84..88791e5b0 100644 --- a/grovedb/src/tests/ordinary_replacement_cost_tests.rs +++ b/grovedb/src/tests/ordinary_replacement_cost_tests.rs @@ -67,7 +67,7 @@ fn overwrite_options() -> InsertOptions { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, } } diff --git a/grovedb/src/tests/provable_count_indexed_tree_tests.rs b/grovedb/src/tests/provable_count_indexed_tree_tests.rs index 2d2306810..8baea3bab 100644 --- a/grovedb/src/tests/provable_count_indexed_tree_tests.rs +++ b/grovedb/src/tests/provable_count_indexed_tree_tests.rs @@ -1552,7 +1552,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }; db.insert( [TEST_LEAF].as_ref(), @@ -1609,7 +1609,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }; let res = db .insert([TEST_LEAF].as_ref(), b"pcit", tampered, Some(opts), None, v) @@ -1909,7 +1909,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }); let result = db .insert( diff --git a/grovedb/src/tests/provable_count_provable_sum_indexed_tree_tests.rs b/grovedb/src/tests/provable_count_provable_sum_indexed_tree_tests.rs index 32391da47..250d15c79 100644 --- a/grovedb/src/tests/provable_count_provable_sum_indexed_tree_tests.rs +++ b/grovedb/src/tests/provable_count_provable_sum_indexed_tree_tests.rs @@ -2057,7 +2057,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }; db.insert( [TEST_LEAF].as_ref(), @@ -2110,7 +2110,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }; let res = db .insert( @@ -2491,7 +2491,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }); let result = db .insert( diff --git a/grovedb/src/tests/provable_sum_indexed_tree_tests.rs b/grovedb/src/tests/provable_sum_indexed_tree_tests.rs index a0c939fbd..f162461c5 100644 --- a/grovedb/src/tests/provable_sum_indexed_tree_tests.rs +++ b/grovedb/src/tests/provable_sum_indexed_tree_tests.rs @@ -1577,7 +1577,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }; db.insert( [TEST_LEAF].as_ref(), @@ -1633,7 +1633,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }; let res = db .insert([TEST_LEAF].as_ref(), b"psit", tampered, Some(opts), None, v) diff --git a/grovedb/src/tests/verify_grovedb_indexed_tests.rs b/grovedb/src/tests/verify_grovedb_indexed_tests.rs index 91dce9fac..03b15d5aa 100644 --- a/grovedb/src/tests/verify_grovedb_indexed_tests.rs +++ b/grovedb/src/tests/verify_grovedb_indexed_tests.rs @@ -1418,7 +1418,7 @@ mod tests { validate_insertion_does_not_override: false, validate_insertion_does_not_override_tree: false, base_root_storage_is_free: true, - propagate_backward_references: false, + propagate_backward_references_when_unsure: false, }), None, &grovedb_version::version::v3::GROVE_V3, From 9feebb72877a137a42a495a46f12be170864b476 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 9 Sep 2026 08:15:25 +0700 Subject: [PATCH 2/5] refactor(batch): name the preprocessor's flag parameter after the option it carries Co-Authored-By: Claude Fable 5.1 --- grovedb/src/batch/backward_references.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/grovedb/src/batch/backward_references.rs b/grovedb/src/batch/backward_references.rs index 632af4e70..941d101e2 100644 --- a/grovedb/src/batch/backward_references.rs +++ b/grovedb/src/batch/backward_references.rs @@ -614,7 +614,7 @@ impl<'db, 'g> Expansion<'db, 'g> { /// Expand `ops` with the derived operations the backward-references rules /// require, per the module documentation. /// -/// `flag_on` is the batch's `propagate_backward_references_when_unsure`. +/// `propagate_backward_references_when_unsure` is the batch option of the same name. /// With it set every op gets the bookkeeping; without it the pass runs in /// per-op mode for the `DeleteWithCascade` ops the batch carries — the /// other ops are not read (they pay nothing extra) and only their certain @@ -624,7 +624,7 @@ pub(super) fn expand_backward_references_ops( db: &GroveDb, tx: &TxRef<'_, '_>, ops: Vec, - flag_on: bool, + propagate_backward_references_when_unsure: bool, validate_insertion_does_not_override: bool, grove_version: &GroveVersion, ) -> CostResult, Error> { @@ -679,7 +679,7 @@ pub(super) fn expand_backward_references_ops( // `DeleteWithCascade` under a subtree the same batch creates has // nothing to delete; its previous-state read fails the batch, which is // the fail-closed outcome.) - if flag_on { + if propagate_backward_references_when_unsure { let mut tree_write_positions: Vec = expansion .ops .iter() @@ -736,7 +736,7 @@ pub(super) fn expand_backward_references_ops( | GroveOp::Patch { element, .. } | GroveOp::InsertIfNotExists { element, .. } | GroveOp::InsertWithKnownToNotAlreadyExist { element } => { - if !flag_on { + if !propagate_backward_references_when_unsure { // Per-op mode: writes carry no bookkeeping (family // payloads were rejected upstream) and are not read. // Stage the ones that certainly land so a cascade @@ -905,7 +905,7 @@ pub(super) fn expand_backward_references_ops( let check = match op_kind { GroveOp::DeleteWithCascade => true, GroveOp::DeleteWithNoBackwardsReferenceCheck => false, - _ => flag_on, + _ => propagate_backward_references_when_unsure, }; if !check { // No read, no bookkeeping: the position simply goes @@ -967,7 +967,7 @@ pub(super) fn expand_backward_references_ops( flags, .. } => { - if !flag_on { + if !propagate_backward_references_when_unsure { // Per-op mode: a refresh is an ordinary unflagged write // (not read, not staged — the rebuilt shape needs the // stored one). From a095c2f70dc46d4d5f180ee1372e7db064e92702 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 9 Sep 2026 08:28:20 +0700 Subject: [PATCH 3/5] fix(batch): preserve later writes after cascade cleanup --- grovedb/src/batch/backward_references.rs | 5 ++ .../tests/batch_backward_references_tests.rs | 83 +++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/grovedb/src/batch/backward_references.rs b/grovedb/src/batch/backward_references.rs index 941d101e2..006f47a8f 100644 --- a/grovedb/src/batch/backward_references.rs +++ b/grovedb/src/batch/backward_references.rs @@ -744,6 +744,11 @@ pub(super) fn expand_backward_references_ops( // a conditional insert may write nothing, and its // stored state stays authoritative. if !matches!(op_kind, GroveOp::InsertIfNotExists { .. }) { + // A later write supersedes registration cleanup + // queued by an earlier cascade. Otherwise that + // derived op would collide with this write or + // silently replace its payload during execution. + expansion.derived.remove(&position); expansion.store.stage(position, Some(element.clone())); } continue; diff --git a/grovedb/src/tests/batch_backward_references_tests.rs b/grovedb/src/tests/batch_backward_references_tests.rs index 65290560e..5a889daa1 100644 --- a/grovedb/src/tests/batch_backward_references_tests.rs +++ b/grovedb/src/tests/batch_backward_references_tests.rs @@ -2547,6 +2547,89 @@ fn typed_cascade_delete_of_a_reference_deregisters_like_live() { roots_match(&batch_db, &live_db, grove_version); } +fn assert_typed_cascade_then_plain_overwrite_matches_live(options: Option) { + let grove_version = GroveVersion::latest(); + let replacement = Element::new_item(b"replacement".to_vec()); + for write in [ + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + replacement.clone(), + ), + QualifiedGroveDbOp::replace_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + replacement.clone(), + ), + ] { + let (batch_db, live_db) = twin_dbs_with_chain(grove_version); + // Deleting r1 queues registration cleanup on value. The later + // unflagged overwrite must supersede that derived rewrite. + let ops = vec![ + QualifiedGroveDbOp::delete_with_cascade_op(vec![TEST_LEAF.to_vec()], b"r1".to_vec()), + write, + ]; + assert!(QualifiedGroveDbOp::verify_consistency_of_operations(&ops).is_empty()); + batch_db + .apply_batch(ops, options.clone(), None, grove_version) + .unwrap() + .expect("distinct user positions with a later overwrite are valid"); + + live_db + .delete( + &[TEST_LEAF], + b"r1", + live_flagged_delete(), + None, + grove_version, + ) + .unwrap() + .unwrap(); + live_db + .insert( + &[TEST_LEAF], + b"value", + replacement.clone(), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + assert_eq!( + batch_db + .get_raw( + SubtreePath::from([TEST_LEAF].as_ref()), + b"value", + None, + grove_version, + ) + .unwrap() + .unwrap(), + replacement, + "the user overwrite must supersede the cascade's registration cleanup", + ); + for key in [b"r1".as_slice(), b"r2"] { + assert_absent(&batch_db, key, grove_version); + } + roots_match(&batch_db, &live_db, grove_version); + } +} + +#[test] +fn typed_cascade_then_plain_overwrite_matches_live() { + assert_typed_cascade_then_plain_overwrite_matches_live(None); +} + +#[test] +fn typed_cascade_then_plain_overwrite_with_prevalidated_input_matches_live() { + assert_typed_cascade_then_plain_overwrite_matches_live(Some(BatchApplyOptions { + disable_operation_consistency_check: true, + ..Default::default() + })); +} + #[test] fn typed_cascade_delete_requires_consent() { let grove_version = GroveVersion::latest(); From 0ae46d3806565838a0aa960dda9ddb26ba257b14 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 9 Sep 2026 14:55:30 +0700 Subject: [PATCH 4/5] test(batch): cover typed-delete labels, estimation constructors, and per-op-mode branches Codecov flagged the Debug labels of the two typed deletes, their _estimated_op constructors, BatchApplyOptions::as_insert_options through apply_operations_without_batching, the fresh-subtree pre-scan arms for Replace and known-new tree writes under the flag, and the RefreshReference skip in per-op mode. Each now has a test. Co-Authored-By: Claude Fable 5.1 --- .../batch_backward_references_cost_tests.rs | 13 +- .../tests/batch_backward_references_tests.rs | 136 ++++++++++++++++++ 2 files changed, 143 insertions(+), 6 deletions(-) diff --git a/grovedb/src/tests/batch_backward_references_cost_tests.rs b/grovedb/src/tests/batch_backward_references_cost_tests.rs index 05b318b4f..db2b75f72 100644 --- a/grovedb/src/tests/batch_backward_references_cost_tests.rs +++ b/grovedb/src/tests/batch_backward_references_cost_tests.rs @@ -708,17 +708,18 @@ fn typed_delete_fan_out_follows_the_op_not_the_flag() { b"value".to_vec(), )] }; + let leaf = || KeyInfoPath::from_known_owned_path(vec![TEST_LEAF.to_vec()]); let cascade = || { - vec![QualifiedGroveDbOp::delete_with_cascade_op( - vec![TEST_LEAF.to_vec()], - b"value".to_vec(), + vec![QualifiedGroveDbOp::delete_with_cascade_estimated_op( + leaf(), + KeyInfo::KnownKey(b"value".to_vec()), )] }; let no_check = || { vec![ - QualifiedGroveDbOp::delete_with_no_backwards_reference_check_op( - vec![TEST_LEAF.to_vec()], - b"value".to_vec(), + QualifiedGroveDbOp::delete_with_no_backwards_reference_check_estimated_op( + leaf(), + KeyInfo::KnownKey(b"value".to_vec()), ), ] }; diff --git a/grovedb/src/tests/batch_backward_references_tests.rs b/grovedb/src/tests/batch_backward_references_tests.rs index 5a889daa1..4d5550f8b 100644 --- a/grovedb/src/tests/batch_backward_references_tests.rs +++ b/grovedb/src/tests/batch_backward_references_tests.rs @@ -2978,3 +2978,139 @@ fn typed_delete_sort_tags_are_pinned() { assert!(GroveOp::DeleteWithCascade > GroveOp::Delete); assert!(GroveOp::DeleteWithNoBackwardsReferenceCheck > GroveOp::DeleteWithCascade); } + +#[test] +fn typed_delete_debug_labels() { + let cascade = + QualifiedGroveDbOp::delete_with_cascade_op(vec![TEST_LEAF.to_vec()], b"value".to_vec()); + let no_check = QualifiedGroveDbOp::delete_with_no_backwards_reference_check_op( + vec![TEST_LEAF.to_vec()], + b"value".to_vec(), + ); + assert!(format!("{cascade:?}").contains("Delete With Cascade")); + assert!(format!("{no_check:?}").contains("Delete With No Backwards Reference Check")); +} + +#[test] +fn flagged_batch_fresh_subtree_scan_covers_replace_and_known_new_tree_writes() { + let grove_version = GroveVersion::latest(); + let db = make_test_grovedb(grove_version); + db.insert( + &[TEST_LEAF], + b"old_tree", + Element::empty_tree(), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + // A tree written as known-new is fresh (its content exists only in the + // overlay); a tree re-written with `Replace` over a stored tree is not. + // Both carry a family item written in the same flagged batch. + db.apply_batch( + vec![ + QualifiedGroveDbOp::insert_only_op( + vec![TEST_LEAF.to_vec()], + b"new_tree".to_vec(), + Element::empty_tree(), + ), + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"new_tree".to_vec()], + b"f".to_vec(), + Element::new_item_allowing_bidirectional_references(b"fresh".to_vec()), + ), + QualifiedGroveDbOp::replace_op( + vec![TEST_LEAF.to_vec()], + b"old_tree".to_vec(), + Element::empty_tree(), + ), + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec(), b"old_tree".to_vec()], + b"g".to_vec(), + Element::new_item_allowing_bidirectional_references(b"stored".to_vec()), + ), + ], + batch_flag_on(), + None, + grove_version, + ) + .unwrap() + .unwrap(); + + for (tree, key) in [ + (b"new_tree".as_slice(), b"f".as_slice()), + (b"old_tree", b"g"), + ] { + db.get(&[TEST_LEAF, tree], key, None, grove_version) + .unwrap() + .unwrap(); + } + assert!(db + .verify_grovedb(None, true, true, grove_version) + .unwrap() + .is_empty()); +} + +#[test] +fn typed_cascade_delete_alongside_a_refresh_in_unflagged_batch() { + let grove_version = GroveVersion::latest(); + let (db, _) = twin_dbs_with_chain(grove_version); + db.insert( + &[TEST_LEAF], + b"other", + Element::new_item(b"plain".to_vec()), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + db.insert( + &[TEST_LEAF], + b"pref", + Element::new_reference(ReferencePathType::SiblingReference(b"other".to_vec())), + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + // Per-op mode: the refresh is an ordinary unflagged write next to the + // cascade. + db.apply_batch( + vec![ + QualifiedGroveDbOp::delete_with_cascade_op(vec![TEST_LEAF.to_vec()], b"value".to_vec()), + QualifiedGroveDbOp::refresh_reference_op( + vec![TEST_LEAF.to_vec()], + b"pref".to_vec(), + ReferencePathType::SiblingReference(b"other".to_vec()), + None, + None, + false, + true, + ), + ], + None, + None, + grove_version, + ) + .unwrap() + .unwrap(); + + for key in [b"value".as_slice(), b"r1", b"r2"] { + assert_absent(&db, key, grove_version); + } + assert_eq!( + db.get(&[TEST_LEAF], b"pref", None, grove_version) + .unwrap() + .unwrap(), + Element::new_item(b"plain".to_vec()) + ); + assert!(db + .verify_grovedb(None, true, true, grove_version) + .unwrap() + .is_empty()); +} From 6681236dfc70513b0f16d6b5aaa7eb43834a6aa5 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Wed, 9 Sep 2026 14:56:16 +0700 Subject: [PATCH 5/5] test(batch): forward batch options as insert options on the without-batching path Co-Authored-By: Claude Fable 5.1 --- .../tests/batch_backward_references_tests.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/grovedb/src/tests/batch_backward_references_tests.rs b/grovedb/src/tests/batch_backward_references_tests.rs index 4d5550f8b..ef2ebd32c 100644 --- a/grovedb/src/tests/batch_backward_references_tests.rs +++ b/grovedb/src/tests/batch_backward_references_tests.rs @@ -2887,6 +2887,13 @@ fn typed_deletes_through_apply_operations_without_batching_match_live() { vec![TEST_LEAF.to_vec()], b"value".to_vec(), ), + // A plain write alongside, so the batch options are also + // forwarded as insert options on this path. + QualifiedGroveDbOp::insert_or_replace_op( + vec![TEST_LEAF.to_vec()], + b"extra".to_vec(), + Element::new_item(b"x".to_vec()), + ), ], batch_flag_on(), None, @@ -2898,7 +2905,19 @@ fn typed_deletes_through_apply_operations_without_batching_match_live() { .delete(&[TEST_LEAF], b"value", None, None, grove_version) .unwrap() .unwrap(); + live_db + .insert( + &[TEST_LEAF], + b"extra", + Element::new_item(b"x".to_vec()), + flag_on(), + None, + grove_version, + ) + .unwrap() + .unwrap(); assert_present(&batch_db, b"r1", grove_version); + assert_present(&batch_db, b"extra", grove_version); assert_eq!( batch_db.root_hash(None, grove_version).unwrap().unwrap(), live_db.root_hash(None, grove_version).unwrap().unwrap()