-
Notifications
You must be signed in to change notification settings - Fork 54
refactor(drive): unify AVG no-prove dispatch into a single count+sum walk #3690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
f945e00
refactor(drive): unify AVG no-prove dispatch into a single count+sum …
QuantumExplorer e7be89d
refactor(drive): enforce mode×shape pairing in joint count+sum dispat…
QuantumExplorer 0f6512e
fix(drive): close AVG no-proof amplification + honor request.limit (#…
QuantumExplorer 5ed1f0d
fix(drive): tighten AVG no-proof API contract per PR #3690 review
QuantumExplorer 3a1cea1
docs(drive): rewrite AVG no-proof chapter section as architecture-cur…
QuantumExplorer f9c70f3
docs(drive): refresh joint count+sum module/dispatcher docstrings
QuantumExplorer a4c70da
docs(drive): clarify joint dispatch chapter section per CodeRabbit nits
QuantumExplorer 8dbe4a1
refactor(drive): tighten joint AVG dispatcher per PR #3690 review
QuantumExplorer dcc3824
feat(drive): collapse AVG no-proof aggregate range to single grovedb …
QuantumExplorer 866b74f
docs(drive): drop remaining stale "two calls" / "pre-#3687" references
QuantumExplorer 32602ee
docs(drive): drop stale "count and sum accumulator calls" comment
QuantumExplorer 8b829e9
chore(drive): bump grovedb pin to develop head (post-#676 merge)
QuantumExplorer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1,115 changes: 787 additions & 328 deletions
1,115
packages/rs-drive/src/query/drive_document_average_query/drive_dispatcher.rs
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
248 changes: 248 additions & 0 deletions
248
packages/rs-drive/src/query/drive_document_count_and_sum_query/drive_dispatcher.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,248 @@ | ||
| //! Joint count-and-sum dispatcher entry point for the AVG no-prove | ||
| //! path. | ||
| //! | ||
| //! Resolves [issue #3687](https://github.com/dashpay/platform/issues/3687): | ||
| //! consumes a [`DocumentAverageRequest`] with `prove = false` and | ||
| //! routes it through one of three joint per-mode executors | ||
| //! (`Total` / `PerInValue` / `RangeNoProof`), each of which walks | ||
| //! grovedb once and reads `(count, sum)` from every visited | ||
| //! count-sum-bearing element. The dispatcher delegates routing to | ||
| //! sum's versioned mode-detection table — see [the routing | ||
| //! table](crate::query::drive_document_sum_query::mode_detection) for | ||
| //! the contract. | ||
| //! | ||
| //! # Perf characteristic | ||
| //! | ||
| //! One grovedb walk per AVG no-prove query (compared to the pre-#3687 | ||
| //! composition of two parallel walks zipped at the dispatcher). The | ||
| //! `PerInValue` and compound-aggregate range branches still issue | ||
| //! multiple per-In sub-reads, but each of those is a single grovedb | ||
| //! call yielding `(count, sum)` together rather than the two parallel | ||
| //! calls the composition did before. | ||
| //! | ||
| //! # Atomicity | ||
| //! | ||
| //! - `Total` and the distinct `RangeNoProof` branch issue exactly one | ||
| //! grovedb read each; atomicity is inherent (one read sees one | ||
| //! snapshot). | ||
| //! - The `PerInValue` branch and (under the unified walk) the | ||
| //! compound-aggregate `RangeNoProof` branch issue multiple per-In | ||
| //! reads. Their executors open a short-lived shared read transaction | ||
| //! internally when `transaction.is_none()` so the per-In reads see a | ||
| //! consistent snapshot. The previous AVG dispatcher's shared-tx | ||
| //! plumbing moved into those executors. | ||
|
|
||
| use super::super::drive_document_average_query::{ | ||
| AverageMode, DocumentAverageRequest, DocumentAverageResponse, | ||
| }; | ||
| use super::super::drive_document_sum_query::mode_detection::detect_sum_mode_from_inputs; | ||
| use super::super::drive_document_sum_query::{DocumentSumMode, RangeSumOptions, SumMode}; | ||
| use crate::drive::Drive; | ||
| use crate::error::Error; | ||
| use dpp::data_contract::accessors::v0::DataContractV0Getters; | ||
| use dpp::data_contract::document_type::accessors::DocumentTypeV0Getters; | ||
| use dpp::version::PlatformVersion; | ||
| use grovedb::TransactionArg; | ||
|
|
||
| #[cfg(feature = "server")] | ||
| impl Drive { | ||
| /// Joint count-and-sum no-prove dispatcher. | ||
| /// | ||
| /// Consumes a [`DocumentAverageRequest`] with `prove = false` and | ||
| /// produces a [`DocumentAverageResponse`] in the appropriate shape | ||
| /// (`Aggregate { count, sum }` for non-grouped modes, | ||
| /// `Entries(Vec<AverageEntry>)` for grouped modes). | ||
| /// | ||
| /// The dispatcher returns `Proof(_)`-shape responses only when | ||
| /// invoked via [`Drive::execute_document_average_request`]'s | ||
| /// `prove = true` arm; the no-prove path here never produces proof | ||
| /// bytes. | ||
| /// | ||
| /// # Routing | ||
| /// | ||
| /// Reuses sum's versioned routing table via | ||
| /// [`detect_sum_mode_from_inputs`] with the request's | ||
| /// [`AverageMode`] converted 1:1 to [`SumMode`] (the two enums are | ||
| /// structurally identical — same four variants). `prove = false` | ||
| /// is passed unconditionally because the prove path never reaches | ||
| /// this function. The resolved [`DocumentSumMode`] is one of three | ||
| /// no-prove values: `Total`, `PerInValue`, or `RangeNoProof`; the | ||
| /// four prove-mode values are unreachable. The match arm therefore | ||
| /// returns `CorruptedCodeExecution` on those branches rather than | ||
| /// silently misbehaving — if the routing table is ever extended | ||
| /// with a new prove-only mode that accidentally fires here, we | ||
| /// want to fail loudly. | ||
| pub fn execute_document_count_and_sum_request( | ||
| &self, | ||
| request: DocumentAverageRequest, | ||
| transaction: TransactionArg, | ||
| platform_version: &PlatformVersion, | ||
| ) -> Result<DocumentAverageResponse, Error> { | ||
| // Convert AverageMode → SumMode (1:1 by construction); sum's | ||
| // routing table is the single source of truth for the | ||
| // `(where_clauses × mode × prove=false)` triple. Forking a | ||
| // parallel "count_and_sum mode" table here is what #3687 | ||
| // explicitly warned against — PR #3661 caught one drift bug | ||
| // (count's `GroupByCompound` row had diverged from sum's), | ||
| // and the joint executor's existence is itself the reason a | ||
| // single source of truth is now mandatory. | ||
| let sum_mode = match request.mode { | ||
| AverageMode::Aggregate => SumMode::Aggregate, | ||
| AverageMode::GroupByIn => SumMode::GroupByIn, | ||
| AverageMode::GroupByRange => SumMode::GroupByRange, | ||
| AverageMode::GroupByCompound => SumMode::GroupByCompound, | ||
| }; | ||
|
|
||
| let resolved_mode = | ||
| detect_sum_mode_from_inputs(&request.where_clauses, sum_mode, false, platform_version)?; | ||
|
|
||
| let contract_id = request.contract.id().to_buffer(); | ||
| let document_type_name = request.document_type.name().to_string(); | ||
| let where_clauses = request.where_clauses; | ||
| let sum_property = request.sum_property; | ||
| let order_by_ascending = request | ||
| .order_clauses | ||
| .first() | ||
| .map(|c| c.ascending) | ||
| .unwrap_or(true); | ||
|
|
||
| match resolved_mode { | ||
| DocumentSumMode::Total => self.execute_document_count_and_sum_total_no_proof( | ||
| contract_id, | ||
| request.document_type, | ||
| document_type_name, | ||
| where_clauses, | ||
| sum_property, | ||
| transaction, | ||
| platform_version, | ||
| ), | ||
| DocumentSumMode::PerInValue => { | ||
| let options = RangeSumOptions { | ||
| return_distinct_sums_in_range: false, | ||
| carrier_outer_limit: None, | ||
| left_to_right: order_by_ascending, | ||
| }; | ||
| self.execute_document_count_and_sum_per_in_value_no_proof( | ||
| contract_id, | ||
| request.document_type, | ||
| document_type_name, | ||
| where_clauses, | ||
| sum_property, | ||
| options, | ||
| transaction, | ||
| platform_version, | ||
| ) | ||
| } | ||
| DocumentSumMode::RangeNoProof => { | ||
| // Distinct flag set for the GroupByRange / | ||
| // GroupByCompound shapes (per-distinct-value entries); | ||
| // cleared for the Aggregate / GroupByIn shapes (single | ||
| // folded pair). Mirrors sum's dispatcher. | ||
| let return_distinct = matches!( | ||
| request.mode, | ||
| AverageMode::GroupByRange | AverageMode::GroupByCompound | ||
| ); | ||
| let options = RangeSumOptions { | ||
| return_distinct_sums_in_range: return_distinct, | ||
| carrier_outer_limit: None, | ||
| left_to_right: order_by_ascending, | ||
| }; | ||
| let response = self.execute_document_count_and_sum_range_no_proof( | ||
| contract_id, | ||
| request.document_type, | ||
| document_type_name, | ||
| where_clauses, | ||
| sum_property, | ||
| options, | ||
| transaction, | ||
| platform_version, | ||
| )?; | ||
| // Sum's dispatcher applies a second-stage shape on the | ||
| // RangeNoProof path: `Aggregate` mode collapses to a | ||
| // single value, every other mode (GroupByIn / | ||
| // GroupByRange / GroupByCompound) returns `Entries`. | ||
| // GroupByIn + range specifically returns | ||
| // `Entries(vec![one_entry])` with the In axis folded | ||
| // (sum's executor itself emits exactly one entry in | ||
| // that case — see its `execute_range_sum_no_proof` | ||
| // compound-summed branch). The joint executor returns | ||
| // `Aggregate { count, sum }` for the !distinct shape | ||
| // because it has both metrics on hand; we re-shape into | ||
| // `Entries(vec![one_entry])` here when the caller asked | ||
| // for a non-Aggregate mode so the wire shape matches | ||
| // what an independent sum + count GroupByIn dispatch | ||
| // would produce. Without this re-shape, GroupByIn + | ||
| // range AVG silently changes wire shape from `Entries` | ||
| // (pre-#3687) to `Aggregate`. | ||
| // Strict mode×shape pairing. Every legal combination | ||
| // is named explicitly; any other pairing surfaces as | ||
| // `CorruptedCodeExecution` rather than silently | ||
| // forwarding a wrong-shape response. A future executor | ||
| // change that emits, say, `Entries` from an | ||
| // `AverageMode::Aggregate` request would fail loudly | ||
| // here instead of leaking the wrong wire shape to the | ||
| // gRPC handler. | ||
| match (request.mode, response) { | ||
| (AverageMode::Aggregate, resp @ DocumentAverageResponse::Aggregate { .. }) => { | ||
| Ok(resp) | ||
| } | ||
| (AverageMode::GroupByIn, DocumentAverageResponse::Aggregate { count, sum }) => { | ||
| Ok(DocumentAverageResponse::Entries(vec![ | ||
| super::super::drive_document_average_query::AverageEntry { | ||
| in_key: None, | ||
| key: Vec::new(), | ||
| count: Some(count), | ||
| sum: Some(sum), | ||
| }, | ||
| ])) | ||
| } | ||
| ( | ||
| AverageMode::GroupByRange | AverageMode::GroupByCompound, | ||
| resp @ DocumentAverageResponse::Entries(_), | ||
| ) => Ok(resp), | ||
| (mode, _) => Err(Error::Drive( | ||
| crate::error::drive::DriveError::CorruptedCodeExecution(match mode { | ||
| AverageMode::Aggregate => { | ||
| "execute_document_count_and_sum_request: \ | ||
| RangeNoProof executor emitted a non-Aggregate \ | ||
| response for AverageMode::Aggregate — joint \ | ||
| range executor's shape contract violated" | ||
| } | ||
| AverageMode::GroupByIn => { | ||
| "execute_document_count_and_sum_request: \ | ||
| RangeNoProof executor emitted a non-Aggregate \ | ||
| response for AverageMode::GroupByIn — the \ | ||
| in-axis fold should yield a single (count, sum) \ | ||
| pair the dispatcher re-wraps as Entries" | ||
| } | ||
| AverageMode::GroupByRange | AverageMode::GroupByCompound => { | ||
| "execute_document_count_and_sum_request: \ | ||
| RangeNoProof executor emitted a non-Entries \ | ||
| response for a distinct grouped mode — joint \ | ||
| range executor's shape contract violated" | ||
| } | ||
| }), | ||
| )), | ||
| } | ||
| } | ||
| // The four prove-mode resolutions are unreachable here — | ||
| // the dispatcher passes `prove = false` to the routing | ||
| // table, and sum's v0 table has no row that maps a | ||
| // `prove=false` input to any of these four resolutions. | ||
| // Surface as `CorruptedCodeExecution` so a future routing- | ||
| // table change that breaks this assumption fails loudly | ||
| // rather than producing a runtime panic. | ||
| DocumentSumMode::RangeProof | ||
| | DocumentSumMode::RangeDistinctProof | ||
| | DocumentSumMode::PointLookupProof | ||
| | DocumentSumMode::RangeAggregateCarrierProof => Err(Error::Drive( | ||
| crate::error::drive::DriveError::CorruptedCodeExecution( | ||
| "execute_document_count_and_sum_request: sum's routing table \ | ||
| resolved a prove-only mode from a `prove = false` request — \ | ||
| this indicates the routing table has been extended without \ | ||
| updating the joint dispatcher's match arms", | ||
| ), | ||
| )), | ||
| } | ||
| } | ||
| } | ||
21 changes: 21 additions & 0 deletions
21
packages/rs-drive/src/query/drive_document_count_and_sum_query/executors/mod.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| //! Per-`DocumentSumMode` joint count-and-sum no-prove executors. | ||
| //! | ||
| //! Mirrors [`crate::query::drive_document_sum_query::executors`] | ||
| //! one-to-one for the no-prove subset of modes — the prove modes | ||
| //! (`RangeProof`, `RangeDistinctProof`, `PointLookupProof`, | ||
| //! `RangeAggregateCarrierProof`) are unreachable here because the AVG | ||
| //! dispatcher routes prove-true requests through | ||
| //! [`Drive::execute_document_average_prove`] before this module is | ||
| //! consulted. The three no-prove modes are: | ||
| //! | ||
| //! - [`total`] — `DocumentSumMode::Total` (empty-where + | ||
| //! `documents_summable + documents_countable` fast path AND | ||
| //! Equal/In-fully-covered point lookup). | ||
| //! - [`per_in_value`] — `DocumentSumMode::PerInValue` (Equal/In on a | ||
| //! non-range covered index, emitting one entry per In branch). | ||
| //! - [`range_no_proof`] — `DocumentSumMode::RangeNoProof` (range and | ||
| //! distinct shapes over a PCPS-eligible index). | ||
|
|
||
| pub mod per_in_value; | ||
| pub mod range_no_proof; | ||
| pub mod total; |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.