feat: keys-only indexed-axis reads - #824
Merged
Merged
Conversation
The resolving reads (indexed_<axis>_{top_k,top_k_paginated,range})
return each entry with its primary value resolved. That resolution
happens after the secondary page was collected, through the caller's
transaction — and a caller that passes None gets point reads outside
the pinned iterator view the page came from, so a primary deleted or
rewritten by a commit in between is reported as corruption or paired
with a page from the older view. Callers that only rank pay up to k
primary reads for values they discard and inherit that window for
nothing (dashpay/platform#4382 review).
Add indexed_{count,sum,avg}_{top_k,top_k_paginated,range}_keys, which
return the (value, original_key) pairs straight from the secondary view
and never open the primary, plus IndexedTopKKeysPage for the paginated
shape. Each read shape now has one _rows_generic core holding all the
pinned-view logic; the resolving wrapper and the keys-only wrapper are
both built on it, so the two cannot disagree on the page. The per-axis
range bound encodings are factored into shared helpers.
Tests: keys-only pages equal the resolving pages projected to their
ranking pairs (including the skipped count) on every axis, both
directions, with and without an offset; range and plain top-k agree
likewise; keys-only reads cost strictly fewer seeks and loaded bytes;
inverted bounds stay the empty answer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
Next review available in: 58 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #824 +/- ##
===========================================
+ Coverage 92.34% 92.39% +0.04%
===========================================
Files 285 288 +3
Lines 87119 87906 +787
===========================================
+ Hits 80449 81219 +770
- Misses 6670 6687 +17
🚀 New features to boost your workflow:
|
The keys-only wrappers were inserted between the doc comment and the function it documents, which newer clippy flags as empty_line_after_doc_comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ction Same insertion slip as the previous commit, at the range-bound helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
added a commit
that referenced
this pull request
Aug 22, 2026
) run_path_query served every ReadMode::Axis read through the resolving indexed-axis reads, so the unified path had no way to ask for the ranking pairs alone: a caller that only ranks paid up to k primary point reads per page — through its own transaction, after the pinned secondary page was collected, so outside the iterator's view when that transaction is None — for values it discards. #824 gave the standalone API a keys-only shape; this gives the PathQuery vocabulary the same. AxisQuery gains `projection: AxisProjection { Entries (default), Keys }`, encoded as a trailing frozen tag byte; `validate` rejects Keys on the traversals that list no entries (rank-of-key, value-range aggregates). run_path_query routes a Keys read through the _keys reads and returns AxisKeys / BranchedAxisKeys (absent branches None, as for entries); AxisEntries::to_keys is the projection. The projection is an unproved-read choice: a proof always carries the values and verification yields entries, so prover and verifier treat Keys exactly as Entries — a keys read is a strict projection of the verified page, which a test pins. PathQuery::new_axis(path, axis_query) builds a read from an already-configured AxisQuery. Tests: keys == entries projected on every axis, both directions, with and without an offset, for ranked-page and bounded traversals; branched keys == branched entries projected including absent branches; keys == verified entries projected; fewer seeks and loaded bytes; rejection on non-listing traversals; projection round-trips with a frozen tag and an unknown tag is rejected. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
added a commit
to dashpay/platform
that referenced
this pull request
Aug 22, 2026
…is reads The ranked and having-range executors only rank — every entry was projected to its (value, key) pair and the resolved primary value discarded — yet the resolving reads paid up to k primary point reads per page through the caller's transaction, outside the pinned iterator view the page came from, so a primary deleted or rewritten by a commit in between could surface as CorruptedData or pair a newer primary with an older page. grovedb's keys-only reads (dashpay/grovedb#824, indexed_*_top_k_paginated_keys / indexed_*_range_keys) return the ranking pairs straight from the secondary view and never open the primary; the executors and the e2e test helpers use them. grovedb pinned to develop head f52adb64, which also carries the append-only storage accounting (dashpay/grovedb#822 via #825/#826: write churn reported as replaced bytes, each note's permanent bytes charged once) and the keys-only projection on the unified PathQuery (#827). The genesis shielded seeding test passes the grove version to the commitment tree's save/commit_mmr, which now take it. The two shielded fee-floor tests (flat fee >= estimated write cost) remain red at this pin: the estimate for a 1-action transfer is now ~397M credits (storage 133M + processing 264M, the epoch's replaced bytes at the processing rate) against the 131M flat fee — down from 18.5B at the previous pin, and the subject of the shielded pricing change that follows this PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Why
The resolving reads —
indexed_<axis>_{top_k,top_k_paginated,range}— return each entry with its primary value resolved (IndexedAxisEntry, since #817). That resolution runs after the secondary page was collected, through the caller's transaction; a caller that passesNonegets point reads outside the pinned iterator view the page came from. A primary deleted or rewritten by a commit in between is then reported asCorruptedData(the database is fine), or paired with a page from the older view. And a caller that only ranks — Dash Platform's ranked and having-range executors project every entry tokey_pair()— pays up tokprimary reads for values it discards and inherits that window for nothing. Surfaced by review on dashpay/platform#4382.What
Nine keys-only entry points:
They return the
(ordering_value, original_key)pairs straight from the secondary view and never open the primary.Structure: each read shape now has one
_rows_genericcore carrying all of the pinned-view logic (unchanged — the diff in those bodies is only the removal of the resolve call and theTxRefcreation, which moves to the wrappers), and two thin wrappers on top: the resolving one (existing behaviour, existing signatures) and the keys-only one. Both are built on the same core, so they cannot disagree about the page. The per-axis inclusive-bound encodings used by the range reads are factored intocount_range_bounds/sum_range_bounds/avg_range_bounds, shared by both variants.No change to any existing signature or behaviour;
IndexedTopKKeysPageis exported next toIndexedTopKPage.Tests
indexed_axis_keys_only_read_tests: the paginated keys-only page equals the resolving page projected withkey_pairs()— entries andskipped— on every axis, both directions, with and without an offset; range and plain top-k agree likewise; keys-only reads cost strictly fewer seeks and loaded bytes than their resolving counterparts; inverted bounds stay the empty answer. Fullgrovedblib suite green, clippy clean, verifier-only build unaffected.🤖 Generated with Claude Code