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 21 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?
There was a problem hiding this comment.
Addressed. Per the 8.8 docs, both ARINSERT and ARRING return a RESP integer = the last index where a value was inserted — a u64 that exceeds 2^53 once the insert cursor passes Number.MAX_SAFE_INTEGER (e.g. after ARSEEK key <big>). Added both to ARRAY_U64_INTEGER_REPLY_COMMANDS, so the CLI and Workbench request them with { integerReply: 'bigint' } and render the index exactly.
ARSEEK stays out — its reply is 1/0 (cursor set / key missing), never a u64. ARSET likewise returns a new-slot count bounded by the command's args.
…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
…RI-8296]
The RAW, ASCII and UTF8 reply formatters stringified bigint integer replies, so the CLI and Workbench rendered u64 array indexes as quoted strings instead of (integer) N. Tag them as { type: 'integer', value } and render the tag in cliTextFormatter, matching redis-cli. Add ARINSERT and ARRING to the u64 integer-reply opt-in.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4300179f7d
ℹ️ 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".
| BrowserToolArrayCommands.ArGrep, | ||
| BrowserToolArrayCommands.ArInsert, | ||
| BrowserToolArrayCommands.ArRing, |
There was a problem hiding this comment.
When CLI/Workbench users run an AROP operation that returns an integer larger than Number.MAX_SAFE_INTEGER (for example AROP k 0 0 OR over a single value 9007199254740993), this allowlist does not match arop, so CliBusinessService/WorkbenchCommandsExecutor leave integerReply unset and ioredis rounds the RESP integer before formatting. Redis documents AROP AND/OR/XOR/MATCH/USED as integer replies (https://redis.io/docs/latest/commands/arop/#return-information), so include BrowserToolArrayCommands.ArOp in the bigint opt-in set to preserve exact results.
Useful? React with 👍 / 👎.
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 shared Redis client stack via a vendored ioredis patch and changes how array lengths/indexes are read; risk is mitigated by opt-in per command, broad tests, and API responses still serialized as decimal strings.
Overview
Adds exact handling of array u64 indexes/counts in the (2^53, 2^63) RESP-integer range by patching ioredis so commands can opt in with
{ integerReply: 'bigint' }, and wires that through array browser paths, CLI, and Workbench viaARRAY_U64_INTEGER_REPLY_COMMANDS(ARLEN,ARCOUNT,ARNEXT,ARSCAN,ARGREP, plus placeholders for future commands).Array key info no longer pipelines
ARLEN/ARCOUNT(pipeline cannot pass per-command options); those run as separate commands with the bigint opt-in so huge sparse lengths stay precise.Reply/UI formatting now treats
bigintand tagged{ type: 'integer', value }as(integer) Ninstead of quoted strings orJSON.stringifyfailures.Exposes
POST /array/search(RedisARGREP) with predicates, optional AND/OR, range,NOCASE,WITHVALUES, andLIMIT, including flat vs nested reply normalization and 400 mapping for badREpredicates (RegexError).Tests/DX: integration cases for the gap zone, a raw-socket RESP wire contract test, Bruno docs, and test Dockerfile copies
patches/beforeyarn installso patch-package applies reliably.Reviewed by Cursor Bugbot for commit 4300179. Bugbot is set up for automated code reviews on this repo. Configure here.