RI-8299 Keep array append precise above 2^53#6158
Conversation
The append path read ARLEN without the bigint opt-in, so lengths above 2^53 rounded at the transport and the write landed on the wrong slot. Opt in like every other u64 array read, un-skip the integration test pinning it, pin the workbench executor's opt-in forwarding, and refresh stale comments and Bruno docs. References: #RI-8299
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4932b081fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code Coverage - Backend unit tests
Test suite run success3650 tests passing in 319 suites. Report generated by 🧪jest coverage report action from fb49fae |
Code Coverage - Integration Tests
|
ARSEEK repositions the insert cursor in one step, so ARNEXT's bigint opt-in is provable end to end in the lossy (2^53, 2^63) zone after all; replace the duplicate-coverage note with that test. ARCOUNT's zone is unreachable on a real server (counts are memory-bound), so pin its opt-in call shape at the unit level instead. References: #RI-8299
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb49fae075
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
What
The append path was the last array read without the
integerReply: 'bigint'opt-in: it derived the end index from anARLENreply that rounds above 2^53, so appending to such an array overwrote the wrong slot. Adds the opt-in, un-skips the integration test pinning it (the last skipped test in the array suites), pins the workbench executor's opt-in forwarding in its spec (mirroring the CLI spec), and refreshes stale comments and Bruno docs that described the transport fix as pending.The rest of RI-8299's scope already shipped inside the feature verticals: ARLEN, ARSCAN, ARGREP, and AROP each have live integration assertions in the lossy (2^53, 2^63) zone.
Testing
TEST_TAGS=array yarn --cwd redisinsight/api test:apiagainst a real Redis 8.8 standalone: 390 passing, 0 pending. The un-skipped test fails against the unfixed service (derives 9007199254740992 and clobbers the seeded element).Closes #RI-8299
Note
Medium Risk
Changes array write behavior for very large indexes (data corruption risk if wrong); scope is narrow and heavily regression-tested.
Overview
Array append now reads
ARLENwithintegerReply: 'bigint', so the derived end index stays exact when length exceeds 2^53 instead of rounding and writing to the wrong slot.Tests and docs un-skip the append integration case above 2^53, add API coverage for
get-next-indexin the lossy integer zone, tighten unit specs for append/ARCOUNT/workbenchARLENforwarding, and update Bruno aggregate docs to state full precision (no pending RI-8296 rounding caveat).Reviewed by Cursor Bugbot for commit fb49fae. Bugbot is set up for automated code reviews on this repo. Configure here.