Skip to content

fix(#73): add reverse range pagination regression coverage - #75

Merged
s2x merged 1 commit into
masterfrom
fix/issue-73-reverse-pagination
Jul 7, 2026
Merged

fix(#73): add reverse range pagination regression coverage#75
s2x merged 1 commit into
masterfrom
fix/issue-73-reverse-pagination

Conversation

@s2x

@s2x s2x commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #73.

Investigation against a live FDB 7.3.75 cluster showed that the current implementation already yields each key exactly once during reverse iteration. The reverse branch advances the end selector with KeySelector::firstGreaterOrEqual($lastKey), which correctly excludes the already-yielded boundary key.

The fix proposed in the issue (firstGreaterThan($lastKey)) was empirically verified to be incorrect — it causes an infinite loop that returns the boundary key forever (see reproduction below). It was therefore deliberately not applied; applying it would break getRangeAll/getRange with reverse: true.

Changes

  • Extracted RangeResult::paginate() (a static method taking an injectable fetcher) so pagination logic can be unit-tested without a running cluster. Behavior is unchanged.
  • tests/Unit/RangeResultTest.php — FDB-emulating mock; asserts forward and reverse iteration each key exactly once, descending/ascending order, boundary keys, and zero-limit short-circuit. This test fails if the wrong firstGreaterThan change is applied (it reproduces the duplicate/loop symptom).
  • tests/Integration/ReversePaginationTest.php — 1000 keys iterated across real server batches in reverse; asserts count == 1000 and no duplicates.
  • CHANGELOG.md entry under Fixed.

Verification

  • composer lint — passes (PHPCS + Rector + PHPStan).
  • composer test:unit — passes (pre-existing GMP-related TupleTest errors are unrelated to this change).
  • composer test:integration — 203/203 pass against FDB 7.3.75.

Reproduction of why the proposed fix is wrong

With the issue's firstGreaterThan($lastKey) on the reverse branch, iterating 1000 keys yields:

batch it=1  last=test/revp/key0916
batch it=2  first=test/revp/key0916  <- boundary key repeated
...
batch it=524205 last=test/revp/key0916  <- infinite loop, only key0916 returned

Reverse iteration already yields each key exactly once: the reverse branch
advances the end selector with firstGreaterOrEqual(lastKey), which correctly
excludes the boundary key. The issue's proposed firstGreaterThan change was
verified against a live FDB cluster to cause an infinite loop and was not
applied.

- Extract RangeResult::paginate() so pagination can be unit-tested without a cluster
- Add tests/Unit/RangeResultTest.php (FDB-emulating mock, forward + reverse)
- Add tests/Integration/ReversePaginationTest.php (1000 keys across server batches)
@s2x
s2x merged commit c41822e into master Jul 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] RangeResult pagination yields duplicate keys when iterating in reverse

1 participant