Skip to content

[Objects] ZADD: Reply nil to XX INCR on a missing member - #2197

Merged
Badrish Chandramouli (badrishc) merged 2 commits into
microsoft:mainfrom
Laurianti:fix-zadd-xx-incr-nil
Oct 2, 2026
Merged

Badrish Chandramouli (badrishc) merged 2 commits into
microsoft:mainfrom
Laurianti:fix-zadd-xx-incr-nil

Conversation

@Laurianti

Copy link
Copy Markdown
Contributor

Root Cause

In SortedSetObjectImpl.SortedSetAdd, a missing member with XX set was skipped with continue, so with INCR the reply fell through to WriteDoubleNumeric(incrResult) with incrResult still 0. The branch for an existing member aborted by NX, GT or LT already writes null.

Description of Change

libs/server/Objects/SortedSet/SortedSetObjectImpl.cs: with XX and INCR on a missing member, write null and return, as the NX/GT/LT branch does. ZADD ... XX INCR now replies nil when the member does not exist, with or without the key, as in Redis.

Tests

  • RespSortedSetTests.AddWithOptions: ZADD XX INCR on a missing member of an existing key replies null and does not add the member; on a missing key it replies null.
  • Without the change the new assertions fail. Garnet.test.collections: 780 passed, Garnet.test: 1286 passed, Garnet.test.scripting: 638 passed, net10.0 Release.

Issues Fixed

Fixes #2196

Copilot AI balanced review requested due to automatic review settings October 1, 2026 07:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 focused implementation matches existing abort behavior and is adequately tested.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes ZADD XX INCR to return nil when the member is missing, matching Redis behavior.

Changes:

  • Returns null instead of 0 for aborted XX INCR operations.
  • Adds coverage for existing and missing keys.
File Description
SortedSetObjectImpl.cs Writes null when XX INCR targets a missing member.
RespSortedSetTests.cs Tests missing-member and missing-key cases.

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

@badrishc
Badrish Chandramouli (badrishc) merged commit 7b6f3a0 into microsoft:main Oct 2, 2026
15 checks passed
@Laurianti
Federico Laurianti (Laurianti) deleted the fix-zadd-xx-incr-nil branch October 2, 2026 03:43
x@01 (x-at-01) added a commit to webc-fork/garnet that referenced this pull request Oct 4, 2026
…n missing key (microsoft#2192), ZADD XX INCR null (microsoft#2197), no empty object from object RMW (microsoft#2194), fresh ObjectOutput per key HCOLLECT/ZCOLLECT (microsoft#2200), CompletionEvent disposal + LogSizeTracker coalesce (microsoft#2198), dynamic test ports (microsoft#2193)
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.

ZADD with XX and INCR replies 0 instead of nil when the member does not exist

3 participants