Fix duplicate keys in RangeResult reverse pagination (#73)#74
Open
s2x wants to merge 1 commit into
Open
Conversation
When iterating a range in reverse across multiple server pages, the iterator advanced the upper bound selector using KeySelector::firstGreaterOrEqual($lastKey) at every batch boundary, an *inclusive* selector. That selector targets the boundary key again, so the first key of every subsequent batch was a duplicate of the last key yielded by the previous batch. Mirror the forward branch: use firstGreaterThan so the key just produced is excluded from the next batch. Add two integration tests covering reverse pagination both with and without an explicit limit over 1000 keys, large enough to force multiple server pages.
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
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.
Summary
RangeResultreverse pagination yielded each boundary key twice when arange span more than one server page. The iterator advanced the upper
bound at every batch boundary with
KeySelector::firstGreaterOrEqual,an inclusive selector, so the last key returned in batch N was
returned again as the first key in batch N+1.
The forward branch already uses
firstGreaterThanand is unaffected;this PR makes the reverse branch do the same.
Reproducer (before the fix)
Fix
src/RangeResult.php— flip the reverse branch fromKeySelector::firstGreaterOrEqual()toKeySelector::firstGreaterThan(), mirroring the forwardbranch.
Tests
tests/Integration/RangeReadTest.php— two new tests over a 1000-key,~50-byte value range (forces multiple server pages):
getRangeReversePaginationYieldsEachKeyOnce— full reverse range,asserts
count == 1000, first isp/0999, last isp/0000,no duplicates, exact ordering.
getRangeReversePaginationWithLimitAcrossBatches—limit: 750onthe same range size, asserts
count == 750, no duplicates,last-key == first of new batch.
Both tests fail under the old behavior and pass after the fix.
Checklist
composer test:unit— 429 tests, 874 assertions, OKcomposer cs— cleancomposer rector— clean (also appliedSortCallLikeNamedArgsRector)composer phpstan— cleancomposer test:integration— local environment has no FDBcluster; covered by CI against
docker-compose upUnreleased / Fixedwith the [[Bug] RangeResult pagination yields duplicate keys when iterating in reverse #73] anchorfix/issue-73-rangeresult-reverse-duplicatesfrommaster