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
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