Skip to content

[Objects] Do not create an empty object for LSET, HDEL and HPERSIST on a missing key - #2192

Merged
Badrish Chandramouli (badrishc) merged 3 commits into
microsoft:mainfrom
Laurianti:fix-no-create-on-missing-key
Oct 2, 2026
Merged

Badrish Chandramouli (badrishc) merged 3 commits into
microsoft:mainfrom
Laurianti:fix-no-create-on-missing-key

Conversation

@Laurianti

@Laurianti Federico Laurianti (Laurianti) commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Root Cause

GarnetObject.NeedToCreate decides whether an object RMW on a missing key creates the object. LSET, HDEL and HPERSIST were missing from the list of operations that must not create it, so on a missing key they created an empty list or hash. The key then showed up in EXISTS, TYPE and DBSIZE, LPUSHX/RPUSHX pushed into it, and LINSERT got no reply.

Description of Change

libs/server/Objects/Types/GarnetObject.cs: ListOperation.LSET, HashOperation.HDEL and HashOperation.HPERSIST added to the operations that do not create the object. Their handlers already reply on NOTFOUND (ERR no such key, 0, and an array of -2).

ZADD with XX has the same problem but is not changed here: XX is an option of ZADD, so it needs the options from the input and a NOTFOUND reply in the handler.

Tests

  • RespListTests.LSETOnMissingKeyDoesNotCreateKey: after LSET on a missing key, the key does not exist and LPUSHX, RPUSHX and LINSERT return 0.
  • RespHashTests.HDELAndHPERSISTOnMissingKeyDoNotCreateKey: after HDEL and HPERSIST on a missing key, the key does not exist.
  • Removing any of the three lines makes the corresponding test fail. Garnet.test.collections: 782 passed, net10.0 Release.

Issues Fixed

Part of #2191: fixes LSET, HDEL and HPERSIST; ZADD XX remains open.

Copilot AI balanced review requested due to automatic review settings September 30, 2026 20:15

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 the title, description, command handlers, and regression coverage with no identified issues.

Review effort: Balanced
Findings: None

What changed in this PR

Prevents LSET, HDEL, and HPERSIST from creating empty objects when their keys are missing, aligning behavior with Redis and partially addressing #2191.

Changes:

  • Marks the three operations as non-creating.
  • Adds regression tests for missing-key responses and key existence.
  • Leaves ZADD XX explicitly out of scope.
File Description
libs/​server/​Objects/​Types/​GarnetObject.cs Disables initial object creation for the affected operations.
test/​standalone/​Garnet.test.collections/​RespListTests.cs Tests missing-key LSET behavior.
test/​standalone/​Garnet.test.collections/​RespHashTests.cs Tests missing-key HDEL and HPERSIST behavior.

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

@Laurianti Federico Laurianti (Laurianti) changed the title [Objects] Do not create an empty object for LSET, HDEL and HPERSIST on a missing key [Objects] Do not create the key when an object RMW leaves the new object empty Oct 1, 2026
@Laurianti Federico Laurianti (Laurianti) changed the title [Objects] Do not create the key when an object RMW leaves the new object empty [Objects] Do not create an empty object for LSET, HDEL and HPERSIST on a missing key Oct 1, 2026
@Laurianti

Copy link
Copy Markdown
Contributor Author

The branch is back at e2f0ebb, the commit you approved. The follow-up for ZADD XX is in draft #2194, on top of this PR.

@badrishc
Badrish Chandramouli (badrishc) merged commit 99739d2 into microsoft:main Oct 2, 2026
5 checks passed
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.

3 participants