Skip to content

Preserve binary members as VectorSetRange bounds - #3256

Merged
mgravell merged 1 commit into
StackExchange:mainfrom
Laurianti:fix-vrange-binary-members
Oct 2, 2026
Merged

mgravell merged 1 commit into
StackExchange:mainfrom
Laurianti:fix-vrange-binary-members

Conversation

@Laurianti

Copy link
Copy Markdown
Contributor

VectorSetRange and VectorSetRangeEnumerate build the [/( bounds by string concatenation, ("(" + value). For a member that is not valid UTF-8, RedisValue.ToString() returns its hex form, so the bound sent to the server is not the member. ZRANGEBYLEX already builds its bounds from the raw bytes in GetLexRange; this change makes VRANGE use the same helper.

With members a, b and { 0xFF, 0x01 }, against Redis 8:

  • VectorSetRange(key, start: binary, exclude: Exclude.Start) returned the binary member itself instead of nothing.
  • VectorSetRangeEnumerate(key, count: 1) never ended: after a, b, 0xFF01 it kept returning 0xFF01, because each next batch started from the hex text, which sorts before the member.

Added VectorSetRange_BinaryMemberAsBound and VectorSetRangeEnumerate_BinaryMembers to VectorSetIntegrationTests. Against Redis 8, --filter VectorSetRange passes 40 of 40 (RESP2 and RESP3); without the change only the two new tests fail, in both protocols. The library builds for all targets with no warnings.

Checklist

  • I fully and freely contribute this code in accordance with the project license (and am legally able to do so)
  • I take responsibility for this contribution's quality and correctness, including any portions produced with AI assistance (see CONTRIBUTING.md).

Copilot AI balanced review requested due to automatic review settings October 2, 2026 04:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The shared bound construction fixes the binary corruption issue and is covered by focused regression tests.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes VRANGE bounds to preserve binary members by reusing raw-byte lexicographic bound construction.

Changes:

  • Replaces string concatenation with GetLexRange.
  • Adds binary-bound and pagination regression tests.
File Description
src/​StackExchange.Redis/​RedisDatabase.VectorSets.cs Builds vector range bounds from raw bytes.
tests/​StackExchange.Redis.Tests/​VectorSetIntegrationTests.cs Tests binary bounds and finite enumeration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Laurianti

Copy link
Copy Markdown
Contributor Author

The Windows failure is BacklogTests.TotalOutstandingIncludesBacklogQueue, which also failed on Windows in run 36694984305 (marc/bulk-string-concurrency-ceiling); it is unrelated to VRANGE, and the Ubuntu job that runs the vector set tests passed.

@mgravell
mgravell merged commit c137c64 into StackExchange:main Oct 2, 2026
4 of 5 checks passed
@Laurianti
Laurianti deleted the fix-vrange-binary-members branch October 2, 2026 13:33
mgravell added a commit that referenced this pull request Oct 6, 2026
…ented)

VectorSetBound concatenated "[" + value through a string, mangling members
that are not valid UTF-8; it now delegates to SortedSets.LexBound, which
builds the bound from the raw bytes. Upstream's two integration tests are
ported unchanged, and fail without the fix.

Absorbs: c137c64
mgravell added a commit that referenced this pull request Oct 6, 2026
Conflicts, and how each was re-expressed in v4 terms:
- RedisDatabase.VectorSets.cs: modified on main, deleted on v4. Kept deleted;
  the fix lives in VectorSets.VectorSetBound (990dccd).
- VectorSetIntegrationTests.cs: both sides added the same two tests. Took v4's.
FloatingMilkshake pushed a commit to FloatingMilkshake/MechanicalMilkshake that referenced this pull request Oct 9, 2026
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [StackExchange.Redis](https://seredis.dev/) ([source](https://github.com/StackExchange/StackExchange.Redis)) | nuget | minor | `3.3.1` → `3.4.0` |

---

### Release Notes

<details>
<summary>StackExchange/StackExchange.Redis (StackExchange.Redis)</summary>

### [`v3.4.0`](https://github.com/StackExchange/StackExchange.Redis/releases/tag/3.4.0)

[Compare Source](StackExchange/StackExchange.Redis@3.3.1...3.4.0)

#### Major Changes

- In RESP3, by default the library now uses **separate** connections for pub/sub; see [#&#8203;3264](StackExchange/StackExchange.Redis#3264)
- Smart-client-handoffs is now enabled by default on Redis Enterprise and AMR; see [#&#8203;3248](StackExchange/StackExchange.Redis#3248)
- Some logging events incorrectly used duplicated ids - resolved; see [#&#8203;3266](StackExchange/StackExchange.Redis#3266)

#### All Changes

- Recover promptly from a cluster failover; fix the configuration channel under RESP3 and with a channel prefix ([#&#8203;3254](StackExchange/StackExchange.Redis#3254)) by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3255](StackExchange/StackExchange.Redis#3255)
- Preserve binary members as VectorSetRange bounds by [@&#8203;Laurianti](https://github.com/Laurianti) in [#&#8203;3256](StackExchange/StackExchange.Redis#3256)
- Smart client handoffs: reinstate auto-enlistment for the hosted providers and remove \[Experimental] by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3248](StackExchange/StackExchange.Redis#3248)
- docs: fix retry-category test in the custom RetryPolicy sample by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3265](StackExchange/StackExchange.Redis#3265)
- Fix duplicate LoggerMessage event ids; add a guard test by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3266](StackExchange/StackExchange.Redis#3266)
- Scan: observe the faults of abandoned page tasks after a sync timeout or dispose ([#&#8203;3260](StackExchange/StackExchange.Redis#3260)) by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3267](StackExchange/StackExchange.Redis#3267)
- Report the outcome of every physical connection attempt, identifying the client certificate used by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3257](StackExchange/StackExchange.Redis#3257)
- Use a dedicated pub/sub connection under RESP3 by default by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3264](StackExchange/StackExchange.Redis#3264)

#### Tooling

- docs(agents): add create-implementation-plan-for-redis-api-change skill by [@&#8203;uglide](https://github.com/uglide) in [#&#8203;3268](StackExchange/StackExchange.Redis#3268)

#### New Contributors

- [@&#8203;Laurianti](https://github.com/Laurianti) made their first contribution in [#&#8203;3256](StackExchange/StackExchange.Redis#3256)

**Full Changelog**: <StackExchange/StackExchange.Redis@3.3.1...3.4.0>

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4xNDkuMCIsInVwZGF0ZWRJblZlciI6IjQ0LjE0OS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Reviewed-on: https://git.milkshake.services/FloatingMilkshake/MechanicalMilkshake/pulls/9
Reviewed-by: Milkshake <[email protected]>
FloatingMilkshake pushed a commit to FloatingMilkshake/Azusa that referenced this pull request Oct 9, 2026
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [StackExchange.Redis](https://seredis.dev/) ([source](https://github.com/StackExchange/StackExchange.Redis)) | nuget | minor | `3.3.1` → `3.4.0` |

---

### Release Notes

<details>
<summary>StackExchange/StackExchange.Redis (StackExchange.Redis)</summary>

### [`v3.4.0`](https://github.com/StackExchange/StackExchange.Redis/releases/tag/3.4.0)

[Compare Source](StackExchange/StackExchange.Redis@3.3.1...3.4.0)

#### Major Changes

- In RESP3, by default the library now uses **separate** connections for pub/sub; see [#&#8203;3264](StackExchange/StackExchange.Redis#3264)
- Smart-client-handoffs is now enabled by default on Redis Enterprise and AMR; see [#&#8203;3248](StackExchange/StackExchange.Redis#3248)
- Some logging events incorrectly used duplicated ids - resolved; see [#&#8203;3266](StackExchange/StackExchange.Redis#3266)

#### All Changes

- Recover promptly from a cluster failover; fix the configuration channel under RESP3 and with a channel prefix ([#&#8203;3254](StackExchange/StackExchange.Redis#3254)) by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3255](StackExchange/StackExchange.Redis#3255)
- Preserve binary members as VectorSetRange bounds by [@&#8203;Laurianti](https://github.com/Laurianti) in [#&#8203;3256](StackExchange/StackExchange.Redis#3256)
- Smart client handoffs: reinstate auto-enlistment for the hosted providers and remove \[Experimental] by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3248](StackExchange/StackExchange.Redis#3248)
- docs: fix retry-category test in the custom RetryPolicy sample by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3265](StackExchange/StackExchange.Redis#3265)
- Fix duplicate LoggerMessage event ids; add a guard test by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3266](StackExchange/StackExchange.Redis#3266)
- Scan: observe the faults of abandoned page tasks after a sync timeout or dispose ([#&#8203;3260](StackExchange/StackExchange.Redis#3260)) by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3267](StackExchange/StackExchange.Redis#3267)
- Report the outcome of every physical connection attempt, identifying the client certificate used by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3257](StackExchange/StackExchange.Redis#3257)
- Use a dedicated pub/sub connection under RESP3 by default by [@&#8203;mgravell](https://github.com/mgravell) in [#&#8203;3264](StackExchange/StackExchange.Redis#3264)

#### Tooling

- docs(agents): add create-implementation-plan-for-redis-api-change skill by [@&#8203;uglide](https://github.com/uglide) in [#&#8203;3268](StackExchange/StackExchange.Redis#3268)

#### New Contributors

- [@&#8203;Laurianti](https://github.com/Laurianti) made their first contribution in [#&#8203;3256](StackExchange/StackExchange.Redis#3256)

**Full Changelog**: <StackExchange/StackExchange.Redis@3.3.1...3.4.0>

</details>

---

### Configuration

📅 **Schedule**: (UTC)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4xNDkuMCIsInVwZGF0ZWRJblZlciI6IjQ0LjE0OS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Reviewed-on: https://git.milkshake.services/FloatingMilkshake/Azusa/pulls/9
Reviewed-by: Milkshake <[email protected]>
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.

3 participants