RI-8296 Exact u64 array integer replies (per-command ioredis bigint)#6100
RI-8296 Exact u64 array integer replies (per-command ioredis bigint)#6100VaskoAtanasovRedis wants to merge 20 commits into
Conversation
🛡️ Jit Security Scan Results
✅ No security findings were detected in this PR
Security scan by Jit
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12a8e30808
ℹ️ 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 success3610 tests passing in 317 suites. Report generated by 🧪jest coverage report action from 5e323a9 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c4390b2. Configure here.
Code Coverage - Integration Tests
|
VaskoAtanasovRedis
force-pushed
the
be/RI-8296/ioredis-smart-int-parse
branch
from
ce6b4aa to
413f584
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 413f5849a7
ℹ️ 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".
VaskoAtanasovRedis
force-pushed
the
be/RI-8296/ioredis-smart-int-parse
branch
3 times, most recently
from
e333211 to
3a11af7
Compare
POST /array/search backs the Redis ARGREP command: predicate-based search over an array's index range with a single global AND/OR connective, NOCASE, WITHVALUES, LIMIT and -/+ range shorthands. Indexes are carried as decimal strings end to end. Mirrors the existing array read endpoints. References: #RI-8291
References: #RI-8291
Covers predicate matching, the global AND/OR connective, WITHVALUES on/off, and the 404 / 400 wrong-type error paths. Includes a >2^53 index round-trip asserting the exact string survives the transport. Gated to Redis 8.8+. References: #RI-8291
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a11af787d
ℹ️ 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".
|
|
||
| // Array commands whose integer replies are u64 (indexes / counts) and must | ||
| // stay exact above 2^53 — callers request them with { integerReply: 'bigint' }. | ||
| export const ARRAY_U64_INTEGER_REPLY_COMMANDS = new Set<string>([ |
There was a problem hiding this comment.
When CLI/Workbench users run ARINSERT or ARRING after moving the array insert cursor above Number.MAX_SAFE_INTEGER (for example with ARSEEK key 9007199254740993), Redis returns the last inserted index as a RESP integer, but those commands are omitted from the only command set that enables integerReply: 'bigint' in CliBusinessService.sendCommand and WorkbenchCommandsExecutor.sendCommand. The patched parser therefore stays in Number mode and rounds the returned index before formatting, so exact-u64 output still fails for these Redis 8.8 array commands.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@VaskoAtanasovRedis It is valid concern. was it addressed?
…llow empty predicate Real Redis 8.8 returns ARGREP WITHVALUES as nested [[index, value], ...] (the docs show flat); parse both shapes like ARSCAN. Drop the invented AND default so the server's documented OR applies when the connective is omitted. Drop @isnotempty on predicate value so EXACT "" matches empty elements. The >2^53 index precision is an ioredis transport issue shared across the array commands — deferred to RI-8296; this PR's integration test asserts a <=2^53 round-trip. References: #RI-8291
@IsOptional() lets a JSON null through and a destructuring default only fills undefined, so withValues: null wrongly omitted WITHVALUES. Normalize with ?? true. References: #RI-8291
Predicate value now uses the same RedisString handling as array writes/responses (ArrayElementDto.value), so a binary value the API returns can be searched (EXACT) and escaped input is decoded to bytes rather than sent literally. References: #RI-8291
…les [RI-8291] The Search.bru doc said the combinator defaults to AND; the server (and the DTO) default to OR. Add example bodies covering an OR range search and an AND indexes-only search, per review. References: #RI-8291
Move mockGetArraySearchDto / mockGetArraySearchResponse out of the static __mocks__ module into getArraySearchDtoFactory / getArraySearchResponseFactory (raw reply arrays stay static). Addresses review feedback on #6090. References: #RI-8291
An invalid RE predicate is bad client input, but only WRONGTYPE was mapped to BadRequest — Redis's 'ERR invalid regular expression' fell through to a 500. Match that marker (verified via redis-cli) and surface it as 400; cover it with unit + real-server integration tests. References: #RI-8291
…8291]
The earlier marker only matched 'invalid regular expression'; Redis 8.8 also rejects empty patterns ('regular expression is empty') and backreferences ('... backreferences are not supported') as 500s. All share the 'regular expression' substring (verified via redis-cli), so widen RedisErrorCodes.RegexError to cover the family; unit tests assert each message and a backreference integration case guards the broadening.
References: #RI-8291
VaskoAtanasovRedis
force-pushed
the
be/RI-8291/argrep-search-endpoint
branch
from
96c3d38 to
c73ec0d
Compare
…gint [RI-8296]
Adopt the ioredis patch that toggles the parser's stringNumbers per command (via the command queue) and returns BigInt for commands that opt in with { integerReply: 'bigint' } — so other commands are untouched. Opt in the five array integer-reply commands (ARLEN/ARCOUNT/ARNEXT/ARSCAN/ARGREP); their indexes/counts are normalised to strings by toRequiredIndexString. Replaces the earlier global redis-parser approach.
References: #RI-8296
…296]
Detect array u64 integer-reply commands (ARLEN/ARCOUNT/ARNEXT/ARSCAN/ARGREP) in the CLI and request { integerReply: 'bigint' }; the raw/utf-8/text output formatters stringify BigInt so the result isn't rounded and doesn't break JSON serialization.
References: #RI-8296
…set [RI-8296]
Workbench executor opts array u64 integer-reply commands into { integerReply: 'bigint' } and its UTF8/ASCII reply transformers stringify BigInt — same as the CLI. Extract ARRAY_U64_INTEGER_REPLY_COMMANDS into browser-tool-commands.ts as one source of truth for both.
References: #RI-8296
…8296] With the per-command bigint engine, the search endpoint returns the exact u64 index, so the >2^53 round-trip (de-scoped on #6090) is proven again. References: #RI-8296
…8296] main's read-endpoint precision tests use values >= 2^63, which Redis sends as RESP bulk strings (exact without the engine). Add round-trip cases at 2^53+1 — a RESP integer that a JS number rounds — so ARLEN and ARSCAN actually exercise the per-command bigint path. ARCOUNT/ARNEXT can't reach the zone (count/cursor never approach 2^53). References: #RI-8296
Raw-socket probe asserting the actual RESP type byte: a length in (2^53, 2^63) comes back as a RESP integer (':') exact on the wire, and a length >= 2^63 as a bulk string ('$'). Pins the boundary the per-command bigint engine relies on, so a future Redis encoding change trips a test instead of invalidating a comment. Gated to standalone 8.8.
References: #RI-8296
The node-redis client drops integerReply (no redis-parser patch there); u64 integer replies in (2^53, 2^63) round on that path until RESP3 type mapping is added. References: #RI-8296
…-8296]
Use requirements('rte.type<>CLUSTER') instead of an in-test this.skip(); the imperative form tripped the strict check tsconfig (TS2683 'this' implicitly any).
References: #RI-8296
Key-info read ARLEN/ARCOUNT via sendPipeline, which can't carry per-command options, so length/count rounded in the (2^53, 2^63) zone. Read them per-command with the bigint opt-in (TTL/MEMORY USAGE stay pipelined for graceful degradation); add a real-server gap-zone integration case. The CLI TEXT formatter recursed array leaves through JSON.stringify, which throws on BigInt, so ARSCAN/ARGREP replies became failed executions. Format BigInt leaves as '(integer) N' like the top level. References: #RI-8296
test.Dockerfile copied patches/ after yarn install, so patch-package found nothing and the ioredis bigint parser never applied — every (2^53, 2^63) array reply rounded in CI. Copy patches/ before install, mirroring the root Dockerfile. Also fix the RESP wire test's cluster gate: conditionalIgnore has no '<>' operator (always skips), so use the supported '!rte.type=CLUSTER'. References: #RI-8296
VaskoAtanasovRedis
force-pushed
the
be/RI-8296/ioredis-smart-int-parse
branch
from
3a11af7 to
5e323a9
Compare
2 participants
What
Makes array u64 integer replies exact. RESP encodes
:integers as signedi64, so Redis sends u64 values ≥ 2^63 as bulk strings (exact) but values in
(2^53, 2^63) as RESP integers — which ioredis coerces to a JS number and
rounds above 2^53. That zone was silently lossy for array indexes/lengths.
A patched ioredis
DataHandlertoggles the parser'sstringNumberspercommand via the command queue and returns
BigIntonly for commands thatopt in with
{ integerReply: 'bigint' }. All other commands are untouched.ARLEN,ARCOUNT,ARNEXT,ARSCAN,ARGREP(sharedARRAY_U64_INTEGER_REPLY_COMMANDSset).stringify
BigInt.toIndexString, soresponses stay decimal-string — no BigInt reaches
res.json.Testing
{ integerReply: 'bigint' }opt-in on each service call.restore the
>2^53ARGREP search assertion (run on the 8.8 RTE).RESP integer (
:), a value ≥ 2^63 as a bulk string ($).Stacked on #6090 (RI-8291 search endpoint).
Closes #RI-8296
Note
Medium Risk
Touches the Redis transport layer via a vendored ioredis patch and changes how array metadata is fetched (extra round-trips for ARLEN/ARCOUNT in key info); scope is opt-in per command but incorrect parser behavior could affect non-array commands if misused.
Overview
Fixes silent precision loss for Redis array u64 indexes and counts when Redis returns them as RESP integers in (2^53, 2^63) — values ioredis would coerce to JS
numberand round.Adds a patch-package change to ioredis so commands can opt in with
{ integerReply: 'bigint' }: the parser togglesstringNumbersper queued command and converts integer replies toBigInt. The Redis client layer exposes that option and extends reply types to includebigint.ARRAY_U64_INTEGER_REPLY_COMMANDS(ARLEN,ARCOUNT,ARNEXT,ARSCAN,ARGREP) drives opt-in across the array browser service, CLI, and Workbench. Array key info stops pipelining those two count commands (pipelines can’t pass per-command options) and fetches them with bigint instead. Reply formatters stringifyBigIntso CLI/text output doesn’t hitJSON.stringifyfailures.Integration tests cover the gap zone end-to-end; a raw-socket test documents RESP
:vs$wire encoding. The test Docker image copiespatches/beforeyarn installso the ioredis patch always applies. node-redis is unchanged and still documented as rounding in that zone.Reviewed by Cursor Bugbot for commit 5e323a9. Bugbot is set up for automated code reviews on this repo. Configure here.