diff --git a/packages/rs-drive/src/query/drive_document_average_query/drive_dispatcher.rs b/packages/rs-drive/src/query/drive_document_average_query/drive_dispatcher.rs index 4810e4ea973..526a5c86b57 100644 --- a/packages/rs-drive/src/query/drive_document_average_query/drive_dispatcher.rs +++ b/packages/rs-drive/src/query/drive_document_average_query/drive_dispatcher.rs @@ -1322,7 +1322,7 @@ mod tests { /// Flat-summed range cross-check: `Aggregate + range` on a PCPS /// index resolves to `DocumentSumMode::RangeNoProof` with - /// `return_distinct_sums_in_range = false`. The joint executor + /// `walk_mode = RangeSumWalkMode::Aggregate`. The joint executor /// folds visited PCPS elements via `count_sum_value_or_default()` /// in Rust (no engine-side combined accumulator exists). Pin parity /// vs. the independent count + sum aggregate dispatch — this is the @@ -1409,7 +1409,7 @@ mod tests { /// Compound-summed range cross-check: `GroupByIn + In + range` on /// a PCPS index resolves to `DocumentSumMode::RangeNoProof` with - /// `return_distinct_sums_in_range = false`. The joint executor's + /// `walk_mode = RangeSumWalkMode::Aggregate`. The joint executor's /// distinct path query expresses the multi-In outer walk as a /// single grovedb call (atomicity inherent) and folds each /// In-branch's PCPS elements into one `(count, sum)` pair via diff --git a/packages/rs-drive/src/query/drive_document_sum_query/drive_dispatcher.rs b/packages/rs-drive/src/query/drive_document_sum_query/drive_dispatcher.rs index 90ca4fbbb92..68ce1b2aad9 100644 --- a/packages/rs-drive/src/query/drive_document_sum_query/drive_dispatcher.rs +++ b/packages/rs-drive/src/query/drive_document_sum_query/drive_dispatcher.rs @@ -13,7 +13,8 @@ use crate::drive::Drive; use crate::error::Error; use crate::query::drive_document_sum_query::{ - DocumentSumMode, DocumentSumRequest, DocumentSumResponse, RangeSumOptions, SumMode, + DocumentSumMode, DocumentSumRequest, DocumentSumResponse, RangeSumOptions, RangeSumWalkMode, + SumMode, }; use crate::query::{OrderClause, WhereClause}; use dpp::data_contract::accessors::v0::DataContractV0Getters; @@ -22,6 +23,27 @@ use dpp::platform_value::Value; use dpp::version::PlatformVersion; use grovedb::TransactionArg; +fn effective_no_proof_distinct_limit( + requested_limit: Option, + drive_config: &crate::config::DriveConfig, +) -> Result { + let effective_limit = requested_limit + .unwrap_or(drive_config.default_query_limit as u32) + .min(drive_config.max_query_limit as u32); + + if effective_limit == 0 { + return Err(Error::Query( + crate::error::query::QuerySyntaxError::InvalidLimit( + "effective distinct SUM limit must be greater than zero".to_string(), + ), + )); + } + + // Both configuration limits are u16, and the `min` above bounds every + // caller-supplied value to `max_query_limit` before this conversion. + Ok(effective_limit as u16) +} + #[cfg(feature = "server")] impl Drive { /// Server-side entry point for the sum surface. Routes a @@ -65,7 +87,7 @@ impl Drive { } DocumentSumMode::PerInValue => { let options = RangeSumOptions { - return_distinct_sums_in_range: false, + walk_mode: RangeSumWalkMode::Aggregate, carrier_outer_limit: None, left_to_right: order_by_ascending, }; @@ -87,8 +109,16 @@ impl Drive { request.mode, SumMode::GroupByRange | SumMode::GroupByCompound ); + let walk_mode = if return_distinct { + RangeSumWalkMode::Distinct(effective_no_proof_distinct_limit( + request.limit, + request.drive_config, + )?) + } else { + RangeSumWalkMode::Aggregate + }; let options = RangeSumOptions { - return_distinct_sums_in_range: return_distinct, + walk_mode, carrier_outer_limit: None, left_to_right: order_by_ascending, }; @@ -252,3 +282,37 @@ pub fn where_clauses_from_value(value: &Value) -> Result, Error pub fn order_clauses_from_value(value: &Value) -> Result, Error> { crate::query::drive_document_count_query::drive_dispatcher::order_clauses_from_value(value) } + +#[cfg(test)] +mod tests { + use super::effective_no_proof_distinct_limit; + use crate::config::DriveConfig; + + #[test] + fn no_proof_distinct_limit_uses_the_default_and_clamps_to_the_maximum() { + let config = DriveConfig { + default_query_limit: 25, + max_query_limit: 100, + ..DriveConfig::default() + }; + + assert_eq!( + effective_no_proof_distinct_limit(None, &config).unwrap(), + 25 + ); + assert_eq!( + effective_no_proof_distinct_limit(Some(7), &config).unwrap(), + 7 + ); + assert_eq!( + effective_no_proof_distinct_limit(Some(10_000), &config).unwrap(), + 100 + ); + + let disabled = DriveConfig { + max_query_limit: 0, + ..config + }; + assert!(effective_no_proof_distinct_limit(None, &disabled).is_err()); + } +} diff --git a/packages/rs-drive/src/query/drive_document_sum_query/execute_range_sum.rs b/packages/rs-drive/src/query/drive_document_sum_query/execute_range_sum.rs index 2696fd519f7..baeb52c1c3d 100644 --- a/packages/rs-drive/src/query/drive_document_sum_query/execute_range_sum.rs +++ b/packages/rs-drive/src/query/drive_document_sum_query/execute_range_sum.rs @@ -12,7 +12,7 @@ //! regular range proof against the `ProvableSumTree`, returning //! per-key `KVSum` ops bound to the merk root. -use super::{DriveDocumentSumQuery, RangeSumOptions, SumEntry}; +use super::{DriveDocumentSumQuery, RangeSumOptions, RangeSumWalkMode, SumEntry}; use crate::drive::Drive; use crate::error::query::QuerySyntaxError; use crate::error::Error; @@ -50,7 +50,7 @@ impl DriveDocumentSumQuery<'_> { .iter() .any(|wc| wc.operator == WhereOperator::In); - if !options.return_distinct_sums_in_range { + if matches!(options.walk_mode, RangeSumWalkMode::Aggregate) { if has_in_on_prefix { // Enforce exactly one `In` clause. Without this, a request // with multiple In filters would silently use only the @@ -148,13 +148,14 @@ impl DriveDocumentSumQuery<'_> { }]); } - // Distinct mode. Mirror count's analog; currently relies on - // `distinct_sum_path_query` which is stubbed (pending port). - // Defer to the same builder so the error surfaces cleanly when - // distinct mode is requested before the builder body lands. - let (path_query_limit, left_to_right) = (None::, options.left_to_right); - let path_query = - self.distinct_sum_path_query(path_query_limit, left_to_right, platform_version)?; + let RangeSumWalkMode::Distinct(distinct_limit) = options.walk_mode else { + unreachable!("aggregate range sums return before the distinct storage walk") + }; + let path_query = self.distinct_sum_path_query( + Some(distinct_limit), + options.left_to_right, + platform_version, + )?; let base_path_len = path_query.path.len(); let mut drive_operations = vec![]; diff --git a/packages/rs-drive/src/query/drive_document_sum_query/executors/range_no_proof.rs b/packages/rs-drive/src/query/drive_document_sum_query/executors/range_no_proof.rs index 2eada0792e9..be1ad9c44f7 100644 --- a/packages/rs-drive/src/query/drive_document_sum_query/executors/range_no_proof.rs +++ b/packages/rs-drive/src/query/drive_document_sum_query/executors/range_no_proof.rs @@ -15,7 +15,7 @@ use grovedb::TransactionArg; impl Drive { /// Range-sum walk against a `rangeSummable: true` index. Returns /// a summed entry or per-distinct-value entries depending on - /// `options.return_distinct_sums_in_range`. + /// `options.walk_mode`. #[allow(clippy::too_many_arguments)] pub fn execute_document_sum_range_no_proof( &self, diff --git a/packages/rs-drive/src/query/drive_document_sum_query/mod.rs b/packages/rs-drive/src/query/drive_document_sum_query/mod.rs index 8b48a5ab122..c1fb5d6fe6b 100644 --- a/packages/rs-drive/src/query/drive_document_sum_query/mod.rs +++ b/packages/rs-drive/src/query/drive_document_sum_query/mod.rs @@ -261,14 +261,24 @@ pub struct DriveDocumentSumQuery<'a> { pub sum_property: String, } +/// Storage-walk shape for a server-side range sum. +#[cfg(feature = "server")] +#[derive(Clone, Copy, Debug, Default)] +pub enum RangeSumWalkMode { + /// Return one aggregate sum for the range. + #[default] + Aggregate, + /// Return distinct sums, bounded by the supplied storage-walk limit. + Distinct(u16), +} + /// Server-side range-sum executor options, parallels /// [`crate::query::drive_document_count_query::RangeCountOptions`]. #[cfg(feature = "server")] #[derive(Clone, Debug, Default)] pub struct RangeSumOptions { - /// When `true`, emit one `SumEntry` per distinct in-range value - /// rather than a single `Aggregate(i64)`. - pub return_distinct_sums_in_range: bool, + /// Select aggregate execution or a compile-time bounded distinct walk. + pub walk_mode: RangeSumWalkMode, /// `Some(n)` caps the carrier walk for compound `(In, range)` /// shapes at n entries. `None` accepts the platform-wide /// `MAX_CARRIER_AGGREGATE_OUTER_RANGE_LIMIT`. diff --git a/packages/rs-drive/src/query/drive_document_sum_query/tests.rs b/packages/rs-drive/src/query/drive_document_sum_query/tests.rs index 46a7deb18bc..6af4716b682 100644 --- a/packages/rs-drive/src/query/drive_document_sum_query/tests.rs +++ b/packages/rs-drive/src/query/drive_document_sum_query/tests.rs @@ -557,4 +557,67 @@ mod limit_policy_regression { "error must name the rejected limit; got: {msg}" ); } + + #[test] + fn range_distinct_sum_no_proof_applies_default_explicit_and_max_limits() { + let drive = setup_drive_with_initial_state_structure(None); + let platform_version = PlatformVersion::latest(); + let data_contract = build_widget_contract(); + drive + .apply_contract( + &data_contract, + BlockInfo::default(), + true, + StorageFlags::optional_default_as_cow(), + None, + platform_version, + ) + .expect("apply contract"); + + for (i, (color, amount)) in [("blue", 2u64), ("green", 3), ("red", 5), ("yellow", 7)] + .iter() + .enumerate() + { + insert_widget(&drive, &data_contract, i, color, *amount); + } + + let document_type = data_contract + .document_type_for_name("widget") + .expect("widget"); + let drive_config = DriveConfig { + default_query_limit: 2, + max_query_limit: 3, + ..Default::default() + }; + let make_request = |limit| DocumentSumRequest { + contract: &data_contract, + document_type, + sum_property: "amount".to_string(), + where_clauses: vec![WhereClause { + field: "color".to_string(), + operator: WhereOperator::GreaterThan, + value: Value::Text("blue".to_string()), + }], + order_clauses: Vec::new(), + mode: SumMode::GroupByRange, + limit, + prove: false, + drive_config: &drive_config, + }; + + for (requested, expected) in [(None, 2), (Some(1), 1), (Some(10_000), 3)] { + let response = drive + .execute_document_sum_request(make_request(requested), None, platform_version) + .expect("bounded no-proof distinct SUM should succeed"); + let entries = match response { + DocumentSumResponse::Entries(entries) => entries, + other => panic!("expected Entries response, got {other:?}"), + }; + assert_eq!( + entries.len(), + expected, + "unexpected entry count for requested limit {requested:?}" + ); + } + } } diff --git a/packages/rs-drive/src/query/mod.rs b/packages/rs-drive/src/query/mod.rs index 21895f8e16a..2c3669f70e3 100644 --- a/packages/rs-drive/src/query/mod.rs +++ b/packages/rs-drive/src/query/mod.rs @@ -42,11 +42,13 @@ pub use drive_document_count_query::{ DocumentCountRequest, DocumentCountResponse, RangeCountOptions, MAX_LIMIT_AS_FAILSAFE, }; -// `DocumentSumRequest` / `DocumentSumResponse` / `RangeSumOptions` are +// `DocumentSumRequest` / `DocumentSumResponse` / range-sum options are // the server-side executor inputs and stay `server`-only (parallels // the count-side `DocumentCountRequest` etc. above). #[cfg(feature = "server")] -pub use drive_document_sum_query::{DocumentSumRequest, DocumentSumResponse, RangeSumOptions}; +pub use drive_document_sum_query::{ + DocumentSumRequest, DocumentSumResponse, RangeSumOptions, RangeSumWalkMode, +}; // `DocumentAverageRequest` / `DocumentAverageResponse` are the // server-side executor inputs for the average surface and stay