feat: aggregate sum path query#364
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR expands GroveDB and Merk functionalities by integrating new aggregate sum query capabilities. It introduces additional versioning fields, new structs, and methods across multiple modules to support aggregate sum operations. Key changes include the addition of aggregate sum path query methods in version constants, enhanced Element query operations, new error handling, and modifications in iterator validations. The Merk side now supports query insertion and merging logic for aggregate sum queries, and test utilities have been extended to handle new sum tree structures. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant DB as GroveDb
participant Elem as Element
participant Query as AggregateSumQueryOptions
Client->>DB: query_aggregate_sums(aggregate_sum_path_query, options...)
DB->>DB: Perform version check
DB->>Elem: Call get_aggregate_sum_query(QueryOptions)
Elem->>Elem: Process aggregate sum query logic
Elem-->>DB: Return SumItem results
DB-->>Client: Return aggregated query results
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
grovedb/src/tests/mod.rs (1)
125-146: Indentation inconsistency detectedThe indentation for the
.unwrap()and.expect()calls in this function differs from the indentation style used in the similar functionadd_test_leaves(lines 102-123).- .unwrap() - .expect("successful root tree leaf insert"); + .unwrap() + .expect("successful root tree leaf insert"); - .unwrap() - .expect("successful root tree leaf 2 insert"); + .unwrap() + .expect("successful root tree leaf 2 insert");merk/src/proofs/aggregate_sum_query/insert.rs (5)
1-5: Use consistent feature guards for clarity.
While feature gating is helpful, consider clarifying the relationship between"minimal"and"verify"features. If further down the line you add more complex logic that also depends on these features, it may be beneficial to centralize shared code under a single, more descriptive feature gate or utilize separate ones for clarity.
27-32: Methodinsert_keys: Optimize for large input sets if necessary.
Repeatedly callingself.insert_item()in a loop is fine for smaller key sets. However, consider using a more efficient approach (like sorting and merging) if this is used with a large collection of keys.
127-135: Methodinsert_all: Discarding existing items.
UsingQueryItem::RangeFullto discard pre-existing items is straightforward, but ensure that downstream consumers understand that callinginsert_allwill reset all prior constraints. This can otherwise lead to confusion if the caller is unaware of the discard.
137-169: Methodinsert_item: Potential performance bottleneck.
The current approach recreatesself.itemsvia an iterator/filter for every insertion, which could be costly with very large queries. While likely fine for moderate usage, consider a more efficient data structure (e.g., a balanced tree or interval tree) if performance becomes critical.Additionally, the
unreachable!()branch might be triggered in unexpected corner cases if new collision logic is introduced. It's fine short term, but keep in mind it might lead to app crashes without graceful handling of unexpected states.
172-176: Methodinsert_items: Avoid partial insert states if an insertion fails.
If a future insertion can fail (e.g., memory issues, data validation, or advanced logic), consider whether you want a partial set of items inserted or an atomic approach that reverts the entire insertion. Right now, an error partway won't revert earlier successful insertions.merk/src/proofs/aggregate_sum_query/mod.rs (7)
1-2: Organize module imports.
These submodules (merge,insert) are a sizable part of the query logic. If their size grows, consider moving them into a dedicated folder-like structure to keep them logically grouped and more discoverable.
9-11: Clarify doc comment forAggregateSumQuery.
The doc line states “resolve a proof which will include all the requested values.” Consider clarifying it also includes partial sums up tosum_limitif relevant. Right now, the mention ofsum_limitis in the struct fields but not in the doc comment.
26-36:Displaytrait implementation: Provide optional new lines or indentation.
Your current implementation prints new lines between items, which is helpful for debugging, but you might want to ensure the output remains consistent even if the query is large. For instance, you could truncate the printed items if the list is too long.
38-58: Constructorsnew&new_range_full: Possible confusion with direction.
Both methods defaultleft_to_right: true. If a user explicitly wants a reverse-ordered query, they might accidentally picknewand get the wrong behavior. Consider grouping or naming them more distinctly (likenew_ascending_range_full,new_descending_range_full) for clarity.
81-89: Multiple constructor variants: Potential duplication of logic.
You have many constructors that differ slightly only by initialitemsor the direction. To reduce code duplication, consider using a helper function that sets the shared fields, then focusing each constructor on key or range specifics. This approach can simplify maintenance.Also applies to: 91-99, 101-109, 111-119, 121-135
137-140: Methodlen: Evaluate naming.
lenis short and direct. If you foresee adding more comprehensive query info (like distinct range counts vs. key counts), you might rename it or add specialized methods (e.g.,total_items()orrange_count()).
164-168: Methodhas_only_keys: Edge case for an empty query.
An empty query trivially satisfieshas_only_keysbecause there are no range items, though logically it has zero keys. If you expect an empty query scenario, confirm it aligns with your usage patterns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
grovedb-version/src/version/grovedb_versions.rs(3 hunks)grovedb-version/src/version/v1.rs(4 hunks)grovedb-version/src/version/v2.rs(4 hunks)grovedb/src/element/aggregate_sum_query.rs(1 hunks)grovedb/src/element/mod.rs(1 hunks)grovedb/src/element/query.rs(1 hunks)grovedb/src/error.rs(1 hunks)grovedb/src/lib.rs(1 hunks)grovedb/src/operations/get/query.rs(2 hunks)grovedb/src/query/aggregate_sum_path_query.rs(1 hunks)grovedb/src/query/mod.rs(4 hunks)grovedb/src/query_result_type.rs(2 hunks)grovedb/src/tests/mod.rs(2 hunks)merk/src/lib.rs(0 hunks)merk/src/merk/mod.rs(1 hunks)merk/src/proofs/aggregate_sum_query/insert.rs(1 hunks)merk/src/proofs/aggregate_sum_query/merge.rs(1 hunks)merk/src/proofs/aggregate_sum_query/mod.rs(1 hunks)merk/src/proofs/mod.rs(1 hunks)merk/src/proofs/query/query_item/mod.rs(1 hunks)
💤 Files with no reviewable changes (1)
- merk/src/lib.rs
🧰 Additional context used
🧬 Code Definitions (3)
grovedb/src/query/mod.rs (2)
grovedb/src/operations/get/query.rs (1)
query(239-279)grovedb/src/operations/proof/util.rs (1)
hex_to_ascii(284-299)
merk/src/proofs/aggregate_sum_query/merge.rs (3)
grovedb/src/query/aggregate_sum_path_query.rs (1)
new(38-40)grovedb/src/query/mod.rs (2)
new(80-86)new(109-111)merk/src/proofs/aggregate_sum_query/mod.rs (1)
new(40-42)
grovedb/src/element/aggregate_sum_query.rs (7)
grovedb/src/query/aggregate_sum_path_query.rs (2)
fmt(24-33)new(38-40)grovedb/src/element/mod.rs (2)
fmt(147-236)fmt(265-270)grovedb/src/element/query.rs (5)
fmt(62-77)fmt(176-228)default(82-89)format_query(114-147)new(1708-1710)grovedb/src/query/mod.rs (6)
fmt(40-49)fmt(66-75)fmt(464-476)fmt(509-523)new(80-86)new(109-111)grovedb/src/query_result_type.rs (7)
fmt(32-42)fmt(53-59)fmt(75-84)fmt(88-92)fmt(440-463)default(423-425)new(143-145)grovedb/src/operations/get/query.rs (1)
query(239-279)merk/src/proofs/mod.rs (1)
hex_to_ascii(120-126)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Compilation errors
- GitHub Check: Tests
- GitHub Check: Linting
- GitHub Check: Code Coverage
🔇 Additional comments (69)
grovedb-version/src/version/grovedb_versions.rs (4)
8-8: Well-structured addition of aggregate sum path query supportThe addition of
aggregate_sum_path_query_methodsto theGroveDBVersionsstruct properly integrates the new feature into the versioning system, ensuring backward compatibility.
14-17: Good structure for versioning the new featureThe new
GroveDBAggregateSumPathQueryMethodVersionsstruct follows the established pattern for versioning feature capabilities, with themergefield allowing for proper version handling.
98-98: Consistent versioning for query operationsAdding
query_aggregate_sumstoGroveDBOperationsQueryVersionsmaintains consistency with the existing versioning pattern for query operations.
220-220: Complete versioning for element methodsAll necessary method versions for the aggregate sum functionality have been added to
GroveDBElementMethodVersions, covering the full range of operations: querying, applying functions, pushing to path queries, and handling items.Also applies to: 225-225, 227-227, 230-230, 234-234
grovedb/src/query_result_type.rs (2)
18-18: Good import organizationClean import of
SumValuefrom the element module, keeping it separate for clarity.
495-497: Consistent type alias patternThe
KeySumValuePairtype alias follows the established pattern for key-value relationships in the codebase, maintaining consistency with other similar type aliases likeKeyElementPair.grovedb/src/error.rs (1)
162-164: Well-defined error handling for aggregate operationsThe addition of the
Overflowerror variant is appropriate for handling arithmetic overflow scenarios that might occur during aggregate sum operations. The error includes a descriptive string parameter for context, consistent with other error variants.grovedb/src/lib.rs (1)
210-210: Clean public export of the new query typeExporting
AggregateSumPathQueryalongside existing query types makes the new functionality accessible to users while maintaining the established pattern.grovedb/src/element/mod.rs (1)
23-24: LGTM - New module added for aggregate sum query functionalityThe addition of the new
aggregate_sum_querymodule is properly configured with the correct feature flags, consistent with other modules in this file. This module will contain the implementation for the AggregateSumPathQuery functionality described in the PR.merk/src/merk/mod.rs (1)
147-149: Adding parameter to support new aggregate sum functionalityThe method call has been correctly updated to pass an additional
Noneparameter to theiter_is_valid_for_typemethod, aligning with the modified method signature in theQueryItemimplementation.merk/src/proofs/query/query_item/mod.rs (2)
793-797: LGTM - New parameter added to support aggregate sum queryingThe addition of the
aggregate_limitparameter of typeOption<i64>is appropriate for implementing the aggregate sum path query functionality.
804-804: LGTM - Valid logic for aggregate limit validationThe validation logic for the new
aggregate_limitparameter follows the same pattern as the existinglimitparameter validation, ensuring consistency in how different types of limits are handled.grovedb/src/tests/mod.rs (1)
87-100: Looks good: New utility method for creating GroveDB with sum treesThis new helper method follows the established pattern for test utility functions in GroveDB and properly creates a test database with sum trees, which will be useful for testing the new aggregate sum path query functionality.
grovedb/src/element/query.rs (1)
836-837: Method call updated with new parameterThe
iter_is_valid_for_typemethod now takes an additional parameter (None) which is likely related to the new aggregate sum functionality being introduced. This change ensures compatibility with the updated method signature while maintaining backward compatibility by passingNonefor existing query types.grovedb-version/src/version/v2.rs (8)
14-14: Added import for new aggregate sum path query methodsThis import provides access to the
GroveDBAggregateSumPathQueryMethodVersionsstruct, which is necessary for defining version information for the new aggregate sum functionality.
59-59: Added version field for aggregate sum query methodThis new version field properly tracks the implementation version of the
get_aggregate_sum_querymethod, aligning with the PR's objective to introduce aggregate sum query functionality.
64-64: Added version field for aggregate sum query apply functionThis version field tracks the implementation version of the function that applies aggregate sum queries, consistent with the overall feature addition.
66-66: Added version field for aggregate sum path query push methodThis tracks the version of the method that handles pushing aggregate sum path queries, which is a key component of the new functionality.
69-69: Added version field for basic aggregate sum push methodThis tracks the version of the basic implementation for pushing aggregate sum values, maintaining versioning consistency across all related methods.
75-75: Added version field for aggregate sum query item methodThis field tracks the implementation version of the method that processes individual items in aggregate sum queries.
136-136: Added version field for query_aggregate_sums operationThis field tracks the version of the operation that executes aggregate sum queries at the GroveDB level, properly integrating with the versioning system.
185-185: Added aggregate sum path query methods version structThis field incorporates all aggregate sum path query-related methods into the versioning system with initial version 0, ensuring proper version tracking for the new functionality.
grovedb/src/operations/get/query.rs (4)
11-12: Added import for AggregateSumQueryThis import provides access to the necessary aggregate sum query structures from the Merk proofs module, supporting the new functionality.
14-14: Extended imports for aggregate sum related typesAdditional element types like
BigSumValueandCountValueare now imported to support the new aggregate sum functionality.
21-22: Added imports for aggregate sum query options and result typesThese imports provide the specialized options and return types needed for the new aggregate sum query functionality.
492-516:Details
✅ Verification successful
Added new method to query aggregate sums
The
query_aggregate_sumsmethod enables retrieving sum items that match an aggregate sum path query, implementing the core functionality described in the PR objectives. The method properly integrates with the cost tracking system and versioning checks.
🏁 Script executed:
#!/bin/bash # Check if the referenced Element::get_aggregate_sum_query method has been implemented # Search for the get_aggregate_sum_query method implementation echo "Searching for get_aggregate_sum_query implementation..." rg "fn get_aggregate_sum_query" -A 5Length of output: 1174
Verified aggregate sums query implementation – Approved
The implementation of
query_aggregate_sumsingrovedb/src/operations/get/query.rshas been verified to correctly callElement::get_aggregate_sum_query, which is implemented ingrovedb/src/element/aggregate_sum_query.rs. The method appropriately integrates cost tracking and version checks as intended.No further changes are required.
grovedb/src/query/mod.rs (9)
3-4: Add new module for aggregate sum path query
Introducingpub mod aggregate_sum_path_query;is a clear way to encapsulate new functionality for aggregate sum path queries. No issues found here.
12-12: Unconditional bincode usage
Removing any gating attributes for bincode usage aligns with having a more unified codebase. Your unconditional import is fine.
15-15: Unconditional import ofKeyandSubqueryBranch
AllowingKeyandSubqueryBranchto be used without conditional compilation is consistent with the rest of this file’s approach.
21-21: Exposehex_to_asciiutility unconditionally
Removing the feature gate forhex_to_asciiis consistent with the code’s broader usage.
23-23: Unconditional import of crate::Error
This is in line with the remainder of the file, ensuring errors are always accessible.
454-461: MakeHasSubqueryalways available
Unconditionally deriving and exposingHasSubquerylooks good. This helps keep the code straightforward and feature-flag free in this region.
462-462: Unconditionalimpl fmt::Display for HasSubquery
The display implementation is simple, and removing conditional compilation here is consistent with the newly unified approach.
495-495: MakeSinglePathSubqueryalways available
This struct is now consistently exposed. Looks good.
507-507: Unconditionalimpl fmt::Display for SinglePathSubquery
Similarly, no issues with removing the feature gate. The display logic remains readable.merk/src/proofs/aggregate_sum_query/merge.rs (3)
1-2: New imports forErrorandAggregateSumQuery
Bringing these types in is straightforward. No issues here.
4-42:merge_multiplemethod logic
Merging multiple queries while enforcing the sameleft_to_rightvalue and performing overflow checks onsum_limitandlimit_of_items_to_checkis carefully handled. Returning an error for mismatches or overflow scenarios is appropriate. The approach of producingNoneforlimit_of_items_to_checkwhen any input isNonealso appears reasonable for an unbounded stance.
44-66:merge_withmethod considerations
Similarly merges sum limits with proper overflow checks and merges item limits. Inserting items from theotherquery is neatly placed. No concurrency or logical pitfalls stand out.merk/src/proofs/mod.rs (1)
9-10: Addaggregate_sum_querymodule
This supports the new aggregate sum functionality. Looks consistent with the broader effort in this PR.grovedb-version/src/version/v1.rs (4)
14-14: New import usage is aligned with introduced functionality.Importing
GroveDBAggregateSumPathQueryMethodVersionslooks consistent with the new methods introduced below and doesn't present any immediate issues.
59-75: New version fields align well with existing naming conventions.These added version references:
get_aggregate_sum_queryget_aggregate_sum_query_apply_functionaggregate_sum_path_query_pushbasic_aggregate_sum_pushaggregate_sum_query_itemare consistent with the established field naming scheme in
GroveDBElementMethodVersions. This appears to be the correct approach for integrating new query-related methods into the versioning system, ensuring version compatibility checks can properly handle them.
136-136: Field name is consistent with existing query operations.Adding
query_aggregate_sumsinGroveDBOperationsQueryVersionsfollows the same naming style as other query operations. This helps maintain clarity and consistency across the codebase.
185-185: New aggregate sum path query method version entry is coherent.Including
aggregate_sum_path_query_methodsinGroveDBVersionsis the appropriate way to keep the new merge logic encapsulated and version-controlled. The naming is succinct and mirrors existing patterns.grovedb/src/query/aggregate_sum_path_query.rs (7)
1-9: Imports appear necessary and well-scoped.All imported crates, modules, and traits (e.g.,
Decode,Encode,AggregateSumQuery,QueryItem, etc.) align with the file’s functionality. No redundancy is apparent, and the usage of version checks is consistent with the approach in the rest of GroveDB code.
10-21: Struct definition is clear and well-documented.
- The public fields
pathandaggregate_sum_queryare descriptive.- Documentation comments adequately explain purpose and usage.
23-34:fmt::Displayimplementation is straightforward and beneficial for debugging.
- Using
hex_to_asciito render path elements is consistent with other parts of the code.- The format string is well-structured; errors are properly handled.
36-41:newconstructor is concise and clear.Returning
Self { path, aggregate_sum_query }is idiomatic. No issues found.
42-48:new_single_keyconvenience constructor is helpful.Use of
AggregateSumQuery::new_single_keyaligns well with the design, simplifying single-key usage.
50-56:new_single_query_itemconstructor extends functionality for more complex queries.Provides an easy interface for adding one
QueryItemand optional limit. Implementation looks correct.
58-99:mergemethod logic is robust and well-validated.
- Checking for empty input and ensuring all paths match is good defensive programming.
- Merging multiple
AggregateSumQueryobjects cleanly delegates logic tomerge_multiple.- Error handling for mismatched paths or empty queries is consistent with GroveDB’s approach.
grovedb/src/element/aggregate_sum_query.rs (14)
1-9: Module-level comments and imports are logically grouped.The doc comments effectively explain that this file implements aggregate sum querying in the
Elementstruct. The imported modules (e.g.,grovedb_merk,grovedb_storage) are appropriate for the sum query functionality.
32-37:AggregateSumQueryOptionsstruct encapsulates query behavior well.Storing flags for raw retrieval, cache, and intermediate path-tree behavior helps ensure flexible query execution.
39-51:fmt::DisplayforAggregateSumQueryOptionsis coherent.
- The additional indentation and labeled fields make debugging more transparent.
- No performance concerns for debug usage.
53-61: Default implementation sets cautious and safe behavior.
allow_cache: trueis usually beneficial for performance.error_if_intermediate_path_tree_not_present: trueensures explicit control over absent subtrees.
63-80:AggregateSumPathQueryPushArgsstruct is straightforward.
- Captures transaction, storage context, and limiting parameters for sum queries.
- The
'db: 'ctxlifetime constraints align with typical Rust usage in DB contexts.
81-97: Helper functionformat_queryaids debugging.The structured formatting of items in
AggregateSumQueryprovides clarity for logs or error messages.
99-135: Display implementation forAggregateSumPathQueryPushArgsis consistent with GroveDB’s approach.
- Renders all struct fields in a thorough, human-readable format.
- Hex-encoding for keys is consistent across the codebase.
137-166:get_aggregate_sum_querymethod integrates version checks and delegated logic.
- Good structure for bridging path-based queries to the generic apply function.
- No immediate correctness issues identified.
168-250:get_aggregate_sum_query_apply_functionhandles iteration logic effectively.
- Splitting the ascending and descending iteration via
left_to_rightis clean.- Properly breaks early if
sum_limitorlimit_of_items_to_checkare reached, preventing unnecessary overhead.- The use of
CostResultfor error handling is consistent with GroveDB code.
252-282:aggregate_sum_path_query_pushensures sum items are handled properly.
- Early check for
SumItemelements avoids type mismatches.- The delegation to
basic_aggregate_sum_pushis logically separate, improving maintainability.
284-290: Inline documentation clarifies the performance-related rationale.
- Explanation of expensive queries without limit reduction is relevant for sum-based workloads.
291-463:aggregate_sum_query_itemmerges iteration approach with robust error handling.
- Distinguishes between key-based queries and range queries.
- Skips or short-circuits gracefully depending on
error_if_intermediate_path_tree_not_present.- Thorough checks to avoid partial or inconsistent results.
465-499:basic_aggregate_sum_pushfinalizes item insertion logic.
- Correctly decrements
limitandsum_limit_left.- Ensures references are converted if needed (
convert_if_reference_to_absolute_reference).
503-1197: Extensive test coverage is commendable.
- Multiple test scenarios (keys, ranges, single items, descending, sub-ranges) confirm correctness.
- The variety of sum limits tested helps minimize regressions.
- Test naming is intuitive and clarifies the expected behavior.
merk/src/proofs/aggregate_sum_query/insert.rs (2)
15-18: Methodinsert_key: Confirm fallback behavior for duplicates.
When adding an individual key that already belongs to an existing range, the code effectively becomes a no-op. This is correct for avoiding duplicates, but ensure your unit tests or usage patterns confirm this is the desired behavior (i.e., new calls won't silently mask other concerns if the key was already covered by a range).
40-43: Insertion of various range types: Validate range overlaps in tests.
All these methods rely onself.insert_itemfor merge logic when overlaps occur. Although the doc comments mention correct behaviors, ensure you have test coverage for scenarios like partially overlapping ranges, adjacent ranges, and identical ranges. Confirmation via robust unit tests will avoid subtle off-by-one or boundary misinterpretations.Also applies to: 51-54, 63-66, 75-78, 87-90, 99-101, 111-113, 123-125
merk/src/proofs/aggregate_sum_query/mod.rs (3)
19-23:sum_limit&limit_of_items_to_check: Confirm protective boundaries.
Consider verifying thatsum_limitremains within valid ranges (e.g., non-negative, or within an upper bound) and thatlimit_of_items_to_checkdoesn't exceed feasible memory constraints. Input validation or checks can help mitigate potential edge cases.
70-79: Methodnew_single_key: Potential edge case with nolimit_of_items_to_check.
Currently it setslimit_of_items_to_checktoSome(1). If the single key does not exist or is out of range, no records are found, and summing might be trivially zero. Ensure that’s indeed the intended fallback.
142-162: Iterators: Confirm consistent iteration order.
You offer standard.iter(),.rev_iter(), and.directional_iter(). Ensure that the logic for reversing is thoroughly tested, especially when mixing single keys and multiple ranges. Some boundary conditions might appear reversed in partial results.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
grovedb/Cargo.toml (1)
23-23: Enforce Exact Version for bincode Dependency
Changing the version constraint to=2.0.0-rc.3ensures that only this exact version is used across builds. This change improves consistency in dependency resolution and minimizes unexpected updates from version ranges.merk/Cargo.toml (1)
20-20: Consistent bincode Version Specification
The update tobincode = { version = "=2.0.0-rc.3" }aligns the dependency version with the configuration in grovedb. This consistency minimizes potential compatibility issues across components. Ensure that this strict version requirement is in line with overall dependency management and that it does not restrict future patch updates that might include necessary fixes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
grovedb/Cargo.toml(1 hunks)merk/Cargo.toml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Compilation errors
- GitHub Check: Linting
- GitHub Check: Tests
- GitHub Check: Code Coverage
Resolve merge conflicts from develop refactoring: - Element system moved to grovedb-element crate (use extension trait pattern) - Op/Node/encoding moved to grovedb-query crate - Dependency versions updated (bincode 2.0.1, blake3 1.8.1) - Query modules restructured into separate files Additional fixes for AggregateSumPathQuery compatibility: - Convert impl Element to ElementAggregateSumQueryExtensions trait - Fix raw_decode import (helpers module removed, use ElementDecodeExtensions) - Fix Error::WrongElementType -> Error::InvalidInput - Make QueryItem::merge_assign public (needed cross-crate) - Add MerkErrorExt import for error conversion - Remove unused format_query and len methods Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Coverage Diff Report
PR coverage: 42397/49255 lines (86.08%) New test functions detected: 44 44 new test functions (click to expand)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #364 +/- ##
===========================================
+ Coverage 86.90% 88.60% +1.70%
===========================================
Files 176 189 +13
Lines 47133 49300 +2167
===========================================
+ Hits 40959 43681 +2722
+ Misses 6174 5619 -555 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Introduces AggregateSumQuery struct with constructors, insert helpers, merge operations, iterators, Display impl, and bincode serialization. Adds Overflow variant to query Error enum. Includes 40 tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The new Overflow variant added to grovedb_query::error::Error was not covered in the From conversion, causing a non-exhaustive match error. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover all merge branches in QueryItem::merge, AggregateSumQuery merge methods, AggregateSumPathQuery::merge, and Display impls to satisfy the codecov/patch 80% threshold. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove 10 tests from grovedb-query/src/query_item/merge.rs that tested pre-existing merge() code (not new in this PR, doesn't help patch coverage). Add 5 new tests in grovedb/src/element/aggregate_sum_query.rs targeting uncovered production paths: PathKeyNotFound handling, limit break in ascending/descending loops, and missing key tolerance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Deduplicate AggregateSumQuery by removing merk copy, re-exporting from grovedb-query - Fix u64→i64 silent truncation of sum_limit with try_into + Overflow error - Fix sum_limit_left subtraction overflow with saturating_sub - Fix limit underflow with saturating_sub - Add early-return guard for exhausted limits (sum_limit <= 0 or limit == 0) - Replace expect() panics on iterator with Error::CorruptedData propagation - Add From<grovedb_query::error::Error> impl for grovedb Error - Add test for zero sum_limit with key query Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… following to AggregateSumQuery - Add skip_items/skip_references options to AggregateSumQueryOptions for silently skipping non-SumItem elements (skipped items still decrement limit) - Add version-gated hard scan limit (GroveDBQueryLimits) to prevent unbounded iteration, returning partial results when reached - Follow references up to 3 hops to resolve target elements instead of erroring - Add GroveDBQueryLimits struct to grovedb-version with max_aggregate_sum_query_elements_scanned - Handle ItemWithSumItem in basic_aggregate_sum_push alongside SumItem - Add comprehensive tests for all new behaviors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…etadata - Rename error_if_non_sum_item_or_reference_found to error_if_non_sum_item_found (remove misleading "or_reference" suffix — references handled by ignore_references) - Expose AggregateSumQueryOptions in public API via query_aggregate_sums_with_options() - Add AggregateSumQueryResult struct with hard_limit_reached flag for truncation detection - Split aggregate_sum_query.rs into module directory (mod.rs + tests.rs) - Rename skip_items/skip_references to error_if_non_sum_item_found/ignore_references - Fix reference hop off-by-one (MAX=3 now allows 3 intermediate hops) - Skipped elements no longer decrement user limit, only system elements_scanned - Remove unused allow_get_raw field - Add coverage gap tests (descending hard limit, descending skip, key query skip with limit) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add tests for query_aggregate_sums and query_aggregate_sums_with_options - Add Display test for AggregateSumPathQueryPushArgs - Remove unused AggregateSumQueryResult import from tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Issue being fixed or feature implemented
We needed a new type of query that would get Sum Items from a tree up to a cumulative aggregate value. For example, if a SumTree has items with values 5, 10, 3, 8, and we want all items until the running total reaches 15, the query would return items summing up to that limit.
What was done?
New types
AggregateSumQuery(merk level): A query type similar toQuerybut with asum_limitfield that specifies when to stop traversing. Supports directional iteration (left-to-right or right-to-left), item count limits, and all existingQueryItemtypes (keys, ranges, etc.).AggregateSumPathQuery(grovedb level): The path-level equivalent ofAggregateSumQuery, wrapping it with a path to target a specific subtree. Supports merging multiple queries together.New operations
GroveDb::aggregate_sum_path_query: Executes an aggregate sum query on a SumTree, returning elements until the cumulative sum reaches the specified limit.AggregateSumQuery: Supports inserting new query items and merging overlapping ranges, mirroring the existingQueryinsert/merge infrastructure.Extension trait pattern
Since the
Elementtype now lives in thegrovedb-elementcrate, the query execution logic uses theElementAggregateSumQueryExtensionstrait to add aggregate sum query methods toElement.Merge with develop
Resolved merge conflicts from significant develop refactoring:
grovedb-elementcrate (adapted with extension trait pattern)grovedb-querycrateHow Has This Been Tested?
test_aggregate_sum_path_query_basic— basic sum limit query on a SumTreetest_aggregate_sum_path_query_with_limit— sum limit + item count limittest_aggregate_sum_path_query_descending— right-to-left traversaltest_aggregate_sum_path_query_key_not_found— querying a non-existent key-D warningsBreaking Changes
None — this is purely additive.
QueryItem::merge_assignwas madepub(previouslypub(crate)) to support cross-crate usage.Checklist:
For repository code-owners and collaborators only