From 9ee29c265130c7d086d4a85080b0ee67e3a48ff7 Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Tue, 12 Aug 2025 02:57:44 +0000
Subject: [PATCH 01/30] fixing test issue

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/aof.c                                |  10 +-
 src/db.c                                 |  49 +++++----
 src/entry.c                              | 132 ++++++++++++++++++-----
 src/entry.h                              |  20 +++-
 src/module.c                             |  13 ++-
 src/rdb.c                                |   9 +-
 src/server.h                             |   3 +-
 src/t_hash.c                             | 116 +++++++++++---------
 src/valkeymodule.h                       |   2 +
 src/ziplist.c                            |   2 +-
 tests/modules/Makefile                   |   1 +
 tests/modules/hash_view_value.c          |  64 +++++++++++
 tests/unit/moduleapi/hash_view_value.tcl |  21 ++++
 tests/unit/scripting.tcl                 |   2 +-
 14 files changed, 333 insertions(+), 111 deletions(-)
 create mode 100644 tests/modules/hash_view_value.c
 create mode 100644 tests/unit/moduleapi/hash_view_value.tcl

diff --git a/src/aof.c b/src/aof.c
index b71ef4cba8..edc9fbb663 100644
--- a/src/aof.c
+++ b/src/aof.c
@@ -1943,8 +1943,9 @@ static int rioWriteHashIteratorCursor(rio *r, hashTypeIterator *hi, int what) {
         else
             return rioWriteBulkLongLong(r, vll);
     } else if (hi->encoding == OBJ_ENCODING_HASHTABLE) {
-        sds value = hashTypeCurrentFromHashTable(hi, what);
-        return rioWriteBulkString(r, value, sdslen(value));
+        size_t len;
+        char *value = hashTypeCurrentFromHashTable(hi, what, &len);
+        return rioWriteBulkString(r, value, len);
     }
 
     serverPanic("Unknown hash encoding");
@@ -1962,7 +1963,8 @@ int rewriteHashObject(rio *r, robj *key, robj *o) {
         while (hashTypeNext(&hi) != C_ERR) {
             long long expiry = entryGetExpiry(hi.next);
             sds field = entryGetField(hi.next);
-            sds value = entryGetValue(hi.next);
+            size_t value_len;
+            sds value = entryGetValue(hi.next, &value_len);
             if (rioWriteBulkCount(r, '*', 8) == 0) return 0;
             if (rioWriteBulkString(r, "HSETEX", 6) == 0) return 0;
             if (rioWriteBulkObject(r, key) == 0) return 0;
@@ -1971,7 +1973,7 @@ int rewriteHashObject(rio *r, robj *key, robj *o) {
             if (rioWriteBulkString(r, "FIELDS", 6) == 0) return 0;
             if (rioWriteBulkLongLong(r, 1) == 0) return 0;
             if (rioWriteBulkString(r, field, sdslen(field)) == 0) return 0;
-            if (rioWriteBulkString(r, value, sdslen(value)) == 0) return 0;
+            if (rioWriteBulkString(r, value, value_len) == 0) return 0;
             volatile_items++;
         }
         hashTypeResetIterator(&hi);
diff --git a/src/db.c b/src/db.c
index c01c10b969..9efdeba824 100644
--- a/src/db.c
+++ b/src/db.c
@@ -1004,6 +1004,17 @@ int objectTypeCompare(robj *o, long long target) {
         return 1;
 }
 
+typedef struct {
+    const char *buf;
+    size_t len;
+} scanDataItem;
+
+static void addScanDataItem(vector *result, const char *buf, size_t len) {
+    scanDataItem *item = vectorPush(result);
+    item->buf = buf;
+    item->len = len;
+}
+
 /* Hashtable scan callback used by scanCallback when scanning the keyspace. */
 void keysScanCallback(void *privdata, void *entry, int didx) {
     scanData *data = (scanData *)privdata;
@@ -1034,15 +1045,15 @@ void keysScanCallback(void *privdata, void *entry, int didx) {
     }
 
     /* Keep this key. */
-    sds *item = vectorPush(data->result);
-    *item = key;
+    addScanDataItem(data->result, (const char *)key, sdslen(key));
 }
 
 /* This callback is used by scanGenericCommand in order to collect elements
  * returned by the dictionary iterator into a list. */
 void hashtableScanCallback(void *privdata, void *entry) {
     scanData *data = (scanData *)privdata;
-    sds val = NULL;
+    const char *val = NULL;
+    size_t val_len = 0;
     sds key = NULL;
 
     robj *o = data->o;
@@ -1062,7 +1073,7 @@ void hashtableScanCallback(void *privdata, void *entry) {
     } else if (o->type == OBJ_HASH) {
         key = entryGetField(entry);
         if (!data->only_keys) {
-            val = entryGetValue(entry);
+            val = entryGetValue(entry, &val_len);
         }
     } else {
         serverPanic("Type not handled in hashtable SCAN callback.");
@@ -1083,16 +1094,14 @@ void hashtableScanCallback(void *privdata, void *entry) {
         key = sdsdup(node->ele);
         if (!data->only_keys) {
             char buf[MAX_LONG_DOUBLE_CHARS];
-            int len = ld2string(buf, sizeof(buf), node->score, LD_STR_AUTO);
-            val = sdsnewlen(buf, len);
+            val_len = ld2string(buf, sizeof(buf), node->score, LD_STR_AUTO);
+            val = (const char *)sdsnewlen(buf, val_len);
         }
     }
 
-    sds *item = vectorPush(data->result);
-    *item = key;
+    addScanDataItem(data->result, (const char *)key, sdslen(key));
     if (val) {
-        item = vectorPush(data->result);
-        *item = val;
+        addScanDataItem(data->result, val, sdslen(val));
     }
 }
 
@@ -1247,7 +1256,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
         /* scanning ZSET allocates temporary strings even though it's a dict */
         free_callback = sdsfree;
     }
-    vectorInit(&result, SCAN_VECTOR_INITIAL_ALLOC, sizeof(sds));
+    vectorInit(&result, SCAN_VECTOR_INITIAL_ALLOC, sizeof(scanDataItem));
 
     /* For main hash table scan or scannable data structure. */
     if (!o || ht) {
@@ -1308,8 +1317,8 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
             if (use_pattern && !stringmatchlen(pat, sdslen(pat), key, len, 0)) {
                 continue;
             }
-            sds *item = vectorPush(&result);
-            *item = sdsnewlen(key, len);
+            sds item = sdsnewlen(key, len);
+            addScanDataItem(&result, (const char *)item, sdslen(item));
         }
         setTypeReleaseIterator(si);
         cursor = 0;
@@ -1329,13 +1338,13 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
                 continue;
             }
             /* add key object */
-            sds *item = vectorPush(&result);
-            *item = sdsnewlen(str, len);
+            sds item = sdsnewlen(str, len);
+            addScanDataItem(&result, (const char *)item, sdslen(item));
             /* add value object */
             if (!only_keys) {
                 str = lpGet(p, &len, intbuf);
-                item = vectorPush(&result);
-                *item = sdsnewlen(str, len);
+                item = sdsnewlen(str, len);
+                addScanDataItem(&result, (const char *)item, sdslen(item));
             }
             p = lpNext(o->ptr, p);
         }
@@ -1350,10 +1359,10 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
 
     addReplyArrayLen(c, vectorLen(&result));
     for (uint32_t i = 0; i < vectorLen(&result); i++) {
-        sds *key = vectorGet(&result, i);
-        addReplyBulkCBuffer(c, *key, sdslen(*key));
+        scanDataItem *key = vectorGet(&result, i);
+        addReplyBulkCBuffer(c, key->buf, key->len);
         if (free_callback) {
-            free_callback(*key);
+            free_callback((sds)(key->buf));
         }
     }
 
diff --git a/src/entry.c b/src/entry.c
index 097a36387c..ad69f5cff4 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -43,6 +43,10 @@ enum {
      * pointer located in memory before the embedded field. If unset, the entry
      * instead has an embedded value located after the embedded field. */
     FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR = 1,
+    /* SDS aux flag. If set, it indicates that the hash entry's value is a StringViewValue,
+     * meaning it's a char* and size_t managed externally by a module.
+     * This bit is mutually exclusive with the actual value being a normal sds. */
+    FIELD_SDS_AUX_BIT_VIEW_VALUE = 2,
     FIELD_SDS_AUX_BIT_MAX
 };
 static_assert(FIELD_SDS_AUX_BIT_MAX < sizeof(char) - SDS_TYPE_BITS, "too many sds bits are used for entry metadata");
@@ -59,6 +63,11 @@ bool entryHasEmbeddedValue(entry *entry) {
     return (!entryHasValuePtr(entry));
 }
 
+/* Returns true in case the entry holds a view of the value.
+ * Returns false otherwise. */
+bool entryIsStringViewValue(const entry *e) {
+    return sdsGetAuxBit(e, FIELD_SDS_AUX_BIT_VIEW_VALUE);
+}
 /* Returns true in case the entry has expiration timestamp.
  * Returns false otherwise. */
 bool entryHasExpiry(const entry *entry) {
@@ -70,6 +79,31 @@ sds entryGetField(const entry *entry) {
     return (sds)entry;
 }
 
+/* Create an entry for a view value.
+ * The 'value' (buf and len) is not owned by hash but keeps just a view of the
+ * buffer. */
+entry *createStringViewEntry(sds field, const char *buf, size_t len) {
+    StringViewValue *ext_value = zmalloc(sizeof(StringViewValue));
+    ext_value->buf = buf;
+    ext_value->len = len;
+
+    size_t field_len = sdslen(field);
+    char field_sds_type = sdsReqType(field_len);
+    if (field_sds_type == SDS_TYPE_5) field_sds_type = SDS_TYPE_8; // Ensure we can set aux bits
+    size_t field_size = sdsReqSize(field_len, field_sds_type);
+
+    size_t alloc_size = sizeof(void *) + field_size; // Store pointer to StringViewValue
+    char *alloc_buf = zmalloc(alloc_size);
+
+    *(void **)alloc_buf = ext_value; // Store the pointer to our struct
+
+    sds embedded_field_sds = sdswrite(alloc_buf + sizeof(void *), field_size, field_sds_type, field, field_len);
+
+    sdsSetAuxBit(embedded_field_sds, FIELD_SDS_AUX_BIT_VIEW_VALUE, 1);          // Mark as view value
+    serverAssert(entryIsStringViewValue(embedded_field_sds));
+
+    return (entry *)embedded_field_sds;
+}
 /* Returns the location of a pointer to a separately allocated value. Only for
  * an entry without an embedded value. */
 static sds *entryGetValueRef(const entry *entry) {
@@ -79,15 +113,31 @@ static sds *entryGetValueRef(const entry *entry) {
     return (sds *)field_data;
 }
 
-/* Returns the sds of the entry's value. */
-sds entryGetValue(const entry *entry) {
+StringViewValue *entryGetViewValueRef(const entry *entry) {
+    serverAssert(entryIsStringViewValue(entry));
+    char *field_data = sdsAllocPtr(entry);
+    field_data -= sizeof(StringViewValue);
+    return (StringViewValue *)field_data;
+}
+
+/* Returns the entry's value. */
+char *entryGetValue(const entry *entry, size_t *len) {
+    if (entryIsStringViewValue(entry)) {
+        serverAssert(entryHasValuePtr(entry)); // StringView must use value pointer
+        StringViewValue *ext_value = entryGetViewValueRef(entry);
+        *len = ext_value->len;
+        return (char *)ext_value->buf;
+    }
     if (entryHasValuePtr(entry)) {
-        return *entryGetValueRef(entry);
-    } else {
-        /* Skip field content, field null terminator and value sds8 hdr. */
-        size_t offset = sdslen(entry) + 1 + sdsHdrSize(SDS_TYPE_8);
-        return (char *)entry + offset;
+        sds *value = entryGetValueRef(entry);
+        *len = sdslen(*value);
+        return *value;
     }
+    /* Skip field content, field null terminator and value sds8 hdr. */
+    size_t offset = sdslen(entry) + 1 + sdsHdrSize(SDS_TYPE_8);
+    sds value = (char *)entry + offset;
+    *len = sdslen(value);
+    return value;
 }
 
 /* Modify the value of this entry and return a pointer to the (potentially new) entry.
@@ -107,7 +157,11 @@ entry *entrySetValue(entry *e, sds value) {
 /* Returns the address of the entry allocation. */
 void *entryGetAllocPtr(const entry *entry) {
     char *buf = sdsAllocPtr(entry);
-    if (entryHasValuePtr(entry)) buf -= sizeof(sds);
+    if (entryIsStringViewValue(entry)) {
+        buf -= sizeof(StringViewValue *);
+    } else if (entryHasValuePtr(entry)) {
+        buf -= sizeof(sds);
+    }
     if (entryHasExpiry(entry)) buf -= sizeof(long long);
     return buf;
 }
@@ -149,14 +203,24 @@ bool entryIsExpired(entry *entry) {
 /**************************************** Entry Expiry API - End *****************************************/
 
 void entryFree(entry *entry) {
+  /*
+    if (entryIsStringViewValue(entry)) {
+        StringViewValue *ext_value = entryGetViewValueRef(entry);
+        zfree(ext_value);
+    } else */
     if (entryHasValuePtr(entry)) {
-        sdsfree(entryGetValue(entry));
+        size_t len;
+        sdsfree(entryGetValue(entry, &len));
+    }
+    /*else if (entryHasValuePtr(entry)) {
+        sdsfree(*entryGetValueRef(entry));
     }
+    */
     zfree(entryGetAllocPtr(entry));
 }
 
-static inline size_t entryReqSize(const_sds field,
-                                  sds value,
+static inline size_t entryReqSize(size_t field_len,
+                                  size_t value_len,
                                   long long expiry,
                                   bool *is_value_embedded,
                                   int *field_sds_type,
@@ -164,17 +228,15 @@ static inline size_t entryReqSize(const_sds field,
                                   size_t *expiry_size,
                                   size_t *embedded_value_size) {
     size_t expiry_alloc_size = (expiry == EXPIRY_NONE) ? 0 : sizeof(long long);
-    size_t field_len = sdslen(field);
     int embedded_field_sds_type = sdsReqType(field_len);
     if (embedded_field_sds_type == SDS_TYPE_5 && (expiry_alloc_size > 0)) {
         embedded_field_sds_type = SDS_TYPE_8;
     }
     size_t field_alloc_size = sdsReqSize(field_len, embedded_field_sds_type);
-    size_t value_len = value ? sdslen(value) : 0;
-    size_t embedded_value_alloc_size = value ? sdsReqSize(value_len, SDS_TYPE_8) : 0;
+    size_t embedded_value_alloc_size = value_len != SIZE_MAX ? sdsReqSize(value_len, SDS_TYPE_8) : 0;
     size_t alloc_size = field_alloc_size + expiry_alloc_size;
     bool embed_value = false;
-    if (value) {
+    if (value_len != SIZE_MAX) {
         if (alloc_size + embedded_value_alloc_size <= EMBED_VALUE_MAX_ALLOC_SIZE) {
             /* Embed field and value. Value is fixed to SDS_TYPE_8. Unused
              * allocation space is recorded in the embedded value's SDS header.
@@ -219,6 +281,7 @@ static inline size_t entryReqSize(const_sds field,
 static entry *entryWrite(char *buf,
                          size_t buf_size,
                          const_sds field,
+                         size_t field_len,
                          sds value,
                          long long expiry,
                          bool embed_value,
@@ -244,7 +307,7 @@ static entry *entryWrite(char *buf,
         }
     }
     /* Set the field data */
-    entry *new_entry = sdswrite(buf, embedded_field_sds_size, embedded_field_sds_type, field, sdslen(field));
+    entry *new_entry = sdswrite(buf, embedded_field_sds_size, embedded_field_sds_type, field, field_len);
 
     /* Field sds aux bits are zero, which we use for this entry encoding. */
     sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR, embed_value ? 0 : 1);
@@ -257,17 +320,18 @@ static entry *entryWrite(char *buf,
 }
 
 /* Takes ownership of value. does not take ownership of field */
-entry *entryCreate(const_sds field, sds value, long long expiry) {
+entry *entryCreate(const char *field, size_t field_len, sds value, long long expiry) {
     bool embed_value = false;
     int embedded_field_sds_type;
     size_t expiry_size, embedded_value_sds_size, embedded_field_sds_size;
-    size_t alloc_size = entryReqSize(field, value, expiry, &embed_value, &embedded_field_sds_type, &embedded_field_sds_size, &expiry_size, &embedded_value_sds_size);
+    size_t value_len = value ? sdslen(value) : SIZE_MAX;
+    size_t alloc_size = entryReqSize(field_len, value_len, expiry, &embed_value, &embedded_field_sds_type, &embedded_field_sds_size, &expiry_size, &embedded_value_sds_size);
     size_t buf_size;
 
     /* allocate the buffer */
     char *buf = zmalloc_usable(alloc_size, &buf_size);
 
-    return entryWrite(buf, buf_size, field, value, expiry, embed_value, embedded_field_sds_type, embedded_field_sds_size, embedded_value_sds_size, expiry_size);
+    return entryWrite(buf, buf_size, field, field_len, value, expiry, embed_value, embedded_field_sds_type, embedded_field_sds_size, embedded_value_sds_size, expiry_size);
 }
 
 /* Modify the entry's value and/or expiration time.
@@ -284,12 +348,16 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
     /* Just a sanity check. If nothing changes, lets just return */
     if (!update_value && !update_expiry)
         return e;
-
-    if (!value) value = entryGetValue(e);
+    size_t value_len = SIZE_MAX;
+    if (value) {
+        value_len = sdslen(value);
+    } else {
+        value = entryGetValue(e, &value_len);
+    }
     bool embed_value = false;
     int embedded_field_sds_type;
     size_t expiry_size, embedded_value_size, embedded_field_size;
-    size_t required_entry_size = entryReqSize(field, value, expiry, &embed_value, &embedded_field_sds_type, &embedded_field_size, &expiry_size, &embedded_value_size);
+    size_t required_entry_size = entryReqSize(sdslen(field), value_len, expiry, &embed_value, &embedded_field_sds_type, &embedded_field_size, &expiry_size, &embedded_value_size);
     size_t current_embedded_allocation_size = entryHasValuePtr(e) ? 0 : entryMemUsage(e);
 
     bool expiry_add_remove = update_expiry && (curr_expiration_time == EXPIRY_NONE || expiry == EXPIRY_NONE); // In case we are toggling expiration
@@ -321,7 +389,8 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
                 *value_ref = value;
             } else {
                 /* Skip field content, field null terminator and value sds8 hdr. */
-                sds old_value = entryGetValue(e);
+                size_t len;
+                char *old_value = entryGetValue(e, &len);
                 /* We are using the same entry memory in order to store a potentially new value.
                  * In such cases the old value alloc was adjusted to the real buffer size part it was embedded to.
                  * Since we can potentially write here a smaller value, which requires less allocation space, we would like to
@@ -329,6 +398,7 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
                 size_t value_size = sdsHdrSize(SDS_TYPE_8) + sdsalloc(old_value) + 1;
                 sdswrite(sdsAllocPtr(old_value), value_size, SDS_TYPE_8, value, sdslen(value));
                 sdsfree(value);
+                sdsSetAuxBit(e, FIELD_SDS_AUX_BIT_VIEW_VALUE, 0);
             }
         }
         new_entry = e;
@@ -349,13 +419,14 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
         /* allocate the buffer for a new entry */
         size_t buf_size;
         char *buf = zmalloc_usable(required_entry_size, &buf_size);
-        new_entry = entryWrite(buf, buf_size, entryGetField(e), value, expiry, embed_value, embedded_field_sds_type, embedded_field_size, embedded_value_size, expiry_size);
+        new_entry = entryWrite(buf, buf_size, field, sdslen(field), value, expiry, embed_value, embedded_field_sds_type, embedded_field_size, embedded_value_size, expiry_size);
         debugServerAssert(new_entry != e);
         entryFree(e);
     }
     /* Check that the new entry was built correctly */
     debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR) == (embed_value ? 0 : 1));
     debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_EXPIRY) == (expiry_size > 0 ? 1 : 0));
+    debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE) == 0);
     serverAssert(new_entry);
     return new_entry;
 }
@@ -363,8 +434,14 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
 /* Returns memory usage of a entry, including all allocations owned by
  * the entry. */
 size_t entryMemUsage(entry *entry) {
-    size_t mem = 0;
+    if (entryIsStringViewValue(entry)) {
+        size_t mem = zmalloc_usable_size(entryGetAllocPtr(entry));
+        StringViewValue *ext_value = entryGetViewValueRef(entry);
+        mem += zmalloc_usable_size(ext_value);
+        return mem;
+    }
 
+    size_t mem = 0;
     if (entryHasValuePtr(entry)) {
         /* In case the value is not embedded we might not be able to sum all the allocation sizes since the field
          * header could be too small for holding the real allocation size. */
@@ -373,7 +450,8 @@ size_t entryMemUsage(entry *entry) {
         mem += sdsReqSize(sdslen(entry), sdsType(entry));
         if (entryHasExpiry(entry)) mem += sizeof(long long);
     }
-    mem += sdsAllocSize(entryGetValue(entry));
+    size_t len;
+    mem += sdsAllocSize((sds)entryGetValue(entry, &len));
     return mem;
 }
 
@@ -385,6 +463,7 @@ size_t entryMemUsage(entry *entry) {
  * If the location of the entry changed we return the new location,
  * otherwise we return NULL. */
 entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) {
+    if (entryIsStringViewValue(entry)) return NULL;
     if (entryHasValuePtr(entry)) {
         sds *value_ref = entryGetValueRef(entry);
         sds new_value = sdsdefragfn(*value_ref);
@@ -403,6 +482,7 @@ entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(s
 /* Used for releasing memory to OS to avoid unnecessary CoW. Called when we've
  * forked and memory won't be used again. See zmadvise_dontneed() */
 void entryDismissMemory(entry *entry) {
+    if (entryIsStringViewValue(entry)) return;
     /* Only dismiss values memory since the field size usually is small. */
     if (entryHasValuePtr(entry)) {
         dismissSds(*entryGetValueRef(entry));
diff --git a/src/entry.h b/src/entry.h
index f23f3dfc7b..ba7be52905 100644
--- a/src/entry.h
+++ b/src/entry.h
@@ -43,6 +43,17 @@
  * - Used for large value sizes. */
 typedef void entry;
 
+/* Structure representing an externalized string.
+ * This allows modules to store a char* and length directly in a hash field,
+ * bypassing normal SDS string allocation for the value.
+ * The module using this structure is responsible for the lifetime management
+ * of the memory pointed to by 'buf'. Valkey core will not free 'buf'.
+ */
+typedef struct StringViewValue {
+    const char *buf; /* Pointer to the externalized buffer */
+    size_t len;      /* Length of the buffer */
+} StringViewValue;
+
 /* The maximum allocation size we want to use for entries with embedded
  * values. */
 #define EMBED_VALUE_MAX_ALLOC_SIZE 128
@@ -51,7 +62,7 @@ typedef void entry;
 sds entryGetField(const entry *entry);
 
 /* Returns the value string (sds) from the entry. */
-sds entryGetValue(const entry *entry);
+char *entryGetValue(const entry *entry, size_t *len);
 
 /* Sets or replaces the value string in the entry. May reallocate and return a new pointer. */
 entry *entrySetValue(entry *entry, sds value);
@@ -62,6 +73,9 @@ long long entryGetExpiry(const entry *entry);
 /* Returns true if the entry has an expiration timestamp set. */
 bool entryHasExpiry(const entry *entry);
 
+/* Returns true if the entry value is externalized. */
+bool entryIsStringViewValue(const entry *e);
+
 /* Sets the expiration timestamp. */
 entry *entrySetExpiry(entry *entry, long long expiry);
 
@@ -72,7 +86,9 @@ bool entryIsExpired(entry *entry);
 void entryFree(entry *entry);
 
 /* Creates a new entry with the given field, value, and optional expiry. */
-entry *entryCreate(const_sds field, sds value, long long expiry);
+entry *entryCreate(const char *field, size_t field_len, sds value, long long expiry);
+entry *createStringViewEntry(sds field, const char *buf, size_t len);
+StringViewValue *entryGetViewValueRef(const entry *e);
 
 /* Updates the value and/or expiry of an existing entry.
  * In case value is NULL, will use the existing entry value.
diff --git a/src/module.c b/src/module.c
index 5b836f1186..fd1ade5438 100644
--- a/src/module.c
+++ b/src/module.c
@@ -5251,6 +5251,13 @@ int VM_ZsetRangePrev(ValkeyModuleKey *key) {
  * See also VM_ValueLength(), which returns the number of fields in a hash.
  * -------------------------------------------------------------------------- */
 
+/* Sets a view of a buffer to a hash field.
+ * The function takes the hash key, hash field, and a buffer along with its length. */
+int VM_HashSetValueView(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) {
+    if (key->value == NULL) moduleCreateEmptyKey(key, VALKEYMODULE_KEYTYPE_HASH);
+    if (!key || key->value->type != OBJ_HASH || !field || !buf) return VALKEYMODULE_ERR;
+    return hashTypeSetValueView(key->value, field->ptr, buf, len);
+}
 /* Set the field of the specified hash field to the specified value.
  * If the key is an empty key open for writing, it is created with an empty
  * hash value, in order to set the specified field.
@@ -11245,8 +11252,9 @@ static void moduleScanKeyHashtableCallback(void *privdata, void *entry) {
         value = createStringObjectFromLongDouble(node->score, 0);
     } else if (o->type == OBJ_HASH) {
         key = entryGetField(entry);
-        sds val = entryGetValue(entry);
-        value = createStringObject(val, sdslen(val));
+        size_t val_len;
+        char *val = entryGetValue(entry, &val_len);
+        value = createStringObject(val, val_len);
     } else {
         serverPanic("unexpected object type");
     }
@@ -13989,6 +13997,7 @@ void moduleRegisterCoreAPI(void) {
     REGISTER_API(ZsetRangeEndReached);
     REGISTER_API(HashSet);
     REGISTER_API(HashGet);
+    REGISTER_API(HashSetValueView);
     REGISTER_API(StreamAdd);
     REGISTER_API(StreamDelete);
     REGISTER_API(StreamIteratorStart);
diff --git a/src/rdb.c b/src/rdb.c
index 397d0a40a6..4d1d25b409 100644
--- a/src/rdb.c
+++ b/src/rdb.c
@@ -975,14 +975,15 @@ ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid) {
             void *next;
             while (hashtableNext(&iter, &next)) {
                 sds field = entryGetField(next);
-                sds value = entryGetValue(next);
+                size_t value_len;
+                unsigned char *value = (unsigned char *)entryGetValue(next, &value_len);
 
                 if ((n = rdbSaveRawString(rdb, (unsigned char *)field, sdslen(field))) == -1) {
                     hashtableResetIterator(&iter);
                     return -1;
                 }
                 nwritten += n;
-                if ((n = rdbSaveRawString(rdb, (unsigned char *)value, sdslen(value))) == -1) {
+                if ((n = rdbSaveRawString(rdb, value, value_len)) == -1) {
                     hashtableResetIterator(&iter);
                     return -1;
                 }
@@ -2144,7 +2145,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) {
                 (sdslen(field) > server.hash_max_listpack_value || sdslen(value) > server.hash_max_listpack_value ||
                  !lpSafeToAdd(o->ptr, sdslen(field) + sdslen(value)))) {
                 hashTypeConvert(o, OBJ_ENCODING_HASHTABLE);
-                entry *entry = entryCreate(field, value, EXPIRY_NONE);
+                entry *entry = entryCreate(field, sdslen(field), value, EXPIRY_NONE);
                 sdsfree(field);
                 if (!hashtableAdd((hashtable *)o->ptr, entry)) {
                     rdbReportCorruptRDB("Duplicate hash fields detected");
@@ -2202,7 +2203,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) {
             }
 
             /* Add pair to hash table */
-            entry *entry = entryCreate(field, value, itemexpiry);
+            entry *entry = entryCreate(field, sdslen(field), value, itemexpiry);
             sdsfree(field);
             if (!hashtableAdd((hashtable *)o->ptr, entry)) {
                 rdbReportCorruptRDB("Duplicate hash fields detected");
diff --git a/src/server.h b/src/server.h
index 06ef8fc9da..59f47809bf 100644
--- a/src/server.h
+++ b/src/server.h
@@ -3412,13 +3412,14 @@ void hashTypeCurrentFromListpack(hashTypeIterator *hi,
                                  unsigned char **vstr,
                                  unsigned int *vlen,
                                  long long *vll);
-sds hashTypeCurrentFromHashTable(hashTypeIterator *hi, int what);
+char *hashTypeCurrentFromHashTable(hashTypeIterator *hi, int what, size_t *len);
 sds hashTypeCurrentObjectNewSds(hashTypeIterator *hi, int what);
 robj *hashTypeLookupWriteOrCreate(client *c, robj *key);
 robj *hashTypeGetValueObject(robj *o, sds field);
 int hashTypeSet(robj *o, sds field, sds value, long long expiry, int flags);
 robj *hashTypeDup(robj *o);
 bool hashTypeHasVolatileFields(robj *o);
+int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len);
 
 /* Pub / Sub */
 int pubsubUnsubscribeAllChannels(client *c, int notify);
diff --git a/src/t_hash.c b/src/t_hash.c
index 84a0300ca1..60403506d1 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -235,10 +235,11 @@ int hashTypeGetValue(robj *o, sds field, unsigned char **vstr, unsigned int *vle
         void *entry = NULL;
         hashtableFind(o->ptr, field, &entry);
         if (entry) {
-            sds value = entryGetValue(entry);
+            size_t len = 0;
+            char *value = entryGetValue(entry, &len);
             serverAssert(value != NULL);
             *vstr = (unsigned char *)value;
-            *vlen = sdslen(value);
+            *vlen = len;
             if (expiry) *expiry = entryGetExpiry(entry);
             return C_OK;
         }
@@ -310,6 +311,39 @@ int hashTypeExists(robj *o, sds field) {
     return hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) == C_OK;
 }
 
+/* Set an externalized string field in a hash.
+ * Returns 0 on insert, 1 on update.
+ * Assumes the key 'o' is already a hash or a new key.
+ * The 'field' sds is consumed by this function.
+ */
+int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len) {
+    if (o->encoding == OBJ_ENCODING_LISTPACK) {
+        // StringView require HASHTABLE encoding due to aux bits and pointer storage.
+        hashTypeConvert(o, OBJ_ENCODING_HASHTABLE);
+    }
+
+    hashtable *ht = o->ptr;
+    hashtablePosition position;
+    void *existing;
+    if (hashtableFindPositionForInsert(ht, field, &position, &existing)) {
+        /* does not exist yet */
+        entry *e = createStringViewEntry(field, buf, len);
+        hashtableInsertAtPosition(ht, e, &position);
+        return 0;
+    }
+    if (entryIsStringViewValue(existing)) {
+        StringViewValue *ext_value = entryGetViewValueRef(existing);
+        ext_value->buf = (char *)buf;
+        ext_value->len = len;
+        return 1;
+    }
+    entry *new_entry = createStringViewEntry(field, buf, len);
+    int replaced = hashtableReplaceReallocatedEntry(ht, existing, new_entry);
+    serverAssert(replaced);
+    entryFree(existing);
+    return 1;
+}
+
 /* Add a new field, overwrite the old with the new value if it already exists.
  * Return 0 on insert and 1 on update.
  *
@@ -384,7 +418,7 @@ int hashTypeSet(robj *o, sds field, sds value, long long expiry, int flags) {
         void *existing;
         if (hashtableFindPositionForInsert(ht, field, &position, &existing)) {
             /* does not exist yet */
-            entry *entry = entryCreate(field, v, expiry);
+            entry *entry = entryCreate(field, sdslen(field), v, expiry);
             hashtableInsertAtPosition(ht, entry, &position);
             /* In case an expiry is set on the new entry, we need to track it */
             if (expiry != EXPIRY_NONE) {
@@ -675,49 +709,31 @@ void hashTypeCurrentFromListpack(hashTypeIterator *hi,
 /* Get the field or value at iterator cursor, for an iterator on a hash value
  * encoded as a hash table. Prototype is similar to
  * `hashTypeGetFromHashTable`. */
-sds hashTypeCurrentFromHashTable(hashTypeIterator *hi, int what) {
+char *hashTypeCurrentFromHashTable(hashTypeIterator *hi, int what, size_t *len) {
     serverAssert(hi->encoding == OBJ_ENCODING_HASHTABLE);
 
     if (what & OBJ_HASH_FIELD) {
-        return entryGetField(hi->next);
-    } else {
-        return entryGetValue(hi->next);
-    }
-}
-
-/* Higher level function of hashTypeCurrent*() that returns the hash value
- * at current iterator position.
- *
- * The returned element is returned by reference in either *vstr and *vlen if
- * it's returned in string form, or stored in *vll if it's returned as
- * a number.
- *
- * If *vll is populated *vstr is set to NULL, so the caller
- * can always check the function return by checking the return value
- * type checking if vstr == NULL. */
-static void hashTypeCurrentObject(hashTypeIterator *hi, int what, unsigned char **vstr, unsigned int *vlen, long long *vll) {
-    if (hi->encoding == OBJ_ENCODING_LISTPACK) {
-        *vstr = NULL;
-        hashTypeCurrentFromListpack(hi, what, vstr, vlen, vll);
-    } else if (hi->encoding == OBJ_ENCODING_HASHTABLE) {
-        sds ele = hashTypeCurrentFromHashTable(hi, what);
-        *vstr = (unsigned char *)ele;
-        *vlen = sdslen(ele);
-    } else {
-        serverPanic("Unknown hash encoding");
+        sds key = entryGetField(hi->next);
+        *len = sdslen(key);
+        return key;
     }
+    return entryGetValue(hi->next, len);
 }
 
 /* Return the field or value at the current iterator position as a new
  * SDS string. */
 sds hashTypeCurrentObjectNewSds(hashTypeIterator *hi, int what) {
-    unsigned char *vstr;
-    unsigned int vlen;
-    long long vll;
-
-    hashTypeCurrentObject(hi, what, &vstr, &vlen, &vll);
-    if (vstr) return sdsnewlen(vstr, vlen);
-    return sdsfromlonglong(vll);
+    unsigned char *vstr = NULL;
+    if (hi->encoding == OBJ_ENCODING_LISTPACK) {
+        long long vll;
+        unsigned int vlen;
+        hashTypeCurrentFromListpack(hi, what, &vstr, &vlen, &vll);
+        if (vstr) return sdsnewlen(vstr, vlen);
+        return sdsfromlonglong(vll);
+    }
+    size_t vlen;
+    vstr = (unsigned char *)hashTypeCurrentFromHashTable(hi, what, &vlen);
+    return sdsnewlen(vstr, vlen);
 }
 
 robj *hashTypeLookupWriteOrCreate(client *c, robj *key) {
@@ -750,7 +766,7 @@ void hashTypeConvertListpack(robj *o, int enc) {
         while (hashTypeNext(&hi) != C_ERR) {
             sds field = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_FIELD);
             sds value = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_VALUE);
-            entry *entry = entryCreate(field, value, EXPIRY_NONE);
+            entry *entry = entryCreate(field, sdslen(field), value, EXPIRY_NONE);
             sdsfree(field);
             if (!hashtableAdd(ht, entry)) {
                 entryFree(entry);
@@ -805,11 +821,13 @@ robj *hashTypeDup(robj *o) {
         hashTypeInitIterator(o, &hi);
         while (hashTypeNext(&hi) != C_ERR) {
             /* Extract a field-value pair from an original hash object.*/
-            sds field = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_FIELD);
-            sds value = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_VALUE);
+            size_t field_str_len, value_str_len;
+            char *field_str = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_FIELD, &field_str_len);
+            char *value_str = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_VALUE, &value_str_len);
             long long expiry = entryGetExpiry(hi.next);
             /* Add a field-value pair to a new hash object. */
-            entry *entry = entryCreate(field, sdsdup(value), expiry);
+            sds value = sdsnewlen(value_str, value_str_len);
+            entry *entry = entryCreate(field_str, field_str_len, value, expiry);
             hashtableAdd(ht, entry);
             if (expiry != EXPIRY_NONE)
                 hashTypeTrackEntry(hobj, entry);
@@ -859,11 +877,7 @@ static void hashTypeRandomElement(robj *hashobj, unsigned long hashsize, listpac
             field->sval = (unsigned char *)sds_field;
             field->slen = sdslen(sds_field);
             if (val) {
-                entry *hash_entry = e;
-                sds sds_val = entryGetValue(hash_entry);
-                val->sval = (unsigned char *)sds_val;
-                val->slen =
-                    sdslen(sds_val);
+                val->sval = (unsigned char *)entryGetValue(e, (size_t *)&val->slen);
             }
         }
         hashTypeIgnoreTTL(hashobj, false);
@@ -1073,8 +1087,9 @@ static void addHashIteratorCursorToReply(writePreparedClient *wpc, hashTypeItera
         else
             addWritePreparedReplyBulkLongLong(wpc, vll);
     } else if (hi->encoding == OBJ_ENCODING_HASHTABLE) {
-        sds value = hashTypeCurrentFromHashTable(hi, what);
-        addWritePreparedReplyBulkCBuffer(wpc, value, sdslen(value));
+        size_t len;
+        char *value = hashTypeCurrentFromHashTable(hi, what, &len);
+        addWritePreparedReplyBulkCBuffer(wpc, value, len);
     } else {
         serverPanic("Unknown hash encoding");
     }
@@ -2008,10 +2023,11 @@ void hrandfieldWithCountCommand(client *c, long l, int withvalues) {
         void *next;
         while (hashtableNext(&iter, &next)) {
             sds field = entryGetField(next);
-            sds value = entryGetValue(next);
+            size_t value_len;
+            char *value = entryGetValue(next, &value_len);
             if (withvalues && c->resp > 2) addWritePreparedReplyArrayLen(wpc, 2);
             addWritePreparedReplyBulkCBuffer(wpc, field, sdslen(field));
-            if (withvalues) addWritePreparedReplyBulkCBuffer(wpc, value, sdslen(value));
+            if (withvalues) addWritePreparedReplyBulkCBuffer(wpc, value, value_len);
         }
 
         hashtableResetIterator(&iter);
diff --git a/src/valkeymodule.h b/src/valkeymodule.h
index cbb3e9ffa0..ce1dd5d385 100644
--- a/src/valkeymodule.h
+++ b/src/valkeymodule.h
@@ -1384,6 +1384,7 @@ VALKEYMODULE_API int (*ValkeyModule_ZsetRangePrev)(ValkeyModuleKey *key) VALKEYM
 VALKEYMODULE_API int (*ValkeyModule_ZsetRangeEndReached)(ValkeyModuleKey *key) VALKEYMODULE_ATTR;
 VALKEYMODULE_API int (*ValkeyModule_HashSet)(ValkeyModuleKey *key, int flags, ...) VALKEYMODULE_ATTR;
 VALKEYMODULE_API int (*ValkeyModule_HashGet)(ValkeyModuleKey *key, int flags, ...) VALKEYMODULE_ATTR;
+VALKEYMODULE_API int (*ValkeyModule_HashSetValueView)(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) VALKEYMODULE_ATTR;
 VALKEYMODULE_API int (*ValkeyModule_StreamAdd)(ValkeyModuleKey *key,
                                                int flags,
                                                ValkeyModuleStreamID *id,
@@ -2028,6 +2029,7 @@ static int ValkeyModule_Init(ValkeyModuleCtx *ctx, const char *name, int ver, in
     VALKEYMODULE_GET_API(ZsetRangeEndReached);
     VALKEYMODULE_GET_API(HashSet);
     VALKEYMODULE_GET_API(HashGet);
+    VALKEYMODULE_GET_API(HashSetValueView);
     VALKEYMODULE_GET_API(StreamAdd);
     VALKEYMODULE_GET_API(StreamDelete);
     VALKEYMODULE_GET_API(StreamIteratorStart);
diff --git a/src/ziplist.c b/src/ziplist.c
index 608487fa2b..1198c86f0e 100644
--- a/src/ziplist.c
+++ b/src/ziplist.c
@@ -740,7 +740,7 @@ unsigned char *ziplistResize(unsigned char *zl, size_t len) {
  * updated, i.e. consecutive fields MAY need an update. */
 unsigned char *__ziplistCascadeUpdate(unsigned char *zl, unsigned char *p) {
     zlentry cur;
-    size_t prevlen, prevlensize, prevoffset; /* Informat of the last changed entry. */
+    size_t prevlen, prevlensize, prevoffset; /* Information of the last changed entry. */
     size_t firstentrylen;                    /* Used to handle insert at head. */
     size_t rawlen, curlen = intrev32ifbe(ZIPLIST_BYTES(zl));
     size_t extra = 0, cnt = 0, offset;
diff --git a/tests/modules/Makefile b/tests/modules/Makefile
index 1d35dfac82..90ecc50571 100644
--- a/tests/modules/Makefile
+++ b/tests/modules/Makefile
@@ -48,6 +48,7 @@ TEST_MODULES = \
     defragtest.so \
     keyspecs.so \
     hash.so \
+    hash_view_value.so \
     zset.so \
     stream.so \
     mallocsize.so \
diff --git a/tests/modules/hash_view_value.c b/tests/modules/hash_view_value.c
new file mode 100644
index 0000000000..2bbb2a1152
--- /dev/null
+++ b/tests/modules/hash_view_value.c
@@ -0,0 +1,64 @@
+#include "valkeymodule.h"
+#include <string.h>
+
+typedef struct bufferNode {
+    char *buf;
+    size_t len;
+    struct bufferNode *next;
+} bufferNode;
+
+bufferNode *head = NULL;
+
+bufferNode *addBuffer(const char *buf, size_t len) {
+    if (!buf || len == 0) return NULL;
+
+    bufferNode *node = malloc(sizeof(bufferNode));
+    node->buf = malloc(len);
+    memcpy(node->buf, buf, len);
+    node->len = len;
+    node->next = head;
+    head = node;
+    return node;
+}
+
+void freeBufferList(void) {
+    bufferNode *current = head;
+    while (current) {
+        bufferNode *next = current->next;
+        free(current->buf);
+        free(current);
+        current = next;
+    }
+}
+
+int hashSetValueView(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {
+    if (argc != 4) return ValkeyModule_WrongArity(ctx);
+
+    ValkeyModule_AutoMemory(ctx);
+    ValkeyModuleKey *key = ValkeyModule_OpenKey(ctx, argv[1], VALKEYMODULE_WRITE);
+
+    size_t buf_len;
+    const char *buf = ValkeyModule_StringPtrLen(argv[3], &buf_len);
+    bufferNode *node = addBuffer(buf, buf_len);
+
+    int result = ValkeyModule_HashSetValueView(key, argv[2], node->buf, node->len);
+    return ValkeyModule_ReplyWithLongLong(ctx, result);
+}
+
+int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {
+    VALKEYMODULE_NOT_USED(argv);
+    VALKEYMODULE_NOT_USED(argc);
+    if (ValkeyModule_Init(ctx, "hash.set_view", 1, VALKEYMODULE_APIVER_1) ==
+        VALKEYMODULE_OK &&
+        ValkeyModule_CreateCommand(ctx, "hash.set_view", hashSetValueView, "write",
+                                  1, 1, 1) == VALKEYMODULE_OK) {
+        return VALKEYMODULE_OK;
+    }
+    return VALKEYMODULE_ERR;
+}
+
+int ValkeyModule_OnUnload(ValkeyModuleCtx *ctx) {
+    VALKEYMODULE_NOT_USED(ctx);
+    freeBufferList();
+    return VALKEYMODULE_OK;
+}
diff --git a/tests/unit/moduleapi/hash_view_value.tcl b/tests/unit/moduleapi/hash_view_value.tcl
new file mode 100644
index 0000000000..7ed715bae8
--- /dev/null
+++ b/tests/unit/moduleapi/hash_view_value.tcl
@@ -0,0 +1,21 @@
+set testmodule [file normalize tests/modules/hash_view_value.so]
+
+start_server {tags {"modules"}} {
+    r module load $testmodule
+
+    test {Module hash set} {
+        r hash.set_view k f hello
+        assert_equal "hello" [r hget k f]
+        r hgetall k
+        r hset k f hello1
+        r hash.set_view k f hello2
+        assert_equal "hello2" [r hget k f]
+        r hdel k f
+        r hash.set_view k f hello3
+        assert_equal "hello3" [r hget k f]
+    }
+
+    test "Unload the module - hash" {
+        assert_equal {OK} [r module unload hash.set_view]
+    }
+}
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index 9f8bde5fa3..3af1676a1b 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -1203,7 +1203,7 @@ start_server {tags {"scripting"}} {
         set rd [valkey_deferring_client]
         r config set lua-time-limit 10
 
-        # senging (in a pipeline):
+        # sending (in a pipeline):
         # 1. eval "while 1 do redis.call('ping') end" 0
         # 2. ping
         if {$is_eval == 1} {

From a9fc01754dd24dc5d2e8f2ccdc15b90d15e12dd1 Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Tue, 12 Aug 2025 04:08:56 +0000
Subject: [PATCH 02/30] fixing test issue

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/entry.c  | 26 +++++++++++++-------------
 src/entry.h  |  2 +-
 src/t_hash.c |  2 +-
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/entry.c b/src/entry.c
index ad69f5cff4..4d3cfd0a3d 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -65,7 +65,7 @@ bool entryHasEmbeddedValue(entry *entry) {
 
 /* Returns true in case the entry holds a view of the value.
  * Returns false otherwise. */
-bool entryIsStringViewValue(const entry *e) {
+bool entryHasViewValue(const entry *e) {
     return sdsGetAuxBit(e, FIELD_SDS_AUX_BIT_VIEW_VALUE);
 }
 /* Returns true in case the entry has expiration timestamp.
@@ -100,7 +100,7 @@ entry *createStringViewEntry(sds field, const char *buf, size_t len) {
     sds embedded_field_sds = sdswrite(alloc_buf + sizeof(void *), field_size, field_sds_type, field, field_len);
 
     sdsSetAuxBit(embedded_field_sds, FIELD_SDS_AUX_BIT_VIEW_VALUE, 1);          // Mark as view value
-    serverAssert(entryIsStringViewValue(embedded_field_sds));
+    serverAssert(entryHasViewValue(embedded_field_sds));
 
     return (entry *)embedded_field_sds;
 }
@@ -114,7 +114,7 @@ static sds *entryGetValueRef(const entry *entry) {
 }
 
 StringViewValue *entryGetViewValueRef(const entry *entry) {
-    serverAssert(entryIsStringViewValue(entry));
+    serverAssert(entryHasViewValue(entry));
     char *field_data = sdsAllocPtr(entry);
     field_data -= sizeof(StringViewValue);
     return (StringViewValue *)field_data;
@@ -122,7 +122,7 @@ StringViewValue *entryGetViewValueRef(const entry *entry) {
 
 /* Returns the entry's value. */
 char *entryGetValue(const entry *entry, size_t *len) {
-    if (entryIsStringViewValue(entry)) {
+    if (entryHasViewValue(entry)) {
         serverAssert(entryHasValuePtr(entry)); // StringView must use value pointer
         StringViewValue *ext_value = entryGetViewValueRef(entry);
         *len = ext_value->len;
@@ -130,7 +130,9 @@ char *entryGetValue(const entry *entry, size_t *len) {
     }
     if (entryHasValuePtr(entry)) {
         sds *value = entryGetValueRef(entry);
-        *len = sdslen(*value);
+        if (*value) {
+            *len = sdslen(*value);
+        }
         return *value;
     }
     /* Skip field content, field null terminator and value sds8 hdr. */
@@ -157,7 +159,7 @@ entry *entrySetValue(entry *e, sds value) {
 /* Returns the address of the entry allocation. */
 void *entryGetAllocPtr(const entry *entry) {
     char *buf = sdsAllocPtr(entry);
-    if (entryIsStringViewValue(entry)) {
+    if (entryHasViewValue(entry)) {
         buf -= sizeof(StringViewValue *);
     } else if (entryHasValuePtr(entry)) {
         buf -= sizeof(sds);
@@ -203,12 +205,10 @@ bool entryIsExpired(entry *entry) {
 /**************************************** Entry Expiry API - End *****************************************/
 
 void entryFree(entry *entry) {
-  /*
-    if (entryIsStringViewValue(entry)) {
+    if (entryHasViewValue(entry)) {
         StringViewValue *ext_value = entryGetViewValueRef(entry);
         zfree(ext_value);
-    } else */
-    if (entryHasValuePtr(entry)) {
+    } else if (entryHasValuePtr(entry)) {
         size_t len;
         sdsfree(entryGetValue(entry, &len));
     }
@@ -434,7 +434,7 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
 /* Returns memory usage of a entry, including all allocations owned by
  * the entry. */
 size_t entryMemUsage(entry *entry) {
-    if (entryIsStringViewValue(entry)) {
+    if (entryHasViewValue(entry)) {
         size_t mem = zmalloc_usable_size(entryGetAllocPtr(entry));
         StringViewValue *ext_value = entryGetViewValueRef(entry);
         mem += zmalloc_usable_size(ext_value);
@@ -463,7 +463,7 @@ size_t entryMemUsage(entry *entry) {
  * If the location of the entry changed we return the new location,
  * otherwise we return NULL. */
 entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) {
-    if (entryIsStringViewValue(entry)) return NULL;
+    if (entryHasViewValue(entry)) return NULL;
     if (entryHasValuePtr(entry)) {
         sds *value_ref = entryGetValueRef(entry);
         sds new_value = sdsdefragfn(*value_ref);
@@ -482,7 +482,7 @@ entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(s
 /* Used for releasing memory to OS to avoid unnecessary CoW. Called when we've
  * forked and memory won't be used again. See zmadvise_dontneed() */
 void entryDismissMemory(entry *entry) {
-    if (entryIsStringViewValue(entry)) return;
+    if (entryHasViewValue(entry)) return;
     /* Only dismiss values memory since the field size usually is small. */
     if (entryHasValuePtr(entry)) {
         dismissSds(*entryGetValueRef(entry));
diff --git a/src/entry.h b/src/entry.h
index ba7be52905..eddca15ad0 100644
--- a/src/entry.h
+++ b/src/entry.h
@@ -74,7 +74,7 @@ long long entryGetExpiry(const entry *entry);
 bool entryHasExpiry(const entry *entry);
 
 /* Returns true if the entry value is externalized. */
-bool entryIsStringViewValue(const entry *e);
+bool entryHasViewValue(const entry *e);
 
 /* Sets the expiration timestamp. */
 entry *entrySetExpiry(entry *entry, long long expiry);
diff --git a/src/t_hash.c b/src/t_hash.c
index 60403506d1..9c11f165f6 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -331,7 +331,7 @@ int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len) {
         hashtableInsertAtPosition(ht, e, &position);
         return 0;
     }
-    if (entryIsStringViewValue(existing)) {
+    if (entryHasViewValue(existing)) {
         StringViewValue *ext_value = entryGetViewValueRef(existing);
         ext_value->buf = (char *)buf;
         ext_value->len = len;

From 7dae1e688e15c53e8b343e168c4cae0879798a2a Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Tue, 12 Aug 2025 14:57:11 +0000
Subject: [PATCH 03/30] addressing unittest issues

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/entry.c           | 84 ++++++++++++++-----------------------------
 src/entry.h           | 14 ++++----
 src/module.c          |  2 +-
 src/server.h          |  2 +-
 src/t_hash.c          | 13 +++----
 src/unit/test_entry.c | 30 ++++++++--------
 src/unit/test_vset.c  |  6 ++--
 7 files changed, 62 insertions(+), 89 deletions(-)

diff --git a/src/entry.c b/src/entry.c
index 4d3cfd0a3d..98828519f2 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -43,9 +43,10 @@ enum {
      * pointer located in memory before the embedded field. If unset, the entry
      * instead has an embedded value located after the embedded field. */
     FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR = 1,
-    /* SDS aux flag. If set, it indicates that the hash entry's value is a StringViewValue,
-     * meaning it's a char* and size_t managed externally by a module.
-     * This bit is mutually exclusive with the actual value being a normal sds. */
+    /* SDS aux flag.  If set, it indicates that the hash entry's value is a **view** of the data.
+     * The hash entry does not own the view buffer and will not free it upon
+     * entry destruction. This is useful for avoiding memory duplication
+     * between the core and a module. */
     FIELD_SDS_AUX_BIT_VIEW_VALUE = 2,
     FIELD_SDS_AUX_BIT_MAX
 };
@@ -79,31 +80,6 @@ sds entryGetField(const entry *entry) {
     return (sds)entry;
 }
 
-/* Create an entry for a view value.
- * The 'value' (buf and len) is not owned by hash but keeps just a view of the
- * buffer. */
-entry *createStringViewEntry(sds field, const char *buf, size_t len) {
-    StringViewValue *ext_value = zmalloc(sizeof(StringViewValue));
-    ext_value->buf = buf;
-    ext_value->len = len;
-
-    size_t field_len = sdslen(field);
-    char field_sds_type = sdsReqType(field_len);
-    if (field_sds_type == SDS_TYPE_5) field_sds_type = SDS_TYPE_8; // Ensure we can set aux bits
-    size_t field_size = sdsReqSize(field_len, field_sds_type);
-
-    size_t alloc_size = sizeof(void *) + field_size; // Store pointer to StringViewValue
-    char *alloc_buf = zmalloc(alloc_size);
-
-    *(void **)alloc_buf = ext_value; // Store the pointer to our struct
-
-    sds embedded_field_sds = sdswrite(alloc_buf + sizeof(void *), field_size, field_sds_type, field, field_len);
-
-    sdsSetAuxBit(embedded_field_sds, FIELD_SDS_AUX_BIT_VIEW_VALUE, 1);          // Mark as view value
-    serverAssert(entryHasViewValue(embedded_field_sds));
-
-    return (entry *)embedded_field_sds;
-}
 /* Returns the location of a pointer to a separately allocated value. Only for
  * an entry without an embedded value. */
 static sds *entryGetValueRef(const entry *entry) {
@@ -113,21 +89,14 @@ static sds *entryGetValueRef(const entry *entry) {
     return (sds *)field_data;
 }
 
-StringViewValue *entryGetViewValueRef(const entry *entry) {
+ViewValue *entryGetViewValueRef(const entry *entry) {
     serverAssert(entryHasViewValue(entry));
-    char *field_data = sdsAllocPtr(entry);
-    field_data -= sizeof(StringViewValue);
-    return (StringViewValue *)field_data;
+    size_t offset = sdslen(entry) + 1 + sdsHdrSize(SDS_TYPE_8);
+    return (ViewValue *)entry + offset;
 }
 
 /* Returns the entry's value. */
 char *entryGetValue(const entry *entry, size_t *len) {
-    if (entryHasViewValue(entry)) {
-        serverAssert(entryHasValuePtr(entry)); // StringView must use value pointer
-        StringViewValue *ext_value = entryGetViewValueRef(entry);
-        *len = ext_value->len;
-        return (char *)ext_value->buf;
-    }
     if (entryHasValuePtr(entry)) {
         sds *value = entryGetValueRef(entry);
         if (*value) {
@@ -137,6 +106,11 @@ char *entryGetValue(const entry *entry, size_t *len) {
     }
     /* Skip field content, field null terminator and value sds8 hdr. */
     size_t offset = sdslen(entry) + 1 + sdsHdrSize(SDS_TYPE_8);
+    if (entryHasViewValue(entry)) {
+        ViewValue *value = (ViewValue *)entry + offset;
+        *len = value->len;
+        return (char *)value->buf;
+    }
     sds value = (char *)entry + offset;
     *len = sdslen(value);
     return value;
@@ -159,9 +133,7 @@ entry *entrySetValue(entry *e, sds value) {
 /* Returns the address of the entry allocation. */
 void *entryGetAllocPtr(const entry *entry) {
     char *buf = sdsAllocPtr(entry);
-    if (entryHasViewValue(entry)) {
-        buf -= sizeof(StringViewValue *);
-    } else if (entryHasValuePtr(entry)) {
+    if (entryHasValuePtr(entry)) {
         buf -= sizeof(sds);
     }
     if (entryHasExpiry(entry)) buf -= sizeof(long long);
@@ -205,17 +177,10 @@ bool entryIsExpired(entry *entry) {
 /**************************************** Entry Expiry API - End *****************************************/
 
 void entryFree(entry *entry) {
-    if (entryHasViewValue(entry)) {
-        StringViewValue *ext_value = entryGetViewValueRef(entry);
-        zfree(ext_value);
-    } else if (entryHasValuePtr(entry)) {
+    if (entryHasValuePtr(entry)) {
         size_t len;
         sdsfree(entryGetValue(entry, &len));
     }
-    /*else if (entryHasValuePtr(entry)) {
-        sdsfree(*entryGetValueRef(entry));
-    }
-    */
     zfree(entryGetAllocPtr(entry));
 }
 
@@ -334,6 +299,17 @@ entry *entryCreate(const char *field, size_t field_len, sds value, long long exp
     return entryWrite(buf, buf_size, field, field_len, value, expiry, embed_value, embedded_field_sds_type, embedded_field_sds_size, embedded_value_sds_size, expiry_size);
 }
 
+/* Create an entry with a view value. The view value structure is stored as an embedded field.  */
+entry *createViewValueEntry(sds field, const char *buf, size_t len, long long expiry) {
+    ViewValue view_value = {buf, len};
+    sds value = sdsnewlen(&view_value, sizeof (ViewValue));
+    entry *new_entry = entryCreate(field, sdslen(field), value, expiry);
+    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE, 1);          // Mark as view value
+                                                                                //
+    debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR) == 0);
+    debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE) == 1);
+    return new_entry;
+}
 /* Modify the entry's value and/or expiration time.
  * In case the provided value is NULL, will use the existing value.
  * Note that the value ownership is moved to this function and the caller should assume the
@@ -398,7 +374,6 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
                 size_t value_size = sdsHdrSize(SDS_TYPE_8) + sdsalloc(old_value) + 1;
                 sdswrite(sdsAllocPtr(old_value), value_size, SDS_TYPE_8, value, sdslen(value));
                 sdsfree(value);
-                sdsSetAuxBit(e, FIELD_SDS_AUX_BIT_VIEW_VALUE, 0);
             }
         }
         new_entry = e;
@@ -423,6 +398,7 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
         debugServerAssert(new_entry != e);
         entryFree(e);
     }
+    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE, 0);
     /* Check that the new entry was built correctly */
     debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR) == (embed_value ? 0 : 1));
     debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_EXPIRY) == (expiry_size > 0 ? 1 : 0));
@@ -434,13 +410,6 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
 /* Returns memory usage of a entry, including all allocations owned by
  * the entry. */
 size_t entryMemUsage(entry *entry) {
-    if (entryHasViewValue(entry)) {
-        size_t mem = zmalloc_usable_size(entryGetAllocPtr(entry));
-        StringViewValue *ext_value = entryGetViewValueRef(entry);
-        mem += zmalloc_usable_size(ext_value);
-        return mem;
-    }
-
     size_t mem = 0;
     if (entryHasValuePtr(entry)) {
         /* In case the value is not embedded we might not be able to sum all the allocation sizes since the field
@@ -482,7 +451,6 @@ entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(s
 /* Used for releasing memory to OS to avoid unnecessary CoW. Called when we've
  * forked and memory won't be used again. See zmadvise_dontneed() */
 void entryDismissMemory(entry *entry) {
-    if (entryHasViewValue(entry)) return;
     /* Only dismiss values memory since the field size usually is small. */
     if (entryHasValuePtr(entry)) {
         dismissSds(*entryGetValueRef(entry));
diff --git a/src/entry.h b/src/entry.h
index eddca15ad0..aba2f46f9b 100644
--- a/src/entry.h
+++ b/src/entry.h
@@ -43,16 +43,16 @@
  * - Used for large value sizes. */
 typedef void entry;
 
-/* Structure representing an externalized string.
+/* Structure representing a view value.
  * This allows modules to store a char* and length directly in a hash field,
  * bypassing normal SDS string allocation for the value.
- * The module using this structure is responsible for the lifetime management
- * of the memory pointed to by 'buf'. Valkey core will not free 'buf'.
+ * The module is responsible for the lifetime management of the memory pointed
+ * to by 'buf'. Valkey core will not free 'buf'.
  */
-typedef struct StringViewValue {
+typedef struct ViewValue {
     const char *buf; /* Pointer to the externalized buffer */
     size_t len;      /* Length of the buffer */
-} StringViewValue;
+} ViewValue;
 
 /* The maximum allocation size we want to use for entries with embedded
  * values. */
@@ -87,8 +87,8 @@ void entryFree(entry *entry);
 
 /* Creates a new entry with the given field, value, and optional expiry. */
 entry *entryCreate(const char *field, size_t field_len, sds value, long long expiry);
-entry *createStringViewEntry(sds field, const char *buf, size_t len);
-StringViewValue *entryGetViewValueRef(const entry *e);
+entry *createViewValueEntry(sds field, const char *buf, size_t len, long long expiry);
+ViewValue *entryGetViewValueRef(const entry *e);
 
 /* Updates the value and/or expiry of an existing entry.
  * In case value is NULL, will use the existing entry value.
diff --git a/src/module.c b/src/module.c
index fd1ade5438..7af9988132 100644
--- a/src/module.c
+++ b/src/module.c
@@ -5256,7 +5256,7 @@ int VM_ZsetRangePrev(ValkeyModuleKey *key) {
 int VM_HashSetValueView(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) {
     if (key->value == NULL) moduleCreateEmptyKey(key, VALKEYMODULE_KEYTYPE_HASH);
     if (!key || key->value->type != OBJ_HASH || !field || !buf) return VALKEYMODULE_ERR;
-    return hashTypeSetValueView(key->value, field->ptr, buf, len);
+    return hashTypeSetViewValue(key->value, field->ptr, buf, len);
 }
 /* Set the field of the specified hash field to the specified value.
  * If the key is an empty key open for writing, it is created with an empty
diff --git a/src/server.h b/src/server.h
index 59f47809bf..29753ab6e5 100644
--- a/src/server.h
+++ b/src/server.h
@@ -3419,7 +3419,7 @@ robj *hashTypeGetValueObject(robj *o, sds field);
 int hashTypeSet(robj *o, sds field, sds value, long long expiry, int flags);
 robj *hashTypeDup(robj *o);
 bool hashTypeHasVolatileFields(robj *o);
-int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len);
+int hashTypeSetViewValue(robj *o, sds field, const char *buf, size_t len);
 
 /* Pub / Sub */
 int pubsubUnsubscribeAllChannels(client *c, int notify);
diff --git a/src/t_hash.c b/src/t_hash.c
index 9c11f165f6..595d2e1c62 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -311,14 +311,14 @@ int hashTypeExists(robj *o, sds field) {
     return hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) == C_OK;
 }
 
-/* Set an externalized string field in a hash.
+/* Set a view value field in a hash.
  * Returns 0 on insert, 1 on update.
  * Assumes the key 'o' is already a hash or a new key.
  * The 'field' sds is consumed by this function.
  */
-int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len) {
+int hashTypeSetViewValue(robj *o, sds field, const char *buf, size_t len) {
     if (o->encoding == OBJ_ENCODING_LISTPACK) {
-        // StringView require HASHTABLE encoding due to aux bits and pointer storage.
+        // require HASHTABLE encoding due to aux bits and pointer storage.
         hashTypeConvert(o, OBJ_ENCODING_HASHTABLE);
     }
 
@@ -327,17 +327,18 @@ int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len) {
     void *existing;
     if (hashtableFindPositionForInsert(ht, field, &position, &existing)) {
         /* does not exist yet */
-        entry *e = createStringViewEntry(field, buf, len);
+        entry *e = createViewValueEntry(field, buf, len, EXPIRY_NONE);
         hashtableInsertAtPosition(ht, e, &position);
         return 0;
     }
     if (entryHasViewValue(existing)) {
-        StringViewValue *ext_value = entryGetViewValueRef(existing);
+        ViewValue *ext_value = entryGetViewValueRef(existing);
         ext_value->buf = (char *)buf;
         ext_value->len = len;
         return 1;
     }
-    entry *new_entry = createStringViewEntry(field, buf, len);
+    long long entry_expiry = entryGetExpiry(existing);
+    entry *new_entry = createViewValueEntry(field, buf, len, entry_expiry);
     int replaced = hashtableReplaceReallocatedEntry(ht, existing, new_entry);
     serverAssert(replaced);
     entryFree(existing);
diff --git a/src/unit/test_entry.c b/src/unit/test_entry.c
index 39afd790f6..714d4015b3 100644
--- a/src/unit/test_entry.c
+++ b/src/unit/test_entry.c
@@ -17,7 +17,9 @@
 /* Verify entry properties */
 static int verify_entry_properties(entry *e, sds field, sds value_copy, long long expiry, bool has_expiry, bool has_valueptr) {
     TEST_ASSERT(sdscmp(entryGetField(e), field) == 0);
-    TEST_ASSERT(sdscmp(entryGetValue(e), value_copy) == 0);
+    size_t len;
+    TEST_ASSERT(sdscmp(entryGetValue(e, &len), value_copy) == 0);
+    TEST_ASSERT(len == sdslen(value_copy));
     TEST_ASSERT(entryGetExpiry(e) == expiry);
     TEST_ASSERT(entryHasExpiry(e) == has_expiry);
     TEST_ASSERT(entryHasEmbeddedValue(e) != has_valueptr);
@@ -41,7 +43,7 @@ int test_entryCreate(int argc, char **argv, int flags) {
     sds value1 = sdsnew(SHORT_VALUE);
     sds value_copy1 = sdsdup(value1); // Keep a copy since entryCreate takes ownership of value
     long long expiry1 = 100;
-    entry *e1 = entryCreate(field1, value1, expiry1);
+    entry *e1 = entryCreate(field1, sdslen(field1), value1, expiry1);
     verify_entry_properties(e1, field1, value_copy1, expiry1, true, false);
 
     // Test with embedded value with no expiry
@@ -49,7 +51,7 @@ int test_entryCreate(int argc, char **argv, int flags) {
     sds value2 = sdsnew(SHORT_VALUE);
     sds value_copy2 = sdsdup(value2);
     long long expiry2 = EXPIRY_NONE;
-    entry *e2 = entryCreate(field2, value2, expiry2);
+    entry *e2 = entryCreate(field2, sdslen(field2), value2, expiry2);
     verify_entry_properties(e2, field2, value_copy2, expiry2, false, false);
 
     // Test with non-embedded field and value with expiry
@@ -57,7 +59,7 @@ int test_entryCreate(int argc, char **argv, int flags) {
     sds value3 = sdsnew(LONG_VALUE);
     sds value_copy3 = sdsdup(value3);
     long long expiry3 = 100;
-    entry *e3 = entryCreate(field3, value3, expiry3);
+    entry *e3 = entryCreate(field3, sdslen(field3), value3, expiry3);
     verify_entry_properties(e3, field3, value_copy3, expiry3, true, true);
 
     // Test with non-embedded field and value with no expiry
@@ -65,7 +67,7 @@ int test_entryCreate(int argc, char **argv, int flags) {
     sds value4 = sdsnew(LONG_VALUE);
     sds value_copy4 = sdsdup(value4);
     long long expiry4 = EXPIRY_NONE;
-    entry *e4 = entryCreate(field4, value4, expiry4);
+    entry *e4 = entryCreate(field4, sdslen(field4), value4, expiry4);
     verify_entry_properties(e4, field4, value_copy4, expiry4, false, true);
 
     entryFree(e1);
@@ -110,7 +112,7 @@ int test_entryUpdate(int argc, char **argv, int flags) {
     sds field = sdsnew(SHORT_FIELD);
     sds value_copy1 = sdsdup(value1);
     long long expiry1 = 100;
-    entry *e1 = entryCreate(field, value1, expiry1);
+    entry *e1 = entryCreate(field, sdslen(field), value1, expiry1);
     verify_entry_properties(e1, field, value_copy1, expiry1, true, false);
 
     // Update only value (keeping embedded)
@@ -231,7 +233,7 @@ int test_entryHasexpiry_entrySetExpiry(int argc, char **argv, int flags) {
     // No expiry
     sds field1 = sdsnew(SHORT_FIELD);
     sds value1 = sdsnew(SHORT_VALUE);
-    entry *e1 = entryCreate(field1, value1, EXPIRY_NONE);
+    entry *e1 = entryCreate(field1, sdslen(field1), value1, EXPIRY_NONE);
     TEST_ASSERT(entryHasExpiry(e1) == false);
     TEST_ASSERT(entryGetExpiry(e1) == EXPIRY_NONE);
 
@@ -251,7 +253,7 @@ int test_entryHasexpiry_entrySetExpiry(int argc, char **argv, int flags) {
     // Test with non-embedded entry
     sds field4 = sdsnew(LONG_FIELD);
     sds value4 = sdsnew(LONG_VALUE);
-    entry *e4 = entryCreate(field4, value4, EXPIRY_NONE);
+    entry *e4 = entryCreate(field4, sdslen(field4), value4, EXPIRY_NONE);
     TEST_ASSERT(entryHasExpiry(e4) == false);
     TEST_ASSERT(entryHasEmbeddedValue(e4) == false);
 
@@ -299,7 +301,7 @@ int test_entryIsExpired(int argc, char **argv, int flags) {
     // No expiry
     sds field1 = sdsnew(SHORT_FIELD);
     sds value1 = sdsnew(SHORT_VALUE);
-    entry *e1 = entryCreate(field1, value1, EXPIRY_NONE);
+    entry *e1 = entryCreate(field1, sdslen(field1), value1, EXPIRY_NONE);
     TEST_ASSERT(entryGetExpiry(e1) == EXPIRY_NONE);
     TEST_ASSERT(entryIsExpired(e1) == false);
 
@@ -307,14 +309,14 @@ int test_entryIsExpired(int argc, char **argv, int flags) {
     sds field2 = sdsnew(SHORT_FIELD);
     sds value2 = sdsnew(SHORT_VALUE);
     long long future_time = current_time + 10000; // 10 seconds in future
-    entry *e2 = entryCreate(field2, value2, future_time);
+    entry *e2 = entryCreate(field2, sdslen(field2), value2, future_time);
     TEST_ASSERT(entryGetExpiry(e2) == future_time);
     TEST_ASSERT(entryIsExpired(e2) == false);
 
     // Current time expiry
     sds field3 = sdsnew(SHORT_FIELD);
     sds value3 = sdsnew(SHORT_VALUE);
-    entry *e3 = entryCreate(field3, value3, current_time);
+    entry *e3 = entryCreate(field3, sdslen(field3), value3, current_time);
     TEST_ASSERT(entryGetExpiry(e3) == current_time);
     TEST_ASSERT(entryIsExpired(e3) == false);
 
@@ -322,7 +324,7 @@ int test_entryIsExpired(int argc, char **argv, int flags) {
     sds field4 = sdsnew(SHORT_FIELD);
     sds value4 = sdsnew(SHORT_VALUE);
     long long past_time = current_time - 10000; // 10 seconds ago
-    entry *e4 = entryCreate(field4, value4, past_time);
+    entry *e4 = entryCreate(field4, sdslen(field4), value4, past_time);
     TEST_ASSERT(entryGetExpiry(e4) == past_time);
     TEST_ASSERT(entryIsExpired(e4) == true);
 
@@ -367,7 +369,7 @@ int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int f
     sds value1 = sdsnew(SHORT_VALUE);
     sds value_copy1 = sdsdup(value1);
     long long expiry1 = EXPIRY_NONE;
-    entry *e1 = entryCreate(field1, value1, expiry1);
+    entry *e1 = entryCreate(field1, sdslen(field1), value1, expiry1);
     size_t e1_entryMemUsage = entryMemUsage(e1);
     verify_entry_properties(e1, field1, value_copy1, expiry1, false, false);
     TEST_ASSERT(e1_entryMemUsage > 0);
@@ -414,7 +416,7 @@ int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int f
     sds value6 = sdsnew(LONG_VALUE);
     sds value_copy6 = sdsdup(value6);
     long long expiry6 = EXPIRY_NONE;
-    entry *e6 = entryCreate(field6, value6, EXPIRY_NONE);
+    entry *e6 = entryCreate(field6, sdslen(field6), value6, EXPIRY_NONE);
     size_t e6_entryMemUsage = entryMemUsage(e6);
     verify_entry_properties(e6, field6, value_copy6, expiry6, false, true);
     TEST_ASSERT(e6_entryMemUsage > 0);
diff --git a/src/unit/test_vset.c b/src/unit/test_vset.c
index 5a0cf06dff..1ee92bf48e 100644
--- a/src/unit/test_vset.c
+++ b/src/unit/test_vset.c
@@ -16,7 +16,7 @@ typedef entry mock_entry;
 
 static mock_entry *mockCreateEntry(const char *keystr, long long expiry) {
     sds field = sdsnew(keystr);
-    mock_entry *e = entryCreate(field, sdsnew("value"), expiry);
+    mock_entry *e = entryCreate(field, sdslen(field), sdsnew("value"), expiry);
     sdsfree(field);
     return e;
 }
@@ -27,7 +27,9 @@ static void mockFreeEntry(void *entry) {
 }
 
 static mock_entry *mockEntryUpdate(mock_entry *entry, long long expiry) {
-    mock_entry *new_entry = entryCreate(entryGetField(entry), sdsdup(entryGetValue(entry)), expiry);
+    sds field = entryGetField(entry);
+    size_t len;
+    mock_entry *new_entry = entryCreate(field, sdslen(field), sdsdup(entryGetValue(entry, &len)), expiry);
     entryFree(entry);
     return new_entry;
 }

From 5d4c16a014148bded4adffb5de61a4d06de07f0b Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Tue, 12 Aug 2025 15:14:43 +0000
Subject: [PATCH 04/30] addressing unittest issues

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/db.c     | 11 +++--------
 src/entry.c  | 17 +++++++----------
 src/entry.h  |  6 +++---
 src/t_hash.c |  2 +-
 4 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/db.c b/src/db.c
index 9efdeba824..7df2d8465f 100644
--- a/src/db.c
+++ b/src/db.c
@@ -1004,13 +1004,8 @@ int objectTypeCompare(robj *o, long long target) {
         return 1;
 }
 
-typedef struct {
-    const char *buf;
-    size_t len;
-} scanDataItem;
-
 static void addScanDataItem(vector *result, const char *buf, size_t len) {
-    scanDataItem *item = vectorPush(result);
+    viewValue *item = vectorPush(result);
     item->buf = buf;
     item->len = len;
 }
@@ -1256,7 +1251,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
         /* scanning ZSET allocates temporary strings even though it's a dict */
         free_callback = sdsfree;
     }
-    vectorInit(&result, SCAN_VECTOR_INITIAL_ALLOC, sizeof(scanDataItem));
+    vectorInit(&result, SCAN_VECTOR_INITIAL_ALLOC, sizeof(viewValue));
 
     /* For main hash table scan or scannable data structure. */
     if (!o || ht) {
@@ -1359,7 +1354,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
 
     addReplyArrayLen(c, vectorLen(&result));
     for (uint32_t i = 0; i < vectorLen(&result); i++) {
-        scanDataItem *key = vectorGet(&result, i);
+        viewValue *key = vectorGet(&result, i);
         addReplyBulkCBuffer(c, key->buf, key->len);
         if (free_callback) {
             free_callback((sds)(key->buf));
diff --git a/src/entry.c b/src/entry.c
index 98828519f2..9ac6c8fd12 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -89,10 +89,10 @@ static sds *entryGetValueRef(const entry *entry) {
     return (sds *)field_data;
 }
 
-ViewValue *entryGetViewValueRef(const entry *entry) {
+viewValue *entryGetViewValueRef(const entry *entry) {
     serverAssert(entryHasViewValue(entry));
     size_t offset = sdslen(entry) + 1 + sdsHdrSize(SDS_TYPE_8);
-    return (ViewValue *)entry + offset;
+    return (viewValue *)entry + offset;
 }
 
 /* Returns the entry's value. */
@@ -107,7 +107,7 @@ char *entryGetValue(const entry *entry, size_t *len) {
     /* Skip field content, field null terminator and value sds8 hdr. */
     size_t offset = sdslen(entry) + 1 + sdsHdrSize(SDS_TYPE_8);
     if (entryHasViewValue(entry)) {
-        ViewValue *value = (ViewValue *)entry + offset;
+        viewValue *value = (viewValue *)entry + offset;
         *len = value->len;
         return (char *)value->buf;
     }
@@ -133,9 +133,7 @@ entry *entrySetValue(entry *e, sds value) {
 /* Returns the address of the entry allocation. */
 void *entryGetAllocPtr(const entry *entry) {
     char *buf = sdsAllocPtr(entry);
-    if (entryHasValuePtr(entry)) {
-        buf -= sizeof(sds);
-    }
+    if (entryHasValuePtr(entry)) buf -= sizeof(sds);
     if (entryHasExpiry(entry)) buf -= sizeof(long long);
     return buf;
 }
@@ -178,8 +176,7 @@ bool entryIsExpired(entry *entry) {
 
 void entryFree(entry *entry) {
     if (entryHasValuePtr(entry)) {
-        size_t len;
-        sdsfree(entryGetValue(entry, &len));
+        sdsfree(*entryGetValueRef(entry));
     }
     zfree(entryGetAllocPtr(entry));
 }
@@ -301,8 +298,8 @@ entry *entryCreate(const char *field, size_t field_len, sds value, long long exp
 
 /* Create an entry with a view value. The view value structure is stored as an embedded field.  */
 entry *createViewValueEntry(sds field, const char *buf, size_t len, long long expiry) {
-    ViewValue view_value = {buf, len};
-    sds value = sdsnewlen(&view_value, sizeof (ViewValue));
+    viewValue view_value = {buf, len};
+    sds value = sdsnewlen(&view_value, sizeof (viewValue));
     entry *new_entry = entryCreate(field, sdslen(field), value, expiry);
     sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE, 1);          // Mark as view value
                                                                                 //
diff --git a/src/entry.h b/src/entry.h
index aba2f46f9b..029a422278 100644
--- a/src/entry.h
+++ b/src/entry.h
@@ -49,10 +49,10 @@ typedef void entry;
  * The module is responsible for the lifetime management of the memory pointed
  * to by 'buf'. Valkey core will not free 'buf'.
  */
-typedef struct ViewValue {
+typedef struct viewValue {
     const char *buf; /* Pointer to the externalized buffer */
     size_t len;      /* Length of the buffer */
-} ViewValue;
+} viewValue;
 
 /* The maximum allocation size we want to use for entries with embedded
  * values. */
@@ -88,7 +88,7 @@ void entryFree(entry *entry);
 /* Creates a new entry with the given field, value, and optional expiry. */
 entry *entryCreate(const char *field, size_t field_len, sds value, long long expiry);
 entry *createViewValueEntry(sds field, const char *buf, size_t len, long long expiry);
-ViewValue *entryGetViewValueRef(const entry *e);
+viewValue *entryGetViewValueRef(const entry *e);
 
 /* Updates the value and/or expiry of an existing entry.
  * In case value is NULL, will use the existing entry value.
diff --git a/src/t_hash.c b/src/t_hash.c
index 595d2e1c62..6c2515f178 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -332,7 +332,7 @@ int hashTypeSetViewValue(robj *o, sds field, const char *buf, size_t len) {
         return 0;
     }
     if (entryHasViewValue(existing)) {
-        ViewValue *ext_value = entryGetViewValueRef(existing);
+        viewValue *ext_value = entryGetViewValueRef(existing);
         ext_value->buf = (char *)buf;
         ext_value->len = len;
         return 1;

From f826960ce75abbf009d28c6b49a695a7d3420925 Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Tue, 12 Aug 2025 15:20:25 +0000
Subject: [PATCH 05/30] addressing lint

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/entry.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/entry.c b/src/entry.c
index 9ac6c8fd12..af53134164 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -299,10 +299,9 @@ entry *entryCreate(const char *field, size_t field_len, sds value, long long exp
 /* Create an entry with a view value. The view value structure is stored as an embedded field.  */
 entry *createViewValueEntry(sds field, const char *buf, size_t len, long long expiry) {
     viewValue view_value = {buf, len};
-    sds value = sdsnewlen(&view_value, sizeof (viewValue));
+    sds value = sdsnewlen(&view_value, sizeof(viewValue));
     entry *new_entry = entryCreate(field, sdslen(field), value, expiry);
-    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE, 1);          // Mark as view value
-                                                                                //
+    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE, 1); // Mark as view value
     debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR) == 0);
     debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE) == 1);
     return new_entry;

From 78b90bb18aba08430bc8cfd3e8b1af1af8f447ac Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Fri, 15 Aug 2025 17:15:03 +0000
Subject: [PATCH 06/30] addressing comments

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/db.c              |  21 +++--
 src/entry.c           | 214 ++++++++++++++++++++++++------------------
 src/entry.h           |  29 +++---
 src/module.c          |  11 ++-
 src/rdb.c             |   4 +-
 src/server.h          |   3 +-
 src/t_hash.c          |  69 ++++++--------
 src/unit/test_entry.c |  30 +++---
 src/unit/test_vset.c  |   4 +-
 src/valkeymodule.h    |   2 +
 10 files changed, 211 insertions(+), 176 deletions(-)

diff --git a/src/db.c b/src/db.c
index 7df2d8465f..bc612e6c8b 100644
--- a/src/db.c
+++ b/src/db.c
@@ -1005,7 +1005,7 @@ int objectTypeCompare(robj *o, long long target) {
 }
 
 static void addScanDataItem(vector *result, const char *buf, size_t len) {
-    viewValue *item = vectorPush(result);
+    bufferView *item = vectorPush(result);
     item->buf = buf;
     item->len = len;
 }
@@ -1047,8 +1047,7 @@ void keysScanCallback(void *privdata, void *entry, int didx) {
  * returned by the dictionary iterator into a list. */
 void hashtableScanCallback(void *privdata, void *entry) {
     scanData *data = (scanData *)privdata;
-    const char *val = NULL;
-    size_t val_len = 0;
+    bufferView val = {NULL, 0};
     sds key = NULL;
 
     robj *o = data->o;
@@ -1068,7 +1067,7 @@ void hashtableScanCallback(void *privdata, void *entry) {
     } else if (o->type == OBJ_HASH) {
         key = entryGetField(entry);
         if (!data->only_keys) {
-            val = entryGetValue(entry, &val_len);
+            val.buf = entryGetValue(entry, &val.len);
         }
     } else {
         serverPanic("Type not handled in hashtable SCAN callback.");
@@ -1089,14 +1088,16 @@ void hashtableScanCallback(void *privdata, void *entry) {
         key = sdsdup(node->ele);
         if (!data->only_keys) {
             char buf[MAX_LONG_DOUBLE_CHARS];
-            val_len = ld2string(buf, sizeof(buf), node->score, LD_STR_AUTO);
-            val = (const char *)sdsnewlen(buf, val_len);
+            int len = ld2string(buf, sizeof(buf), node->score, LD_STR_AUTO);
+            sds tmp = sdsnewlen(buf, len);
+            val.buf = (const char *)tmp;
+            val.len = sdslen(tmp);
         }
     }
 
     addScanDataItem(data->result, (const char *)key, sdslen(key));
-    if (val) {
-        addScanDataItem(data->result, val, sdslen(val));
+    if (val.buf) {
+        addScanDataItem(data->result, val.buf, val.len);
     }
 }
 
@@ -1251,7 +1252,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
         /* scanning ZSET allocates temporary strings even though it's a dict */
         free_callback = sdsfree;
     }
-    vectorInit(&result, SCAN_VECTOR_INITIAL_ALLOC, sizeof(viewValue));
+    vectorInit(&result, SCAN_VECTOR_INITIAL_ALLOC, sizeof(bufferView));
 
     /* For main hash table scan or scannable data structure. */
     if (!o || ht) {
@@ -1354,7 +1355,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
 
     addReplyArrayLen(c, vectorLen(&result));
     for (uint32_t i = 0; i < vectorLen(&result); i++) {
-        viewValue *key = vectorGet(&result, i);
+        bufferView *key = vectorGet(&result, i);
         addReplyBulkCBuffer(c, key->buf, key->len);
         if (free_callback) {
             free_callback((sds)(key->buf));
diff --git a/src/entry.c b/src/entry.c
index af53134164..b97779bbcd 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -47,7 +47,7 @@ enum {
      * The hash entry does not own the view buffer and will not free it upon
      * entry destruction. This is useful for avoiding memory duplication
      * between the core and a module. */
-    FIELD_SDS_AUX_BIT_VIEW_VALUE = 2,
+    FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW = 2,
     FIELD_SDS_AUX_BIT_MAX
 };
 static_assert(FIELD_SDS_AUX_BIT_MAX < sizeof(char) - SDS_TYPE_BITS, "too many sds bits are used for entry metadata");
@@ -60,14 +60,14 @@ static inline bool entryHasValuePtr(const entry *entry) {
 
 /* Returns true in case the entry's value is embedded in the entry.
  * Returns false otherwise. */
-bool entryHasEmbeddedValue(entry *entry) {
+bool entryHasEmbeddedValue(const entry *entry) {
     return (!entryHasValuePtr(entry));
 }
 
 /* Returns true in case the entry holds a view of the value.
  * Returns false otherwise. */
-bool entryHasViewValue(const entry *e) {
-    return sdsGetAuxBit(e, FIELD_SDS_AUX_BIT_VIEW_VALUE);
+bool entryHasValueView(const entry *entry) {
+    return entryHasValuePtr(entry) && sdsGetAuxBit(entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW);
 }
 /* Returns true in case the entry has expiration timestamp.
  * Returns false otherwise. */
@@ -82,58 +82,57 @@ sds entryGetField(const entry *entry) {
 
 /* Returns the location of a pointer to a separately allocated value. Only for
  * an entry without an embedded value. */
-static sds *entryGetValueRef(const entry *entry) {
+static inline void **entryGetValueRef(const entry *entry) {
     serverAssert(entryHasValuePtr(entry));
     char *field_data = sdsAllocPtr(entry);
-    field_data -= sizeof(sds);
-    return (sds *)field_data;
-}
-
-viewValue *entryGetViewValueRef(const entry *entry) {
-    serverAssert(entryHasViewValue(entry));
-    size_t offset = sdslen(entry) + 1 + sdsHdrSize(SDS_TYPE_8);
-    return (viewValue *)entry + offset;
+    field_data -= sizeof(void *);
+    return (void **)field_data;
 }
 
 /* Returns the entry's value. */
 char *entryGetValue(const entry *entry, size_t *len) {
-    if (entryHasValuePtr(entry)) {
-        sds *value = entryGetValueRef(entry);
-        if (*value) {
-            *len = sdslen(*value);
-        }
-        return *value;
+    if (entryHasEmbeddedValue(entry)) {
+        /* Skip field content, field null terminator and value sds8 hdr. */
+        size_t offset = sdslen(entry) + 1 + sdsHdrSize(SDS_TYPE_8);
+        sds value = (char *)entry + offset;
+        if (len) *len = sdslen(value);
+        return value;
     }
-    /* Skip field content, field null terminator and value sds8 hdr. */
-    size_t offset = sdslen(entry) + 1 + sdsHdrSize(SDS_TYPE_8);
-    if (entryHasViewValue(entry)) {
-        viewValue *value = (viewValue *)entry + offset;
-        *len = value->len;
-        return (char *)value->buf;
+    void **value_ref = entryGetValueRef(entry);
+    if (!value_ref) return NULL;
+    if (entryHasValueView(entry)) {
+        bufferView *buffer_view = (bufferView *)*value_ref;
+        if (len) *len = buffer_view->len;
+        return (char *)buffer_view->buf;
     }
-    sds value = (char *)entry + offset;
-    *len = sdslen(value);
-    return value;
+    if (len) *len = sdslen(*value_ref);
+    return *value_ref;
 }
 
+static void entrySetValuePtr(entry *e, sds value) {
+    serverAssert(entryHasValuePtr(e));
+    void **value_ref = entryGetValueRef(e);
+    if (entryHasValueView(e)) {
+        zfree(*value_ref);
+        *value_ref = (bufferView *)value;
+        sdsSetAuxBit(e, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW, 0);
+    } else {
+        sdsfree(*value_ref);
+    }
+    *value_ref = value;
+}
 /* Modify the value of this entry and return a pointer to the (potentially new) entry.
  * The value is taken by the function and cannot be reused after this function returns. */
 entry *entrySetValue(entry *e, sds value) {
-    if (entryHasValuePtr(e)) {
-        sds *value_ref = entryGetValueRef(e);
-        sdsfree(*value_ref);
-        *value_ref = value;
-        return e;
-    } else {
-        entry *new_entry = entryUpdate(e, value, entryGetExpiry(e));
-        return new_entry;
-    }
+    if (entryHasEmbeddedValue(e)) return entryUpdate(e, value, entryGetExpiry(e));
+    entrySetValuePtr(e, value);
+    return e;
 }
 
 /* Returns the address of the entry allocation. */
-void *entryGetAllocPtr(const entry *entry) {
+static void *entryGetAllocPtr(const entry *entry) {
     char *buf = sdsAllocPtr(entry);
-    if (entryHasValuePtr(entry)) buf -= sizeof(sds);
+    if (entryHasValuePtr(entry)) buf -= sizeof(void *);
     if (entryHasExpiry(entry)) buf -= sizeof(long long);
     return buf;
 }
@@ -164,8 +163,7 @@ entry *entrySetExpiry(entry *e, long long expiry) {
         *(long long *)buf = expiry;
         return e;
     }
-    entry *new_entry = entryUpdate(e, NULL, expiry);
-    return new_entry;
+    return entryUpdate(e, NULL, expiry);
 }
 
 /* Return true in case the entry has assigned expiration or false otherwise. */
@@ -174,10 +172,18 @@ bool entryIsExpired(entry *entry) {
 }
 /**************************************** Entry Expiry API - End *****************************************/
 
-void entryFree(entry *entry) {
-    if (entryHasValuePtr(entry)) {
-        sdsfree(*entryGetValueRef(entry));
+static inline void entryFreeValuePtr(entry *entry) {
+    serverAssert(entryHasValuePtr(entry));
+    void **value_ref = entryGetValueRef(entry);
+    if (entryHasValueView(entry)) {
+        zfree(*value_ref);
+        return;
     }
+    sdsfree(*value_ref);
+}
+
+void entryFree(entry *entry) {
+    if (entryHasValuePtr(entry)) entryFreeValuePtr(entry);
     zfree(entryGetAllocPtr(entry));
 }
 
@@ -243,7 +249,6 @@ static inline size_t entryReqSize(size_t field_len,
 static entry *entryWrite(char *buf,
                          size_t buf_size,
                          const_sds field,
-                         size_t field_len,
                          sds value,
                          long long expiry,
                          bool embed_value,
@@ -269,7 +274,7 @@ static entry *entryWrite(char *buf,
         }
     }
     /* Set the field data */
-    entry *new_entry = sdswrite(buf, embedded_field_sds_size, embedded_field_sds_type, field, field_len);
+    entry *new_entry = sdswrite(buf, embedded_field_sds_size, embedded_field_sds_type, field, sdslen(field));
 
     /* Field sds aux bits are zero, which we use for this entry encoding. */
     sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR, embed_value ? 0 : 1);
@@ -282,28 +287,63 @@ static entry *entryWrite(char *buf,
 }
 
 /* Takes ownership of value. does not take ownership of field */
-entry *entryCreate(const char *field, size_t field_len, sds value, long long expiry) {
+entry *entryCreate(const_sds field, sds value, long long expiry) {
     bool embed_value = false;
     int embedded_field_sds_type;
     size_t expiry_size, embedded_value_sds_size, embedded_field_sds_size;
     size_t value_len = value ? sdslen(value) : SIZE_MAX;
-    size_t alloc_size = entryReqSize(field_len, value_len, expiry, &embed_value, &embedded_field_sds_type, &embedded_field_sds_size, &expiry_size, &embedded_value_sds_size);
+    size_t alloc_size = entryReqSize(sdslen(field), value_len, expiry, &embed_value, &embedded_field_sds_type, &embedded_field_sds_size, &expiry_size, &embedded_value_sds_size);
     size_t buf_size;
 
     /* allocate the buffer */
     char *buf = zmalloc_usable(alloc_size, &buf_size);
 
-    return entryWrite(buf, buf_size, field, field_len, value, expiry, embed_value, embedded_field_sds_type, embedded_field_sds_size, embedded_value_sds_size, expiry_size);
+    return entryWrite(buf, buf_size, field, value, expiry, embed_value, embedded_field_sds_type, embedded_field_sds_size, embedded_value_sds_size, expiry_size);
 }
 
-/* Create an entry with a view value. The view value structure is stored as an embedded field.  */
-entry *createViewValueEntry(sds field, const char *buf, size_t len, long long expiry) {
-    viewValue view_value = {buf, len};
-    sds value = sdsnewlen(&view_value, sizeof(viewValue));
-    entry *new_entry = entryCreate(field, sdslen(field), value, expiry);
-    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE, 1); // Mark as view value
-    debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR) == 0);
-    debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE) == 1);
+entry *entrySetValueView(entry *entry, const char *buf, size_t len, long long expiry) {
+    long long entry_expiry = entryGetExpiry(entry);
+    // Check for toggling expiration
+    bool expiry_add_remove = (expiry != entry_expiry) && (entry_expiry == EXPIRY_NONE || expiry == EXPIRY_NONE);
+    if (entryHasValueView(entry) && !expiry_add_remove) {
+        bufferView *value = *entryGetValueRef(entry);
+        value->buf = buf;
+        value->len = len;
+        return entry;
+    }
+    bufferView *value = zmalloc(sizeof(bufferView));
+    value->buf = buf;
+    value->len = len;
+    if (entryHasValuePtr(entry) && !expiry_add_remove) {
+        sds *value_ref = (sds *)entryGetValueRef(entry);
+        sdsfree(*value_ref);
+        *value_ref = (sds)value;
+        sdsSetAuxBit(entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW, 1);
+        return entry;
+    }
+    sds field = entryGetField(entry);
+    size_t field_size = sdsReqSize(sdslen(field), SDS_TYPE_8);
+    size_t alloc_size = field_size + sizeof(void *);
+    alloc_size += (expiry == EXPIRY_NONE) ? 0 : sizeof(expiry);
+
+    size_t buf_size;
+    char *buf_sds = zmalloc_usable(alloc_size, &buf_size);
+
+    /* Set The expiry if exists */
+    if (expiry != EXPIRY_NONE) {
+        *(long long *)buf_sds = expiry;
+        buf_sds += sizeof(expiry);
+        buf_size -= sizeof(expiry);
+    }
+    *(bufferView **)buf_sds = value;
+    buf_sds += sizeof(value);
+    buf_size -= sizeof(value);
+    /* Set the field data */
+    sds new_entry = sdswrite(buf_sds, field_size, SDS_TYPE_8, field, sdslen(field));
+    entryFree(entry);
+
+    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW, 1);
+    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR, 1);
     return new_entry;
 }
 /* Modify the entry's value and/or expiration time.
@@ -314,12 +354,16 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
     sds field = (sds)e;
     entry *new_entry = NULL;
 
+    if (entryHasValueView(e)) {
+        serverAssert(!value);
+        bufferView *value = (bufferView *)*entryGetValueRef(e);
+        return entrySetValueView(e, value->buf, value->len, expiry);
+    }
     bool update_value = value ? true : false;
     long long curr_expiration_time = entryGetExpiry(e);
     bool update_expiry = (expiry != curr_expiration_time) ? true : false;
     /* Just a sanity check. If nothing changes, lets just return */
-    if (!update_value && !update_expiry)
-        return e;
+    if (!update_value && !update_expiry) return e;
     size_t value_len = SIZE_MAX;
     if (value) {
         value_len = sdslen(value);
@@ -330,7 +374,7 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
     int embedded_field_sds_type;
     size_t expiry_size, embedded_value_size, embedded_field_size;
     size_t required_entry_size = entryReqSize(sdslen(field), value_len, expiry, &embed_value, &embedded_field_sds_type, &embedded_field_size, &expiry_size, &embedded_value_size);
-    size_t current_embedded_allocation_size = entryHasValuePtr(e) ? 0 : entryMemUsage(e);
+    size_t current_embedded_allocation_size = entryHasEmbeddedValue(e) ? entryMemUsage(e) : 0;
 
     bool expiry_add_remove = update_expiry && (curr_expiration_time == EXPIRY_NONE || expiry == EXPIRY_NONE); // In case we are toggling expiration
     bool value_change_encoding = update_value && (embed_value != entryHasEmbeddedValue(e));                   // In case we change the way value is embedded or not
@@ -355,14 +399,9 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
         }
         /* In this case we are sure we do not have to allocate new entry, so value must already be set or we have enough room to embed it. */
         if (update_value) {
-            if (entryHasValuePtr(e)) {
-                sds *value_ref = entryGetValueRef(e);
-                sdsfree(*value_ref);
-                *value_ref = value;
-            } else {
+            if (entryHasEmbeddedValue(e)) {
                 /* Skip field content, field null terminator and value sds8 hdr. */
-                size_t len;
-                char *old_value = entryGetValue(e, &len);
+                char *old_value = entryGetValue(e, NULL);
                 /* We are using the same entry memory in order to store a potentially new value.
                  * In such cases the old value alloc was adjusted to the real buffer size part it was embedded to.
                  * Since we can potentially write here a smaller value, which requires less allocation space, we would like to
@@ -370,36 +409,35 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
                 size_t value_size = sdsHdrSize(SDS_TYPE_8) + sdsalloc(old_value) + 1;
                 sdswrite(sdsAllocPtr(old_value), value_size, SDS_TYPE_8, value, sdslen(value));
                 sdsfree(value);
+            } else {
+                entrySetValuePtr(e, value);
             }
         }
         new_entry = e;
 
     } else {
         if (!update_value) {
-            /* Check if the value can be reused. */
-            int value_was_embedded = !entryHasValuePtr(e);
-            /* In case the original entry value is embedded WE WILL HAVE TO DUPLICATE IT
-             * if not we have to duplicate it, remove it from the original entry since we are going to delete it.*/
-            if (value_was_embedded) {
+            /* Check if the value can be reused. In case the original entry value is
+             * embedded WE WILL HAVE TO DUPLICATE IT if not we have to duplicate it,
+             * remove it from the original entry since we are going to delete it.*/
+            if (entryHasEmbeddedValue(e)) {
                 value = sdsdup(value);
             } else {
-                sds *value_ref = entryGetValueRef(e);
+                void **value_ref = entryGetValueRef(e);
                 *value_ref = NULL;
             }
         }
         /* allocate the buffer for a new entry */
         size_t buf_size;
         char *buf = zmalloc_usable(required_entry_size, &buf_size);
-        new_entry = entryWrite(buf, buf_size, field, sdslen(field), value, expiry, embed_value, embedded_field_sds_type, embedded_field_size, embedded_value_size, expiry_size);
-        debugServerAssert(new_entry != e);
+        new_entry = entryWrite(buf, buf_size, field, value, expiry, embed_value, embedded_field_sds_type, embedded_field_size, embedded_value_size, expiry_size);
+        sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW, sdsGetAuxBit(e, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW));
         entryFree(e);
     }
-    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE, 0);
     /* Check that the new entry was built correctly */
+    serverAssert(new_entry);
     debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR) == (embed_value ? 0 : 1));
     debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_EXPIRY) == (expiry_size > 0 ? 1 : 0));
-    debugServerAssert(sdsGetAuxBit(new_entry, FIELD_SDS_AUX_BIT_VIEW_VALUE) == 0);
-    serverAssert(new_entry);
     return new_entry;
 }
 
@@ -407,16 +445,16 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
  * the entry. */
 size_t entryMemUsage(entry *entry) {
     size_t mem = 0;
-    if (entryHasValuePtr(entry)) {
+
+    if (entryHasEmbeddedValue(entry)) {
+        mem += sdsReqSize(sdslen(entry), sdsType(entry));
+        if (entryHasExpiry(entry)) mem += sizeof(long long);
+    } else {
         /* In case the value is not embedded we might not be able to sum all the allocation sizes since the field
          * header could be too small for holding the real allocation size. */
         mem += zmalloc_usable_size(entryGetAllocPtr(entry));
-    } else {
-        mem += sdsReqSize(sdslen(entry), sdsType(entry));
-        if (entryHasExpiry(entry)) mem += sizeof(long long);
     }
-    size_t len;
-    mem += sdsAllocSize((sds)entryGetValue(entry, &len));
+    mem += sdsAllocSize((sds)entryGetValue(entry, NULL));
     return mem;
 }
 
@@ -428,9 +466,9 @@ size_t entryMemUsage(entry *entry) {
  * If the location of the entry changed we return the new location,
  * otherwise we return NULL. */
 entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) {
-    if (entryHasViewValue(entry)) return NULL;
+    if (entryHasValueView(entry)) return NULL;
     if (entryHasValuePtr(entry)) {
-        sds *value_ref = entryGetValueRef(entry);
+        sds *value_ref = (sds *)entryGetValueRef(entry);
         sds new_value = sdsdefragfn(*value_ref);
         if (new_value) *value_ref = new_value;
     }
@@ -448,7 +486,5 @@ entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(s
  * forked and memory won't be used again. See zmadvise_dontneed() */
 void entryDismissMemory(entry *entry) {
     /* Only dismiss values memory since the field size usually is small. */
-    if (entryHasValuePtr(entry)) {
-        dismissSds(*entryGetValueRef(entry));
-    }
+    if (entryHasValuePtr(entry)) entryFreeValuePtr(entry);
 }
diff --git a/src/entry.h b/src/entry.h
index 029a422278..7c666f5877 100644
--- a/src/entry.h
+++ b/src/entry.h
@@ -4,6 +4,15 @@
 #include "sds.h"
 #include <stdbool.h>
 
+/* Structure representing a non-owning view of a buffer.
+ * The view does not manage the underlying memory, so its destruction
+ * will not free the buffer.
+ */
+typedef struct bufferView {
+    const char *buf; /* Pointer to the externalized buffer */
+    size_t len;      /* Length of the buffer */
+} bufferView;
+
 /*-----------------------------------------------------------------------------
  * Entry
  *----------------------------------------------------------------------------*/
@@ -43,17 +52,6 @@
  * - Used for large value sizes. */
 typedef void entry;
 
-/* Structure representing a view value.
- * This allows modules to store a char* and length directly in a hash field,
- * bypassing normal SDS string allocation for the value.
- * The module is responsible for the lifetime management of the memory pointed
- * to by 'buf'. Valkey core will not free 'buf'.
- */
-typedef struct viewValue {
-    const char *buf; /* Pointer to the externalized buffer */
-    size_t len;      /* Length of the buffer */
-} viewValue;
-
 /* The maximum allocation size we want to use for entries with embedded
  * values. */
 #define EMBED_VALUE_MAX_ALLOC_SIZE 128
@@ -74,7 +72,7 @@ long long entryGetExpiry(const entry *entry);
 bool entryHasExpiry(const entry *entry);
 
 /* Returns true if the entry value is externalized. */
-bool entryHasViewValue(const entry *e);
+bool entryHasValueView(const entry *entry);
 
 /* Sets the expiration timestamp. */
 entry *entrySetExpiry(entry *entry, long long expiry);
@@ -86,9 +84,8 @@ bool entryIsExpired(entry *entry);
 void entryFree(entry *entry);
 
 /* Creates a new entry with the given field, value, and optional expiry. */
-entry *entryCreate(const char *field, size_t field_len, sds value, long long expiry);
-entry *createViewValueEntry(sds field, const char *buf, size_t len, long long expiry);
-viewValue *entryGetViewValueRef(const entry *e);
+entry *entryCreate(const_sds field, sds value, long long expiry);
+entry *entrySetValueView(entry *entry, const char *buf, size_t len, long long expiry);
 
 /* Updates the value and/or expiry of an existing entry.
  * In case value is NULL, will use the existing entry value.
@@ -105,6 +102,6 @@ entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(s
 void entryDismissMemory(entry *entry);
 
 /* Internal used for debug. No need to use this function except in tests */
-bool entryHasEmbeddedValue(entry *entry);
+bool entryHasEmbeddedValue(const entry *entry);
 
 #endif
diff --git a/src/module.c b/src/module.c
index 7af9988132..95cb8a5f69 100644
--- a/src/module.c
+++ b/src/module.c
@@ -5254,9 +5254,13 @@ int VM_ZsetRangePrev(ValkeyModuleKey *key) {
 /* Sets a view of a buffer to a hash field.
  * The function takes the hash key, hash field, and a buffer along with its length. */
 int VM_HashSetValueView(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) {
-    if (key->value == NULL) moduleCreateEmptyKey(key, VALKEYMODULE_KEYTYPE_HASH);
-    if (!key || key->value->type != OBJ_HASH || !field || !buf) return VALKEYMODULE_ERR;
-    return hashTypeSetViewValue(key->value, field->ptr, buf, len);
+    if (!key || !key->value || key->value->type != OBJ_HASH || !field || !buf) return VALKEYMODULE_ERR;
+    return hashTypeSetValueView(key->value, field->ptr, buf, len);
+}
+
+int VM_HashHasValueView(ValkeyModuleKey *key, ValkeyModuleString *field) {
+    if (!key || !key->value || key->value->type != OBJ_HASH) return VALKEYMODULE_ERR;
+    return hashTypeHasValueView(key->value, field->ptr);
 }
 /* Set the field of the specified hash field to the specified value.
  * If the key is an empty key open for writing, it is created with an empty
@@ -13998,6 +14002,7 @@ void moduleRegisterCoreAPI(void) {
     REGISTER_API(HashSet);
     REGISTER_API(HashGet);
     REGISTER_API(HashSetValueView);
+    REGISTER_API(HashHasValueView);
     REGISTER_API(StreamAdd);
     REGISTER_API(StreamDelete);
     REGISTER_API(StreamIteratorStart);
diff --git a/src/rdb.c b/src/rdb.c
index 4d1d25b409..90b31a191e 100644
--- a/src/rdb.c
+++ b/src/rdb.c
@@ -2145,7 +2145,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) {
                 (sdslen(field) > server.hash_max_listpack_value || sdslen(value) > server.hash_max_listpack_value ||
                  !lpSafeToAdd(o->ptr, sdslen(field) + sdslen(value)))) {
                 hashTypeConvert(o, OBJ_ENCODING_HASHTABLE);
-                entry *entry = entryCreate(field, sdslen(field), value, EXPIRY_NONE);
+                entry *entry = entryCreate(field, value, EXPIRY_NONE);
                 sdsfree(field);
                 if (!hashtableAdd((hashtable *)o->ptr, entry)) {
                     rdbReportCorruptRDB("Duplicate hash fields detected");
@@ -2203,7 +2203,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) {
             }
 
             /* Add pair to hash table */
-            entry *entry = entryCreate(field, sdslen(field), value, itemexpiry);
+            entry *entry = entryCreate(field, value, itemexpiry);
             sdsfree(field);
             if (!hashtableAdd((hashtable *)o->ptr, entry)) {
                 rdbReportCorruptRDB("Duplicate hash fields detected");
diff --git a/src/server.h b/src/server.h
index 29753ab6e5..f80a2d0b29 100644
--- a/src/server.h
+++ b/src/server.h
@@ -3419,7 +3419,8 @@ robj *hashTypeGetValueObject(robj *o, sds field);
 int hashTypeSet(robj *o, sds field, sds value, long long expiry, int flags);
 robj *hashTypeDup(robj *o);
 bool hashTypeHasVolatileFields(robj *o);
-int hashTypeSetViewValue(robj *o, sds field, const char *buf, size_t len);
+int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len);
+int hashTypeHasValueView(robj *o, sds field);
 
 /* Pub / Sub */
 int pubsubUnsubscribeAllChannels(client *c, int notify);
diff --git a/src/t_hash.c b/src/t_hash.c
index 6c2515f178..492cc26e02 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -311,38 +311,31 @@ int hashTypeExists(robj *o, sds field) {
     return hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) == C_OK;
 }
 
+int hashTypeHasValueView(robj *o, sds field) {
+    if (o->encoding == OBJ_ENCODING_LISTPACK) return 0;
+    hashtable *ht = o->ptr;
+    void **entry_ref = hashtableFindRef(ht, field);
+    return (entryHasValueView(*entry_ref));
+}
 /* Set a view value field in a hash.
- * Returns 0 on insert, 1 on update.
- * Assumes the key 'o' is already a hash or a new key.
- * The 'field' sds is consumed by this function.
+ * Returns C_ERR on error, C_OK on update.
  */
-int hashTypeSetViewValue(robj *o, sds field, const char *buf, size_t len) {
-    if (o->encoding == OBJ_ENCODING_LISTPACK) {
-        // require HASHTABLE encoding due to aux bits and pointer storage.
-        hashTypeConvert(o, OBJ_ENCODING_HASHTABLE);
-    }
+int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len) {
+    unsigned char *vstr = NULL;
+    unsigned int vlen = UINT_MAX;
+    long long vll = LLONG_MAX;
+
+    if (hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) != C_OK || !vstr) return C_ERR;
+    if (len != vlen || memcmp(buf, vstr, len) != 0) return C_ERR;
+    // require HASHTABLE encoding due to aux bits and pointer storage.
+    if (o->encoding == OBJ_ENCODING_LISTPACK) hashTypeConvert(o, OBJ_ENCODING_HASHTABLE);
 
     hashtable *ht = o->ptr;
-    hashtablePosition position;
-    void *existing;
-    if (hashtableFindPositionForInsert(ht, field, &position, &existing)) {
-        /* does not exist yet */
-        entry *e = createViewValueEntry(field, buf, len, EXPIRY_NONE);
-        hashtableInsertAtPosition(ht, e, &position);
-        return 0;
-    }
-    if (entryHasViewValue(existing)) {
-        viewValue *ext_value = entryGetViewValueRef(existing);
-        ext_value->buf = (char *)buf;
-        ext_value->len = len;
-        return 1;
-    }
-    long long entry_expiry = entryGetExpiry(existing);
-    entry *new_entry = createViewValueEntry(field, buf, len, entry_expiry);
-    int replaced = hashtableReplaceReallocatedEntry(ht, existing, new_entry);
-    serverAssert(replaced);
-    entryFree(existing);
-    return 1;
+    void **entry_ref = hashtableFindRef(ht, field);
+    entry *entry = *entry_ref;
+    if (entryHasValueView(entry)) return C_ERR;
+    entrySetValueView(entry, buf, len, entryGetExpiry(entry));
+    return C_OK;
 }
 
 /* Add a new field, overwrite the old with the new value if it already exists.
@@ -419,7 +412,7 @@ int hashTypeSet(robj *o, sds field, sds value, long long expiry, int flags) {
         void *existing;
         if (hashtableFindPositionForInsert(ht, field, &position, &existing)) {
             /* does not exist yet */
-            entry *entry = entryCreate(field, sdslen(field), v, expiry);
+            entry *entry = entryCreate(field, v, expiry);
             hashtableInsertAtPosition(ht, entry, &position);
             /* In case an expiry is set on the new entry, we need to track it */
             if (expiry != EXPIRY_NONE) {
@@ -715,7 +708,7 @@ char *hashTypeCurrentFromHashTable(hashTypeIterator *hi, int what, size_t *len)
 
     if (what & OBJ_HASH_FIELD) {
         sds key = entryGetField(hi->next);
-        *len = sdslen(key);
+        if (key) *len = sdslen(key);
         return key;
     }
     return entryGetValue(hi->next, len);
@@ -732,7 +725,7 @@ sds hashTypeCurrentObjectNewSds(hashTypeIterator *hi, int what) {
         if (vstr) return sdsnewlen(vstr, vlen);
         return sdsfromlonglong(vll);
     }
-    size_t vlen;
+    size_t vlen = 0;
     vstr = (unsigned char *)hashTypeCurrentFromHashTable(hi, what, &vlen);
     return sdsnewlen(vstr, vlen);
 }
@@ -767,7 +760,7 @@ void hashTypeConvertListpack(robj *o, int enc) {
         while (hashTypeNext(&hi) != C_ERR) {
             sds field = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_FIELD);
             sds value = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_VALUE);
-            entry *entry = entryCreate(field, sdslen(field), value, EXPIRY_NONE);
+            entry *entry = entryCreate(field, value, EXPIRY_NONE);
             sdsfree(field);
             if (!hashtableAdd(ht, entry)) {
                 entryFree(entry);
@@ -822,13 +815,13 @@ robj *hashTypeDup(robj *o) {
         hashTypeInitIterator(o, &hi);
         while (hashTypeNext(&hi) != C_ERR) {
             /* Extract a field-value pair from an original hash object.*/
-            size_t field_str_len, value_str_len;
-            char *field_str = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_FIELD, &field_str_len);
-            char *value_str = hashTypeCurrentFromHashTable(&hi, OBJ_HASH_VALUE, &value_str_len);
+            size_t len;
+            sds field = entryGetField(hi.next);
+            char *value_str = entryGetValue(hi.next, &len);
             long long expiry = entryGetExpiry(hi.next);
             /* Add a field-value pair to a new hash object. */
-            sds value = sdsnewlen(value_str, value_str_len);
-            entry *entry = entryCreate(field_str, field_str_len, value, expiry);
+            sds value = sdsnewlen(value_str, len);
+            entry *entry = entryCreate(field, value, expiry);
             hashtableAdd(ht, entry);
             if (expiry != EXPIRY_NONE)
                 hashTypeTrackEntry(hobj, entry);
@@ -1088,7 +1081,7 @@ static void addHashIteratorCursorToReply(writePreparedClient *wpc, hashTypeItera
         else
             addWritePreparedReplyBulkLongLong(wpc, vll);
     } else if (hi->encoding == OBJ_ENCODING_HASHTABLE) {
-        size_t len;
+        size_t len = 0;
         char *value = hashTypeCurrentFromHashTable(hi, what, &len);
         addWritePreparedReplyBulkCBuffer(wpc, value, len);
     } else {
diff --git a/src/unit/test_entry.c b/src/unit/test_entry.c
index 714d4015b3..bc20def9ea 100644
--- a/src/unit/test_entry.c
+++ b/src/unit/test_entry.c
@@ -42,16 +42,16 @@ int test_entryCreate(int argc, char **argv, int flags) {
     sds field1 = sdsnew(SHORT_FIELD);
     sds value1 = sdsnew(SHORT_VALUE);
     sds value_copy1 = sdsdup(value1); // Keep a copy since entryCreate takes ownership of value
-    long long expiry1 = 100;
-    entry *e1 = entryCreate(field1, sdslen(field1), value1, expiry1);
-    verify_entry_properties(e1, field1, value_copy1, expiry1, true, false);
+    long long expiry1 = EXPIRY_NONE;
+    entry *e1 = entryCreate(field1, value1, expiry1);
+    verify_entry_properties(e1, field1, value_copy1, expiry1, false, false);
 
     // Test with embedded value with no expiry
     sds field2 = sdsnew(SHORT_FIELD);
     sds value2 = sdsnew(SHORT_VALUE);
     sds value_copy2 = sdsdup(value2);
     long long expiry2 = EXPIRY_NONE;
-    entry *e2 = entryCreate(field2, sdslen(field2), value2, expiry2);
+    entry *e2 = entryCreate(field2, value2, expiry2);
     verify_entry_properties(e2, field2, value_copy2, expiry2, false, false);
 
     // Test with non-embedded field and value with expiry
@@ -59,7 +59,7 @@ int test_entryCreate(int argc, char **argv, int flags) {
     sds value3 = sdsnew(LONG_VALUE);
     sds value_copy3 = sdsdup(value3);
     long long expiry3 = 100;
-    entry *e3 = entryCreate(field3, sdslen(field3), value3, expiry3);
+    entry *e3 = entryCreate(field3, value3, expiry3);
     verify_entry_properties(e3, field3, value_copy3, expiry3, true, true);
 
     // Test with non-embedded field and value with no expiry
@@ -67,7 +67,7 @@ int test_entryCreate(int argc, char **argv, int flags) {
     sds value4 = sdsnew(LONG_VALUE);
     sds value_copy4 = sdsdup(value4);
     long long expiry4 = EXPIRY_NONE;
-    entry *e4 = entryCreate(field4, sdslen(field4), value4, expiry4);
+    entry *e4 = entryCreate(field4, value4, expiry4);
     verify_entry_properties(e4, field4, value_copy4, expiry4, false, true);
 
     entryFree(e1);
@@ -112,7 +112,7 @@ int test_entryUpdate(int argc, char **argv, int flags) {
     sds field = sdsnew(SHORT_FIELD);
     sds value_copy1 = sdsdup(value1);
     long long expiry1 = 100;
-    entry *e1 = entryCreate(field, sdslen(field), value1, expiry1);
+    entry *e1 = entryCreate(field, value1, expiry1);
     verify_entry_properties(e1, field, value_copy1, expiry1, true, false);
 
     // Update only value (keeping embedded)
@@ -233,7 +233,7 @@ int test_entryHasexpiry_entrySetExpiry(int argc, char **argv, int flags) {
     // No expiry
     sds field1 = sdsnew(SHORT_FIELD);
     sds value1 = sdsnew(SHORT_VALUE);
-    entry *e1 = entryCreate(field1, sdslen(field1), value1, EXPIRY_NONE);
+    entry *e1 = entryCreate(field1, value1, EXPIRY_NONE);
     TEST_ASSERT(entryHasExpiry(e1) == false);
     TEST_ASSERT(entryGetExpiry(e1) == EXPIRY_NONE);
 
@@ -253,7 +253,7 @@ int test_entryHasexpiry_entrySetExpiry(int argc, char **argv, int flags) {
     // Test with non-embedded entry
     sds field4 = sdsnew(LONG_FIELD);
     sds value4 = sdsnew(LONG_VALUE);
-    entry *e4 = entryCreate(field4, sdslen(field4), value4, EXPIRY_NONE);
+    entry *e4 = entryCreate(field4, value4, EXPIRY_NONE);
     TEST_ASSERT(entryHasExpiry(e4) == false);
     TEST_ASSERT(entryHasEmbeddedValue(e4) == false);
 
@@ -301,7 +301,7 @@ int test_entryIsExpired(int argc, char **argv, int flags) {
     // No expiry
     sds field1 = sdsnew(SHORT_FIELD);
     sds value1 = sdsnew(SHORT_VALUE);
-    entry *e1 = entryCreate(field1, sdslen(field1), value1, EXPIRY_NONE);
+    entry *e1 = entryCreate(field1, value1, EXPIRY_NONE);
     TEST_ASSERT(entryGetExpiry(e1) == EXPIRY_NONE);
     TEST_ASSERT(entryIsExpired(e1) == false);
 
@@ -309,14 +309,14 @@ int test_entryIsExpired(int argc, char **argv, int flags) {
     sds field2 = sdsnew(SHORT_FIELD);
     sds value2 = sdsnew(SHORT_VALUE);
     long long future_time = current_time + 10000; // 10 seconds in future
-    entry *e2 = entryCreate(field2, sdslen(field2), value2, future_time);
+    entry *e2 = entryCreate(field2, value2, future_time);
     TEST_ASSERT(entryGetExpiry(e2) == future_time);
     TEST_ASSERT(entryIsExpired(e2) == false);
 
     // Current time expiry
     sds field3 = sdsnew(SHORT_FIELD);
     sds value3 = sdsnew(SHORT_VALUE);
-    entry *e3 = entryCreate(field3, sdslen(field3), value3, current_time);
+    entry *e3 = entryCreate(field3, value3, current_time);
     TEST_ASSERT(entryGetExpiry(e3) == current_time);
     TEST_ASSERT(entryIsExpired(e3) == false);
 
@@ -324,7 +324,7 @@ int test_entryIsExpired(int argc, char **argv, int flags) {
     sds field4 = sdsnew(SHORT_FIELD);
     sds value4 = sdsnew(SHORT_VALUE);
     long long past_time = current_time - 10000; // 10 seconds ago
-    entry *e4 = entryCreate(field4, sdslen(field4), value4, past_time);
+    entry *e4 = entryCreate(field4, value4, past_time);
     TEST_ASSERT(entryGetExpiry(e4) == past_time);
     TEST_ASSERT(entryIsExpired(e4) == true);
 
@@ -369,7 +369,7 @@ int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int f
     sds value1 = sdsnew(SHORT_VALUE);
     sds value_copy1 = sdsdup(value1);
     long long expiry1 = EXPIRY_NONE;
-    entry *e1 = entryCreate(field1, sdslen(field1), value1, expiry1);
+    entry *e1 = entryCreate(field1, value1, expiry1);
     size_t e1_entryMemUsage = entryMemUsage(e1);
     verify_entry_properties(e1, field1, value_copy1, expiry1, false, false);
     TEST_ASSERT(e1_entryMemUsage > 0);
@@ -416,7 +416,7 @@ int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int f
     sds value6 = sdsnew(LONG_VALUE);
     sds value_copy6 = sdsdup(value6);
     long long expiry6 = EXPIRY_NONE;
-    entry *e6 = entryCreate(field6, sdslen(field6), value6, EXPIRY_NONE);
+    entry *e6 = entryCreate(field6, value6, EXPIRY_NONE);
     size_t e6_entryMemUsage = entryMemUsage(e6);
     verify_entry_properties(e6, field6, value_copy6, expiry6, false, true);
     TEST_ASSERT(e6_entryMemUsage > 0);
diff --git a/src/unit/test_vset.c b/src/unit/test_vset.c
index 1ee92bf48e..4615b5e78c 100644
--- a/src/unit/test_vset.c
+++ b/src/unit/test_vset.c
@@ -16,7 +16,7 @@ typedef entry mock_entry;
 
 static mock_entry *mockCreateEntry(const char *keystr, long long expiry) {
     sds field = sdsnew(keystr);
-    mock_entry *e = entryCreate(field, sdslen(field), sdsnew("value"), expiry);
+    mock_entry *e = entryCreate(field, sdsnew("value"), expiry);
     sdsfree(field);
     return e;
 }
@@ -29,7 +29,7 @@ static void mockFreeEntry(void *entry) {
 static mock_entry *mockEntryUpdate(mock_entry *entry, long long expiry) {
     sds field = entryGetField(entry);
     size_t len;
-    mock_entry *new_entry = entryCreate(field, sdslen(field), sdsdup(entryGetValue(entry, &len)), expiry);
+    mock_entry *new_entry = entryCreate(field, sdsdup(entryGetValue(entry, &len)), expiry);
     entryFree(entry);
     return new_entry;
 }
diff --git a/src/valkeymodule.h b/src/valkeymodule.h
index ce1dd5d385..c47b48b5cb 100644
--- a/src/valkeymodule.h
+++ b/src/valkeymodule.h
@@ -1385,6 +1385,7 @@ VALKEYMODULE_API int (*ValkeyModule_ZsetRangeEndReached)(ValkeyModuleKey *key) V
 VALKEYMODULE_API int (*ValkeyModule_HashSet)(ValkeyModuleKey *key, int flags, ...) VALKEYMODULE_ATTR;
 VALKEYMODULE_API int (*ValkeyModule_HashGet)(ValkeyModuleKey *key, int flags, ...) VALKEYMODULE_ATTR;
 VALKEYMODULE_API int (*ValkeyModule_HashSetValueView)(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) VALKEYMODULE_ATTR;
+VALKEYMODULE_API int (*ValkeyModule_HashHasValueView)(ValkeyModuleKey *key, ValkeyModuleString *field) VALKEYMODULE_ATTR;
 VALKEYMODULE_API int (*ValkeyModule_StreamAdd)(ValkeyModuleKey *key,
                                                int flags,
                                                ValkeyModuleStreamID *id,
@@ -2030,6 +2031,7 @@ static int ValkeyModule_Init(ValkeyModuleCtx *ctx, const char *name, int ver, in
     VALKEYMODULE_GET_API(HashSet);
     VALKEYMODULE_GET_API(HashGet);
     VALKEYMODULE_GET_API(HashSetValueView);
+    VALKEYMODULE_GET_API(HashHasValueView);
     VALKEYMODULE_GET_API(StreamAdd);
     VALKEYMODULE_GET_API(StreamDelete);
     VALKEYMODULE_GET_API(StreamIteratorStart);

From 6f7f99022dafca8fb3b54155022dc395035e9010 Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Tue, 19 Aug 2025 03:03:19 +0000
Subject: [PATCH 07/30] renaming, bug fixing and adding unittests

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/db.c              |  8 ++---
 src/entry.c           | 82 +++++++++++++++++++++----------------------
 src/entry.h           |  8 ++---
 src/module.c          | 14 ++++----
 src/server.h          |  4 +--
 src/t_hash.c          | 12 +++----
 src/unit/test_entry.c | 50 ++++++++++++++++++++++++++
 src/unit/test_files.h |  3 +-
 src/valkeymodule.h    |  8 ++---
 9 files changed, 119 insertions(+), 70 deletions(-)

diff --git a/src/db.c b/src/db.c
index bc612e6c8b..0f40e8aea7 100644
--- a/src/db.c
+++ b/src/db.c
@@ -1005,7 +1005,7 @@ int objectTypeCompare(robj *o, long long target) {
 }
 
 static void addScanDataItem(vector *result, const char *buf, size_t len) {
-    bufferView *item = vectorPush(result);
+    stringRef *item = vectorPush(result);
     item->buf = buf;
     item->len = len;
 }
@@ -1047,7 +1047,7 @@ void keysScanCallback(void *privdata, void *entry, int didx) {
  * returned by the dictionary iterator into a list. */
 void hashtableScanCallback(void *privdata, void *entry) {
     scanData *data = (scanData *)privdata;
-    bufferView val = {NULL, 0};
+    stringRef val = {NULL, 0};
     sds key = NULL;
 
     robj *o = data->o;
@@ -1252,7 +1252,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
         /* scanning ZSET allocates temporary strings even though it's a dict */
         free_callback = sdsfree;
     }
-    vectorInit(&result, SCAN_VECTOR_INITIAL_ALLOC, sizeof(bufferView));
+    vectorInit(&result, SCAN_VECTOR_INITIAL_ALLOC, sizeof(stringRef));
 
     /* For main hash table scan or scannable data structure. */
     if (!o || ht) {
@@ -1355,7 +1355,7 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
 
     addReplyArrayLen(c, vectorLen(&result));
     for (uint32_t i = 0; i < vectorLen(&result); i++) {
-        bufferView *key = vectorGet(&result, i);
+        stringRef *key = vectorGet(&result, i);
         addReplyBulkCBuffer(c, key->buf, key->len);
         if (free_callback) {
             free_callback((sds)(key->buf));
diff --git a/src/entry.c b/src/entry.c
index b97779bbcd..259cfb1f91 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -43,11 +43,11 @@ enum {
      * pointer located in memory before the embedded field. If unset, the entry
      * instead has an embedded value located after the embedded field. */
     FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR = 1,
-    /* SDS aux flag.  If set, it indicates that the hash entry's value is a **view** of the data.
-     * The hash entry does not own the view buffer and will not free it upon
+    /* SDS aux flag.  If set, it indicates that the hash entry reference the value.
+     * The hash entry does not own the string reference and will not free it upon
      * entry destruction. This is useful for avoiding memory duplication
      * between the core and a module. */
-    FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW = 2,
+    FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF = 2,
     FIELD_SDS_AUX_BIT_MAX
 };
 static_assert(FIELD_SDS_AUX_BIT_MAX < sizeof(char) - SDS_TYPE_BITS, "too many sds bits are used for entry metadata");
@@ -64,10 +64,10 @@ bool entryHasEmbeddedValue(const entry *entry) {
     return (!entryHasValuePtr(entry));
 }
 
-/* Returns true in case the entry holds a view of the value.
+/* Returns true in case the entry holds a reference of the value.
  * Returns false otherwise. */
-bool entryHasValueView(const entry *entry) {
-    return entryHasValuePtr(entry) && sdsGetAuxBit(entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW);
+bool entryHasStringRef(const entry *entry) {
+    return entryHasValuePtr(entry) && sdsGetAuxBit(entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF);
 }
 /* Returns true in case the entry has expiration timestamp.
  * Returns false otherwise. */
@@ -100,10 +100,10 @@ char *entryGetValue(const entry *entry, size_t *len) {
     }
     void **value_ref = entryGetValueRef(entry);
     if (!value_ref) return NULL;
-    if (entryHasValueView(entry)) {
-        bufferView *buffer_view = (bufferView *)*value_ref;
-        if (len) *len = buffer_view->len;
-        return (char *)buffer_view->buf;
+    if (entryHasStringRef(entry)) {
+        stringRef *string_ref = (stringRef *)*value_ref;
+        if (len) *len = string_ref->len;
+        return (char *)string_ref->buf;
     }
     if (len) *len = sdslen(*value_ref);
     return *value_ref;
@@ -112,10 +112,10 @@ char *entryGetValue(const entry *entry, size_t *len) {
 static void entrySetValuePtr(entry *e, sds value) {
     serverAssert(entryHasValuePtr(e));
     void **value_ref = entryGetValueRef(e);
-    if (entryHasValueView(e)) {
+    if (entryHasStringRef(e)) {
         zfree(*value_ref);
-        *value_ref = (bufferView *)value;
-        sdsSetAuxBit(e, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW, 0);
+        *value_ref = (stringRef *)value;
+        sdsSetAuxBit(e, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 0);
     } else {
         sdsfree(*value_ref);
     }
@@ -138,29 +138,26 @@ static void *entryGetAllocPtr(const entry *entry) {
 }
 
 /**************************************** Entry Expiry API *****************************************/
+static inline long long *entryGetExpiryRef(const entry *entry) {
+    debugServerAssert(entryHasExpiry(entry));
+    char *buf = entryGetAllocPtr(entry);
+    debugServerAssert((((uintptr_t)buf & 0x7) == 0)); /* Test that the allocation is indeed 8 bytes aligned
+                                                       * This is needed since we access the expiry as with pointer casting
+                                                       * which require the access to be 8 bytes aligned. */
+    return (long long *)buf;
+}
 
 /* Returns the entry expiration timestamp.
  * In case this entry has no expiration time, will return EXPIRE_NONE. */
 long long entryGetExpiry(const entry *entry) {
-    long long expiry = EXPIRY_NONE;
-    if (entryHasExpiry(entry)) {
-        char *buf = entryGetAllocPtr(entry);
-        debugServerAssert((((uintptr_t)buf & 0x7) == 0)); /* Test that the allocation is indeed 8 bytes aligned
-                                                           * This is needed since we access the expiry as with pointer casting
-                                                           * which require the access to be 8 bytes aligned. */
-        expiry = *(long long *)buf;
-    }
-    return expiry;
+    if (entryHasExpiry(entry)) return *entryGetExpiryRef(entry);
+    return EXPIRY_NONE;
 }
 
 /* Modify the expiration time of this entry and return a pointer to the (potentially new) entry. */
 entry *entrySetExpiry(entry *e, long long expiry) {
     if (entryHasExpiry(e)) {
-        char *buf = entryGetAllocPtr(e);
-        debugServerAssert((((uintptr_t)buf & 0x7) == 0)); /* Test that the allocation is indeed 8 bytes aligned
-                                                           * This is needed since we access the expiry as with pointer casting
-                                                           * which require the access to be 8 bytes aligned. */
-        *(long long *)buf = expiry;
+        *entryGetExpiryRef(e) = expiry;
         return e;
     }
     return entryUpdate(e, NULL, expiry);
@@ -175,7 +172,7 @@ bool entryIsExpired(entry *entry) {
 static inline void entryFreeValuePtr(entry *entry) {
     serverAssert(entryHasValuePtr(entry));
     void **value_ref = entryGetValueRef(entry);
-    if (entryHasValueView(entry)) {
+    if (entryHasStringRef(entry)) {
         zfree(*value_ref);
         return;
     }
@@ -301,24 +298,26 @@ entry *entryCreate(const_sds field, sds value, long long expiry) {
     return entryWrite(buf, buf_size, field, value, expiry, embed_value, embedded_field_sds_type, embedded_field_sds_size, embedded_value_sds_size, expiry_size);
 }
 
-entry *entrySetValueView(entry *entry, const char *buf, size_t len, long long expiry) {
+entry *entrySetStringRef(entry *entry, const char *buf, size_t len, long long expiry) {
     long long entry_expiry = entryGetExpiry(entry);
     // Check for toggling expiration
     bool expiry_add_remove = (expiry != entry_expiry) && (entry_expiry == EXPIRY_NONE || expiry == EXPIRY_NONE);
-    if (entryHasValueView(entry) && !expiry_add_remove) {
-        bufferView *value = *entryGetValueRef(entry);
+    if (entryHasStringRef(entry) && !expiry_add_remove) {
+        stringRef *value = *entryGetValueRef(entry);
         value->buf = buf;
         value->len = len;
+        if (expiry != EXPIRY_NONE) *entryGetExpiryRef(entry) = expiry;
         return entry;
     }
-    bufferView *value = zmalloc(sizeof(bufferView));
+    stringRef *value = zmalloc(sizeof(stringRef));
     value->buf = buf;
     value->len = len;
     if (entryHasValuePtr(entry) && !expiry_add_remove) {
         sds *value_ref = (sds *)entryGetValueRef(entry);
         sdsfree(*value_ref);
         *value_ref = (sds)value;
-        sdsSetAuxBit(entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW, 1);
+        sdsSetAuxBit(entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
+        if (expiry != EXPIRY_NONE) *entryGetExpiryRef(entry) = expiry;
         return entry;
     }
     sds field = entryGetField(entry);
@@ -335,15 +334,16 @@ entry *entrySetValueView(entry *entry, const char *buf, size_t len, long long ex
         buf_sds += sizeof(expiry);
         buf_size -= sizeof(expiry);
     }
-    *(bufferView **)buf_sds = value;
+    *(stringRef **)buf_sds = value;
     buf_sds += sizeof(value);
     buf_size -= sizeof(value);
     /* Set the field data */
     sds new_entry = sdswrite(buf_sds, field_size, SDS_TYPE_8, field, sdslen(field));
     entryFree(entry);
 
-    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW, 1);
+    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
     sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR, 1);
+    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_EXPIRY, expiry != EXPIRY_NONE);
     return new_entry;
 }
 /* Modify the entry's value and/or expiration time.
@@ -354,10 +354,9 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
     sds field = (sds)e;
     entry *new_entry = NULL;
 
-    if (entryHasValueView(e)) {
-        serverAssert(!value);
-        bufferView *value = (bufferView *)*entryGetValueRef(e);
-        return entrySetValueView(e, value->buf, value->len, expiry);
+    if (entryHasStringRef(e) && !value) {
+        stringRef *value = (stringRef *)*entryGetValueRef(e);
+        return entrySetStringRef(e, value->buf, value->len, expiry);
     }
     bool update_value = value ? true : false;
     long long curr_expiration_time = entryGetExpiry(e);
@@ -431,7 +430,6 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
         size_t buf_size;
         char *buf = zmalloc_usable(required_entry_size, &buf_size);
         new_entry = entryWrite(buf, buf_size, field, value, expiry, embed_value, embedded_field_sds_type, embedded_field_size, embedded_value_size, expiry_size);
-        sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW, sdsGetAuxBit(e, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_VIEW));
         entryFree(e);
     }
     /* Check that the new entry was built correctly */
@@ -454,7 +452,7 @@ size_t entryMemUsage(entry *entry) {
          * header could be too small for holding the real allocation size. */
         mem += zmalloc_usable_size(entryGetAllocPtr(entry));
     }
-    mem += sdsAllocSize((sds)entryGetValue(entry, NULL));
+    if (!entryHasStringRef(entry)) mem += sdsAllocSize((sds)entryGetValue(entry, NULL));
     return mem;
 }
 
@@ -466,7 +464,7 @@ size_t entryMemUsage(entry *entry) {
  * If the location of the entry changed we return the new location,
  * otherwise we return NULL. */
 entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) {
-    if (entryHasValueView(entry)) return NULL;
+    if (entryHasStringRef(entry)) return NULL;
     if (entryHasValuePtr(entry)) {
         sds *value_ref = (sds *)entryGetValueRef(entry);
         sds new_value = sdsdefragfn(*value_ref);
diff --git a/src/entry.h b/src/entry.h
index 7c666f5877..b812046e42 100644
--- a/src/entry.h
+++ b/src/entry.h
@@ -8,10 +8,10 @@
  * The view does not manage the underlying memory, so its destruction
  * will not free the buffer.
  */
-typedef struct bufferView {
+typedef struct stringRef {
     const char *buf; /* Pointer to the externalized buffer */
     size_t len;      /* Length of the buffer */
-} bufferView;
+} stringRef;
 
 /*-----------------------------------------------------------------------------
  * Entry
@@ -72,7 +72,7 @@ long long entryGetExpiry(const entry *entry);
 bool entryHasExpiry(const entry *entry);
 
 /* Returns true if the entry value is externalized. */
-bool entryHasValueView(const entry *entry);
+bool entryHasStringRef(const entry *entry);
 
 /* Sets the expiration timestamp. */
 entry *entrySetExpiry(entry *entry, long long expiry);
@@ -85,7 +85,7 @@ void entryFree(entry *entry);
 
 /* Creates a new entry with the given field, value, and optional expiry. */
 entry *entryCreate(const_sds field, sds value, long long expiry);
-entry *entrySetValueView(entry *entry, const char *buf, size_t len, long long expiry);
+entry *entrySetStringRef(entry *entry, const char *buf, size_t len, long long expiry);
 
 /* Updates the value and/or expiry of an existing entry.
  * In case value is NULL, will use the existing entry value.
diff --git a/src/module.c b/src/module.c
index 95cb8a5f69..ea1b5ccc01 100644
--- a/src/module.c
+++ b/src/module.c
@@ -5251,16 +5251,16 @@ int VM_ZsetRangePrev(ValkeyModuleKey *key) {
  * See also VM_ValueLength(), which returns the number of fields in a hash.
  * -------------------------------------------------------------------------- */
 
-/* Sets a view of a buffer to a hash field.
+/* Sets a reference to the value.
  * The function takes the hash key, hash field, and a buffer along with its length. */
-int VM_HashSetValueView(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) {
+int VM_HashSetStringRef(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) {
     if (!key || !key->value || key->value->type != OBJ_HASH || !field || !buf) return VALKEYMODULE_ERR;
-    return hashTypeSetValueView(key->value, field->ptr, buf, len);
+    return hashTypeSetStringRef(key->value, field->ptr, buf, len);
 }
 
-int VM_HashHasValueView(ValkeyModuleKey *key, ValkeyModuleString *field) {
+int VM_HashHasStringRef(ValkeyModuleKey *key, ValkeyModuleString *field) {
     if (!key || !key->value || key->value->type != OBJ_HASH) return VALKEYMODULE_ERR;
-    return hashTypeHasValueView(key->value, field->ptr);
+    return hashTypeHasStringRef(key->value, field->ptr);
 }
 /* Set the field of the specified hash field to the specified value.
  * If the key is an empty key open for writing, it is created with an empty
@@ -14001,8 +14001,8 @@ void moduleRegisterCoreAPI(void) {
     REGISTER_API(ZsetRangeEndReached);
     REGISTER_API(HashSet);
     REGISTER_API(HashGet);
-    REGISTER_API(HashSetValueView);
-    REGISTER_API(HashHasValueView);
+    REGISTER_API(HashSetStringRef);
+    REGISTER_API(HashHasStringRef);
     REGISTER_API(StreamAdd);
     REGISTER_API(StreamDelete);
     REGISTER_API(StreamIteratorStart);
diff --git a/src/server.h b/src/server.h
index f80a2d0b29..b294c7c00f 100644
--- a/src/server.h
+++ b/src/server.h
@@ -3419,8 +3419,8 @@ robj *hashTypeGetValueObject(robj *o, sds field);
 int hashTypeSet(robj *o, sds field, sds value, long long expiry, int flags);
 robj *hashTypeDup(robj *o);
 bool hashTypeHasVolatileFields(robj *o);
-int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len);
-int hashTypeHasValueView(robj *o, sds field);
+int hashTypeSetStringRef(robj *o, sds field, const char *buf, size_t len);
+int hashTypeHasStringRef(robj *o, sds field);
 
 /* Pub / Sub */
 int pubsubUnsubscribeAllChannels(client *c, int notify);
diff --git a/src/t_hash.c b/src/t_hash.c
index 492cc26e02..fb01c45e6d 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -311,16 +311,16 @@ int hashTypeExists(robj *o, sds field) {
     return hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) == C_OK;
 }
 
-int hashTypeHasValueView(robj *o, sds field) {
+int hashTypeHasStringRef(robj *o, sds field) {
     if (o->encoding == OBJ_ENCODING_LISTPACK) return 0;
     hashtable *ht = o->ptr;
     void **entry_ref = hashtableFindRef(ht, field);
-    return (entryHasValueView(*entry_ref));
+    return (entryHasStringRef(*entry_ref));
 }
-/* Set a view value field in a hash.
+/* Set a reference to the value.
  * Returns C_ERR on error, C_OK on update.
  */
-int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len) {
+int hashTypeSetStringRef(robj *o, sds field, const char *buf, size_t len) {
     unsigned char *vstr = NULL;
     unsigned int vlen = UINT_MAX;
     long long vll = LLONG_MAX;
@@ -333,8 +333,8 @@ int hashTypeSetValueView(robj *o, sds field, const char *buf, size_t len) {
     hashtable *ht = o->ptr;
     void **entry_ref = hashtableFindRef(ht, field);
     entry *entry = *entry_ref;
-    if (entryHasValueView(entry)) return C_ERR;
-    entrySetValueView(entry, buf, len, entryGetExpiry(entry));
+    if (entryHasStringRef(entry)) return C_ERR;
+    entrySetStringRef(entry, buf, len, entryGetExpiry(entry));
     return C_OK;
 }
 
diff --git a/src/unit/test_entry.c b/src/unit/test_entry.c
index bc20def9ea..2ed13892d2 100644
--- a/src/unit/test_entry.c
+++ b/src/unit/test_entry.c
@@ -470,3 +470,53 @@ int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int f
 
     return 0;
 }
+
+int test_entryStringRef(int argc, char **argv, int flags) {
+    UNUSED(argc);
+    UNUSED(argv);
+    UNUSED(flags);
+    sds field1 = sdsnew(SHORT_FIELD);
+    sds value1 = sdsnew(SHORT_VALUE);
+    sds value_copy1 = sdsdup(value1);
+    long long expiry1 = EXPIRY_NONE;
+    entry *e1 = entryCreate(field1, value1, expiry1);
+    entry *e2 = entrySetStringRef(e1, value_copy1, sdslen(value_copy1), entryGetExpiry(e1));
+    verify_entry_properties(e2, field1, value_copy1, expiry1, false, true);
+    TEST_ASSERT(entryHasStringRef(e2) == true);
+
+    long long expiry2 = 100;
+    entry *e3 = entrySetStringRef(e2, value_copy1, sdslen(value_copy1), expiry2);
+    TEST_ASSERT(e2 != e3);
+    verify_entry_properties(e3, field1, value_copy1, expiry2, true, true);
+    TEST_ASSERT(entryHasStringRef(e3) == true);
+
+    long long expiry3 = 200;
+    entry *e4 = entrySetStringRef(e3, value_copy1, sdslen(value_copy1), expiry3);
+    TEST_ASSERT(e3 == e4);
+    verify_entry_properties(e4, field1, value_copy1, expiry3, true, true);
+    TEST_ASSERT(entryHasStringRef(e4) == true);
+
+    sds value2 = sdsnew(SHORT_VALUE);
+    sds value_copy2 = sdsdup(value2);
+    entry *e5 = entryUpdate(e4, value2, expiry3);
+    verify_entry_properties(e5, field1, value_copy2, expiry3, true, false);
+    TEST_ASSERT(entryHasStringRef(e5) == false);
+
+    entry *e6 = entrySetStringRef(e5, value_copy1, sdslen(value_copy1), expiry2);
+    TEST_ASSERT(e5 != e6);
+    verify_entry_properties(e6, field1, value_copy1, expiry2, true, true);
+    TEST_ASSERT(entryHasStringRef(e6) == true);
+
+    sds value3 = sdsnew(LONG_VALUE);
+    sds value_copy3 = sdsdup(value3);
+    entry *e7 = entryUpdate(e6, value3, expiry1);
+    verify_entry_properties(e7, field1, value_copy3, expiry1, false, true);
+    TEST_ASSERT(entryHasStringRef(e7) == false);
+
+    entryFree(e7);
+    sdsfree(value_copy1);
+    sdsfree(value_copy2);
+    sdsfree(value_copy3);
+    sdsfree(field1);
+    return 0;
+}
diff --git a/src/unit/test_files.h b/src/unit/test_files.h
index 34459201bb..d7a69366af 100644
--- a/src/unit/test_files.h
+++ b/src/unit/test_files.h
@@ -25,6 +25,7 @@ int test_entryUpdate(int argc, char **argv, int flags);
 int test_entryHasexpiry_entrySetExpiry(int argc, char **argv, int flags);
 int test_entryIsExpired(int argc, char **argv, int flags);
 int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int flags);
+int test_entryStringRef(int argc, char **argv, int flags);
 int test_cursor(int argc, char **argv, int flags);
 int test_set_hash_function_seed(int argc, char **argv, int flags);
 int test_add_find_delete(int argc, char **argv, int flags);
@@ -255,7 +256,7 @@ unitTest __test_crc64_c[] = {{"test_crc64", test_crc64}, {NULL, NULL}};
 unitTest __test_crc64combine_c[] = {{"test_crc64combine", test_crc64combine}, {NULL, NULL}};
 unitTest __test_dict_c[] = {{"test_dictCreate", test_dictCreate}, {"test_dictAdd16Keys", test_dictAdd16Keys}, {"test_dictDisableResize", test_dictDisableResize}, {"test_dictAddOneKeyTriggerResize", test_dictAddOneKeyTriggerResize}, {"test_dictDeleteKeys", test_dictDeleteKeys}, {"test_dictDeleteOneKeyTriggerResize", test_dictDeleteOneKeyTriggerResize}, {"test_dictEmptyDirAdd128Keys", test_dictEmptyDirAdd128Keys}, {"test_dictDisableResizeReduceTo3", test_dictDisableResizeReduceTo3}, {"test_dictDeleteOneKeyTriggerResizeAgain", test_dictDeleteOneKeyTriggerResizeAgain}, {"test_dictBenchmark", test_dictBenchmark}, {NULL, NULL}};
 unitTest __test_endianconv_c[] = {{"test_endianconv", test_endianconv}, {NULL, NULL}};
-unitTest __test_entry_c[] = {{"test_entryCreate", test_entryCreate}, {"test_entryUpdate", test_entryUpdate}, {"test_entryHasexpiry_entrySetExpiry", test_entryHasexpiry_entrySetExpiry}, {"test_entryIsExpired", test_entryIsExpired}, {"test_entryMemUsage_entrySetExpiry_entrySetValue", test_entryMemUsage_entrySetExpiry_entrySetValue}, {NULL, NULL}};
+unitTest __test_entry_c[] = {{"test_entryCreate", test_entryCreate}, {"test_entryUpdate", test_entryUpdate}, {"test_entryHasexpiry_entrySetExpiry", test_entryHasexpiry_entrySetExpiry}, {"test_entryIsExpired", test_entryIsExpired}, {"test_entryMemUsage_entrySetExpiry_entrySetValue", test_entryMemUsage_entrySetExpiry_entrySetValue}, {"test_entryStringRef", test_entryStringRef}, {NULL, NULL}};
 unitTest __test_hashtable_c[] = {{"test_cursor", test_cursor}, {"test_set_hash_function_seed", test_set_hash_function_seed}, {"test_add_find_delete", test_add_find_delete}, {"test_add_find_delete_avoid_resize", test_add_find_delete_avoid_resize}, {"test_instant_rehashing", test_instant_rehashing}, {"test_bucket_chain_length", test_bucket_chain_length}, {"test_two_phase_insert_and_pop", test_two_phase_insert_and_pop}, {"test_replace_reallocated_entry", test_replace_reallocated_entry}, {"test_incremental_find", test_incremental_find}, {"test_scan", test_scan}, {"test_iterator", test_iterator}, {"test_safe_iterator", test_safe_iterator}, {"test_compact_bucket_chain", test_compact_bucket_chain}, {"test_random_entry", test_random_entry}, {"test_random_entry_with_long_chain", test_random_entry_with_long_chain}, {"test_random_entry_sparse_table", test_random_entry_sparse_table}, {NULL, NULL}};
 unitTest __test_intset_c[] = {{"test_intsetValueEncodings", test_intsetValueEncodings}, {"test_intsetBasicAdding", test_intsetBasicAdding}, {"test_intsetLargeNumberRandomAdd", test_intsetLargeNumberRandomAdd}, {"test_intsetUpgradeFromint16Toint32", test_intsetUpgradeFromint16Toint32}, {"test_intsetUpgradeFromint16Toint64", test_intsetUpgradeFromint16Toint64}, {"test_intsetUpgradeFromint32Toint64", test_intsetUpgradeFromint32Toint64}, {"test_intsetStressLookups", test_intsetStressLookups}, {"test_intsetStressAddDelete", test_intsetStressAddDelete}, {NULL, NULL}};
 unitTest __test_kvstore_c[] = {{"test_kvstoreAdd16Keys", test_kvstoreAdd16Keys}, {"test_kvstoreIteratorRemoveAllKeysNoDeleteEmptyHashtable", test_kvstoreIteratorRemoveAllKeysNoDeleteEmptyHashtable}, {"test_kvstoreIteratorRemoveAllKeysDeleteEmptyHashtable", test_kvstoreIteratorRemoveAllKeysDeleteEmptyHashtable}, {"test_kvstoreHashtableIteratorRemoveAllKeysNoDeleteEmptyHashtable", test_kvstoreHashtableIteratorRemoveAllKeysNoDeleteEmptyHashtable}, {"test_kvstoreHashtableIteratorRemoveAllKeysDeleteEmptyHashtable", test_kvstoreHashtableIteratorRemoveAllKeysDeleteEmptyHashtable}, {NULL, NULL}};
diff --git a/src/valkeymodule.h b/src/valkeymodule.h
index c47b48b5cb..00997d4afb 100644
--- a/src/valkeymodule.h
+++ b/src/valkeymodule.h
@@ -1384,8 +1384,8 @@ VALKEYMODULE_API int (*ValkeyModule_ZsetRangePrev)(ValkeyModuleKey *key) VALKEYM
 VALKEYMODULE_API int (*ValkeyModule_ZsetRangeEndReached)(ValkeyModuleKey *key) VALKEYMODULE_ATTR;
 VALKEYMODULE_API int (*ValkeyModule_HashSet)(ValkeyModuleKey *key, int flags, ...) VALKEYMODULE_ATTR;
 VALKEYMODULE_API int (*ValkeyModule_HashGet)(ValkeyModuleKey *key, int flags, ...) VALKEYMODULE_ATTR;
-VALKEYMODULE_API int (*ValkeyModule_HashSetValueView)(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) VALKEYMODULE_ATTR;
-VALKEYMODULE_API int (*ValkeyModule_HashHasValueView)(ValkeyModuleKey *key, ValkeyModuleString *field) VALKEYMODULE_ATTR;
+VALKEYMODULE_API int (*ValkeyModule_HashSetStringRef)(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) VALKEYMODULE_ATTR;
+VALKEYMODULE_API int (*ValkeyModule_HashHasStringRef)(ValkeyModuleKey *key, ValkeyModuleString *field) VALKEYMODULE_ATTR;
 VALKEYMODULE_API int (*ValkeyModule_StreamAdd)(ValkeyModuleKey *key,
                                                int flags,
                                                ValkeyModuleStreamID *id,
@@ -2030,8 +2030,8 @@ static int ValkeyModule_Init(ValkeyModuleCtx *ctx, const char *name, int ver, in
     VALKEYMODULE_GET_API(ZsetRangeEndReached);
     VALKEYMODULE_GET_API(HashSet);
     VALKEYMODULE_GET_API(HashGet);
-    VALKEYMODULE_GET_API(HashSetValueView);
-    VALKEYMODULE_GET_API(HashHasValueView);
+    VALKEYMODULE_GET_API(HashSetStringRef);
+    VALKEYMODULE_GET_API(HashHasStringRef);
     VALKEYMODULE_GET_API(StreamAdd);
     VALKEYMODULE_GET_API(StreamDelete);
     VALKEYMODULE_GET_API(StreamIteratorStart);

From 848fd6db08d0114555c0e43a0b49196e3af753b0 Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Tue, 19 Aug 2025 03:11:54 +0000
Subject: [PATCH 08/30] unittest fix

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/unit/test_entry.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/unit/test_entry.c b/src/unit/test_entry.c
index 2ed13892d2..d70b0b6cdf 100644
--- a/src/unit/test_entry.c
+++ b/src/unit/test_entry.c
@@ -42,9 +42,9 @@ int test_entryCreate(int argc, char **argv, int flags) {
     sds field1 = sdsnew(SHORT_FIELD);
     sds value1 = sdsnew(SHORT_VALUE);
     sds value_copy1 = sdsdup(value1); // Keep a copy since entryCreate takes ownership of value
-    long long expiry1 = EXPIRY_NONE;
+    long long expiry1 = 100;
     entry *e1 = entryCreate(field1, value1, expiry1);
-    verify_entry_properties(e1, field1, value_copy1, expiry1, false, false);
+    verify_entry_properties(e1, field1, value_copy1, expiry1, true, false);
 
     // Test with embedded value with no expiry
     sds field2 = sdsnew(SHORT_FIELD);

From a7a1537ce203d08a973650d13f07d42d1dc8c37d Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Tue, 19 Aug 2025 03:15:48 +0000
Subject: [PATCH 09/30] cosmetic change

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/aof.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/aof.c b/src/aof.c
index edc9fbb663..8a81a60314 100644
--- a/src/aof.c
+++ b/src/aof.c
@@ -1964,7 +1964,7 @@ int rewriteHashObject(rio *r, robj *key, robj *o) {
             long long expiry = entryGetExpiry(hi.next);
             sds field = entryGetField(hi.next);
             size_t value_len;
-            sds value = entryGetValue(hi.next, &value_len);
+            char *value = entryGetValue(hi.next, &value_len);
             if (rioWriteBulkCount(r, '*', 8) == 0) return 0;
             if (rioWriteBulkString(r, "HSETEX", 6) == 0) return 0;
             if (rioWriteBulkObject(r, key) == 0) return 0;

From 06b6cfc7da5c7ca3a9098c8030ec0b19e627e57d Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Tue, 19 Aug 2025 13:31:06 +0000
Subject: [PATCH 10/30] fixing module tests

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/t_hash.c                                  |  5 +++-
 tests/modules/Makefile                        |  2 +-
 .../{hash_view_value.c => hash_stringref.c}   | 23 +++++++++++++++----
 tests/unit/moduleapi/hash_stringref.tcl       | 22 ++++++++++++++++++
 tests/unit/moduleapi/hash_view_value.tcl      | 21 -----------------
 5 files changed, 45 insertions(+), 28 deletions(-)
 rename tests/modules/{hash_view_value.c => hash_stringref.c} (63%)
 create mode 100644 tests/unit/moduleapi/hash_stringref.tcl
 delete mode 100644 tests/unit/moduleapi/hash_view_value.tcl

diff --git a/src/t_hash.c b/src/t_hash.c
index fb01c45e6d..20cd78e1fd 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -334,7 +334,10 @@ int hashTypeSetStringRef(robj *o, sds field, const char *buf, size_t len) {
     void **entry_ref = hashtableFindRef(ht, field);
     entry *entry = *entry_ref;
     if (entryHasStringRef(entry)) return C_ERR;
-    entrySetStringRef(entry, buf, len, entryGetExpiry(entry));
+    long long expiry = entryGetExpiry(entry);
+    void *new_entry = entrySetStringRef(entry, buf, len, expiry);
+    serverAssert(hashtableReplaceReallocatedEntry(ht, entry, new_entry));
+    hashTypeTrackUpdateEntry(o, entry, new_entry, expiry, expiry);
     return C_OK;
 }
 
diff --git a/tests/modules/Makefile b/tests/modules/Makefile
index 90ecc50571..9811dc883f 100644
--- a/tests/modules/Makefile
+++ b/tests/modules/Makefile
@@ -48,7 +48,7 @@ TEST_MODULES = \
     defragtest.so \
     keyspecs.so \
     hash.so \
-    hash_view_value.so \
+    hash_stringref.so \
     zset.so \
     stream.so \
     mallocsize.so \
diff --git a/tests/modules/hash_view_value.c b/tests/modules/hash_stringref.c
similarity index 63%
rename from tests/modules/hash_view_value.c
rename to tests/modules/hash_stringref.c
index 2bbb2a1152..54cf5701ae 100644
--- a/tests/modules/hash_view_value.c
+++ b/tests/modules/hash_stringref.c
@@ -31,7 +31,17 @@ void freeBufferList(void) {
     }
 }
 
-int hashSetValueView(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {
+int hashHasStringRef(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {
+    if (argc != 3) return ValkeyModule_WrongArity(ctx);
+
+    ValkeyModule_AutoMemory(ctx);
+    ValkeyModuleKey *key = ValkeyModule_OpenKey(ctx, argv[1], VALKEYMODULE_WRITE);
+
+    int result = ValkeyModule_HashHasStringRef(key, argv[2]);
+    return ValkeyModule_ReplyWithLongLong(ctx, result);
+}
+
+int hashSetStringRef(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {
     if (argc != 4) return ValkeyModule_WrongArity(ctx);
 
     ValkeyModule_AutoMemory(ctx);
@@ -41,16 +51,19 @@ int hashSetValueView(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc)
     const char *buf = ValkeyModule_StringPtrLen(argv[3], &buf_len);
     bufferNode *node = addBuffer(buf, buf_len);
 
-    int result = ValkeyModule_HashSetValueView(key, argv[2], node->buf, node->len);
-    return ValkeyModule_ReplyWithLongLong(ctx, result);
+    int result = ValkeyModule_HashSetStringRef(key, argv[2], node->buf, node->len);
+    if (result == 0) return ValkeyModule_ReplyWithLongLong(ctx, result);
+    return ValkeyModule_ReplyWithError(ctx, "Err");
 }
 
 int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {
     VALKEYMODULE_NOT_USED(argv);
     VALKEYMODULE_NOT_USED(argc);
-    if (ValkeyModule_Init(ctx, "hash.set_view", 1, VALKEYMODULE_APIVER_1) ==
+    if (ValkeyModule_Init(ctx, "hash.stringref", 1, VALKEYMODULE_APIVER_1) ==
         VALKEYMODULE_OK &&
-        ValkeyModule_CreateCommand(ctx, "hash.set_view", hashSetValueView, "write",
+        ValkeyModule_CreateCommand(ctx, "hash.set_stringref", hashSetStringRef, "write",
+                                  1, 1, 1) == VALKEYMODULE_OK &&
+        ValkeyModule_CreateCommand(ctx, "hash.has_stringref", hashHasStringRef, "write",
                                   1, 1, 1) == VALKEYMODULE_OK) {
         return VALKEYMODULE_OK;
     }
diff --git a/tests/unit/moduleapi/hash_stringref.tcl b/tests/unit/moduleapi/hash_stringref.tcl
new file mode 100644
index 0000000000..938865d4b8
--- /dev/null
+++ b/tests/unit/moduleapi/hash_stringref.tcl
@@ -0,0 +1,22 @@
+set testmodule [file normalize tests/modules/hash_stringref.so]
+
+start_server {tags {"modules"}} {
+    r module load $testmodule
+
+    test {Module hash set} {
+        r del k
+        set status [catch {r hash.set_stringref k f hello} errmsg]
+        assert {$status == 1}
+        r hset k f hello1
+        assert_equal "0" [r hash.has_stringref k f]
+        set status [catch {r hash.set_stringref k f hello2} errmsg]
+        assert {$status == 1}
+        r hash.set_stringref k f hello1
+        assert_equal "hello1" [r hget k f]
+        assert_equal "1" [r hash.has_stringref k f]
+    }
+
+    test "Unload the module - hash" {
+        assert_equal {OK} [r module unload hash.stringref]
+    }
+}
diff --git a/tests/unit/moduleapi/hash_view_value.tcl b/tests/unit/moduleapi/hash_view_value.tcl
deleted file mode 100644
index 7ed715bae8..0000000000
--- a/tests/unit/moduleapi/hash_view_value.tcl
+++ /dev/null
@@ -1,21 +0,0 @@
-set testmodule [file normalize tests/modules/hash_view_value.so]
-
-start_server {tags {"modules"}} {
-    r module load $testmodule
-
-    test {Module hash set} {
-        r hash.set_view k f hello
-        assert_equal "hello" [r hget k f]
-        r hgetall k
-        r hset k f hello1
-        r hash.set_view k f hello2
-        assert_equal "hello2" [r hget k f]
-        r hdel k f
-        r hash.set_view k f hello3
-        assert_equal "hello3" [r hget k f]
-    }
-
-    test "Unload the module - hash" {
-        assert_equal {OK} [r module unload hash.set_view]
-    }
-}

From 356891da3635d648f4dc89cf952f8ccf1523ecb2 Mon Sep 17 00:00:00 2001
From: yairgott <yairgott@gmail.com>
Date: Wed, 20 Aug 2025 14:26:01 +0000
Subject: [PATCH 11/30] fixing comments

Signed-off-by: yairgott <yairgott@gmail.com>
---
 src/entry.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/entry.c b/src/entry.c
index 259cfb1f91..d46157c913 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -43,9 +43,9 @@ enum {
      * pointer located in memory before the embedded field. If unset, the entry
      * instead has an embedded value located after the embedded field. */
     FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR = 1,
-    /* SDS aux flag.  If set, it indicates that the hash entry reference the value.
+    /* SDS aux flag.  If set, it indicates that the hash entry has a reference to the value.
      * The hash entry does not own the string reference and will not free it upon
-     * entry destruction. This is useful for avoiding memory duplication
+     * entry destruction. The primary usecase is to avoid memory duplication
      * between the core and a module. */
     FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF = 2,
     FIELD_SDS_AUX_BIT_MAX
@@ -138,6 +138,7 @@ static void *entryGetAllocPtr(const entry *entry) {
 }
 
 /**************************************** Entry Expiry API *****************************************/
+/* Returns the location of a pointer to the expiry */
 static inline long long *entryGetExpiryRef(const entry *entry) {
     debugServerAssert(entryHasExpiry(entry));
     char *buf = entryGetAllocPtr(entry);
@@ -168,7 +169,7 @@ bool entryIsExpired(entry *entry) {
     return timestampIsExpired(entryGetExpiry(entry));
 }
 /**************************************** Entry Expiry API - End *****************************************/
-
+/* Frees the entry non-embedded value */
 static inline void entryFreeValuePtr(entry *entry) {
     serverAssert(entryHasValuePtr(entry));
     void **value_ref = entryGetValueRef(entry);

From 6913e21e2e65c506f5b1905f6ab32aa86fd8df85 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Fri, 5 Sep 2025 04:41:35 +0000
Subject: [PATCH 12/30] addressing code review comments

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/entry.c           | 84 +++++++++++++++++--------------------------
 src/entry.h           | 14 +-------
 src/module.c          |  2 +-
 src/server.h          | 11 +++++-
 src/t_hash.c          | 24 ++++++++-----
 src/unit/test_entry.c | 18 +++++-----
 src/unit/test_files.h |  4 +--
 7 files changed, 72 insertions(+), 85 deletions(-)

diff --git a/src/entry.c b/src/entry.c
index d46157c913..6ae987a2bf 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -109,7 +109,7 @@ char *entryGetValue(const entry *entry, size_t *len) {
     return *value_ref;
 }
 
-static void entrySetValuePtr(entry *e, sds value) {
+static void entrySetValueSds(entry *e, sds value) {
     serverAssert(entryHasValuePtr(e));
     void **value_ref = entryGetValueRef(e);
     if (entryHasStringRef(e)) {
@@ -121,14 +121,6 @@ static void entrySetValuePtr(entry *e, sds value) {
     }
     *value_ref = value;
 }
-/* Modify the value of this entry and return a pointer to the (potentially new) entry.
- * The value is taken by the function and cannot be reused after this function returns. */
-entry *entrySetValue(entry *e, sds value) {
-    if (entryHasEmbeddedValue(e)) return entryUpdate(e, value, entryGetExpiry(e));
-    entrySetValuePtr(e, value);
-    return e;
-}
-
 /* Returns the address of the entry allocation. */
 static void *entryGetAllocPtr(const entry *entry) {
     char *buf = sdsAllocPtr(entry);
@@ -161,6 +153,10 @@ entry *entrySetExpiry(entry *e, long long expiry) {
         *entryGetExpiryRef(e) = expiry;
         return e;
     }
+    if (entryHasStringRef(e)) {
+        stringRef *value = (stringRef *)*entryGetValueRef(e);
+        return entryUpdateAsStringRef(e, value->buf, value->len, expiry);
+    }
     return entryUpdate(e, NULL, expiry);
 }
 
@@ -242,18 +238,21 @@ static inline size_t entryReqSize(size_t field_len,
     return alloc_size;
 }
 
-/* Serialize the content of the entry into the provided buffer buf. Make use of the provided arguments provided by a call to entryReqSize.
+/* Serialize the content of the entry into an allocated buffer buf.
  * Note that this function will take ownership of the value so user should not assume it is valid after this call. */
-static entry *entryWrite(char *buf,
-                         size_t buf_size,
-                         const_sds field,
-                         sds value,
-                         long long expiry,
-                         bool embed_value,
-                         int embedded_field_sds_type,
-                         size_t embedded_field_sds_size,
-                         size_t embedded_value_sds_size,
-                         size_t expiry_size) {
+static entry *entryConstruct(size_t alloc_size,
+                             const_sds field,
+                             void *value,
+                             long long expiry,
+                             bool embed_value,
+                             int embedded_field_sds_type,
+                             size_t expiry_size,
+                             size_t embedded_value_sds_size,
+                             size_t embedded_field_sds_size) {
+    size_t buf_size;
+    /* allocate the buffer */
+    char *buf = zmalloc_usable(alloc_size, &buf_size);
+
     /* Set The expiry if exists */
     if (expiry_size) {
         *(long long *)buf = expiry;
@@ -262,9 +261,9 @@ static entry *entryWrite(char *buf,
     }
     if (value) {
         if (!embed_value) {
-            *(sds *)buf = value;
-            buf += sizeof(sds);
-            buf_size -= sizeof(sds);
+            *(void **)buf = value;
+            buf += sizeof(value);
+            buf_size -= sizeof(value);
         } else {
             sdswrite(buf + embedded_field_sds_size, buf_size - embedded_field_sds_size, SDS_TYPE_8, value, sdslen(value));
             sdsfree(value);
@@ -291,15 +290,11 @@ entry *entryCreate(const_sds field, sds value, long long expiry) {
     size_t expiry_size, embedded_value_sds_size, embedded_field_sds_size;
     size_t value_len = value ? sdslen(value) : SIZE_MAX;
     size_t alloc_size = entryReqSize(sdslen(field), value_len, expiry, &embed_value, &embedded_field_sds_type, &embedded_field_sds_size, &expiry_size, &embedded_value_sds_size);
-    size_t buf_size;
-
-    /* allocate the buffer */
-    char *buf = zmalloc_usable(alloc_size, &buf_size);
-
-    return entryWrite(buf, buf_size, field, value, expiry, embed_value, embedded_field_sds_type, embedded_field_sds_size, embedded_value_sds_size, expiry_size);
+    return entryConstruct(alloc_size, field, value, expiry, embed_value, embedded_field_sds_type, expiry_size, embedded_value_sds_size, embedded_field_sds_size);
 }
 
-entry *entrySetStringRef(entry *entry, const char *buf, size_t len, long long expiry) {
+
+entry *entryUpdateAsStringRef(entry *entry, const char *buf, size_t len, long long expiry) {
     long long entry_expiry = entryGetExpiry(entry);
     // Check for toggling expiration
     bool expiry_add_remove = (expiry != entry_expiry) && (entry_expiry == EXPIRY_NONE || expiry == EXPIRY_NONE);
@@ -326,20 +321,9 @@ entry *entrySetStringRef(entry *entry, const char *buf, size_t len, long long ex
     size_t alloc_size = field_size + sizeof(void *);
     alloc_size += (expiry == EXPIRY_NONE) ? 0 : sizeof(expiry);
 
-    size_t buf_size;
-    char *buf_sds = zmalloc_usable(alloc_size, &buf_size);
-
-    /* Set The expiry if exists */
-    if (expiry != EXPIRY_NONE) {
-        *(long long *)buf_sds = expiry;
-        buf_sds += sizeof(expiry);
-        buf_size -= sizeof(expiry);
-    }
-    *(stringRef **)buf_sds = value;
-    buf_sds += sizeof(value);
-    buf_size -= sizeof(value);
-    /* Set the field data */
-    sds new_entry = sdswrite(buf_sds, field_size, SDS_TYPE_8, field, sdslen(field));
+    size_t expiry_size = 0;
+    if (expiry != EXPIRY_NONE) expiry_size = sizeof(expiry);
+    sds new_entry = entryConstruct(alloc_size, field, value, expiry, false, SDS_TYPE_8, expiry_size, field_size, sizeof(value));
     entryFree(entry);
 
     sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
@@ -355,9 +339,10 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
     sds field = (sds)e;
     entry *new_entry = NULL;
 
+    /* Update just the expiry field, no value change, of a string ref entry */
     if (entryHasStringRef(e) && !value) {
         stringRef *value = (stringRef *)*entryGetValueRef(e);
-        return entrySetStringRef(e, value->buf, value->len, expiry);
+        return entryUpdateAsStringRef(e, value->buf, value->len, expiry);
     }
     bool update_value = value ? true : false;
     long long curr_expiration_time = entryGetExpiry(e);
@@ -410,7 +395,7 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
                 sdswrite(sdsAllocPtr(old_value), value_size, SDS_TYPE_8, value, sdslen(value));
                 sdsfree(value);
             } else {
-                entrySetValuePtr(e, value);
+                entrySetValueSds(e, value);
             }
         }
         new_entry = e;
@@ -428,9 +413,7 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
             }
         }
         /* allocate the buffer for a new entry */
-        size_t buf_size;
-        char *buf = zmalloc_usable(required_entry_size, &buf_size);
-        new_entry = entryWrite(buf, buf_size, field, value, expiry, embed_value, embedded_field_sds_type, embedded_field_size, embedded_value_size, expiry_size);
+        new_entry = entryConstruct(required_entry_size, field, value, expiry, embed_value, embedded_field_sds_type, expiry_size, embedded_value_size, embedded_field_size);
         entryFree(e);
     }
     /* Check that the new entry was built correctly */
@@ -465,8 +448,7 @@ size_t entryMemUsage(entry *entry) {
  * If the location of the entry changed we return the new location,
  * otherwise we return NULL. */
 entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) {
-    if (entryHasStringRef(entry)) return NULL;
-    if (entryHasValuePtr(entry)) {
+    if (entryHasValuePtr(entry) && !entryHasStringRef(entry)) {
         sds *value_ref = (sds *)entryGetValueRef(entry);
         sds new_value = sdsdefragfn(*value_ref);
         if (new_value) *value_ref = new_value;
diff --git a/src/entry.h b/src/entry.h
index b812046e42..1aa0281dbe 100644
--- a/src/entry.h
+++ b/src/entry.h
@@ -4,15 +4,6 @@
 #include "sds.h"
 #include <stdbool.h>
 
-/* Structure representing a non-owning view of a buffer.
- * The view does not manage the underlying memory, so its destruction
- * will not free the buffer.
- */
-typedef struct stringRef {
-    const char *buf; /* Pointer to the externalized buffer */
-    size_t len;      /* Length of the buffer */
-} stringRef;
-
 /*-----------------------------------------------------------------------------
  * Entry
  *----------------------------------------------------------------------------*/
@@ -62,9 +53,6 @@ sds entryGetField(const entry *entry);
 /* Returns the value string (sds) from the entry. */
 char *entryGetValue(const entry *entry, size_t *len);
 
-/* Sets or replaces the value string in the entry. May reallocate and return a new pointer. */
-entry *entrySetValue(entry *entry, sds value);
-
 /* Gets the expiration timestamp (UNIX time in milliseconds). */
 long long entryGetExpiry(const entry *entry);
 
@@ -85,7 +73,7 @@ void entryFree(entry *entry);
 
 /* Creates a new entry with the given field, value, and optional expiry. */
 entry *entryCreate(const_sds field, sds value, long long expiry);
-entry *entrySetStringRef(entry *entry, const char *buf, size_t len, long long expiry);
+entry *entryUpdateAsStringRef(entry *entry, const char *buf, size_t len, long long expiry);
 
 /* Updates the value and/or expiry of an existing entry.
  * In case value is NULL, will use the existing entry value.
diff --git a/src/module.c b/src/module.c
index ea1b5ccc01..d0786869df 100644
--- a/src/module.c
+++ b/src/module.c
@@ -5255,7 +5255,7 @@ int VM_ZsetRangePrev(ValkeyModuleKey *key) {
  * The function takes the hash key, hash field, and a buffer along with its length. */
 int VM_HashSetStringRef(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) {
     if (!key || !key->value || key->value->type != OBJ_HASH || !field || !buf) return VALKEYMODULE_ERR;
-    return hashTypeSetStringRef(key->value, field->ptr, buf, len);
+    return hashTypeUpdateAsStringRef(key->value, field->ptr, buf, len);
 }
 
 int VM_HashHasStringRef(ValkeyModuleKey *key, ValkeyModuleString *field) {
diff --git a/src/server.h b/src/server.h
index b294c7c00f..79f158a1b0 100644
--- a/src/server.h
+++ b/src/server.h
@@ -625,6 +625,15 @@ typedef enum { LOG_TIMESTAMP_LEGACY = 0,
 typedef enum { RDB_VERSION_CHECK_STRICT = 0,
                RDB_VERSION_CHECK_RELAXED } rdb_version_check_type;
 
+/* Structure representing a non-owning view of a buffer.
+ * A stringRef struct does not manage the underlying memory, so its destruction
+ * will not free the buffer.
+ */
+typedef struct stringRef {
+    const char *buf; /* Pointer to the externalized buffer */
+    size_t len;      /* Length of the buffer */
+} stringRef;
+
 /* common sets of actions to pause/unpause */
 #define PAUSE_ACTIONS_CLIENT_WRITE_SET \
     (PAUSE_ACTION_CLIENT_WRITE | PAUSE_ACTION_EXPIRE | PAUSE_ACTION_EVICT | PAUSE_ACTION_REPLICA)
@@ -3419,7 +3428,7 @@ robj *hashTypeGetValueObject(robj *o, sds field);
 int hashTypeSet(robj *o, sds field, sds value, long long expiry, int flags);
 robj *hashTypeDup(robj *o);
 bool hashTypeHasVolatileFields(robj *o);
-int hashTypeSetStringRef(robj *o, sds field, const char *buf, size_t len);
+int hashTypeUpdateAsStringRef(robj *o, sds field, const char *buf, size_t len);
 int hashTypeHasStringRef(robj *o, sds field);
 
 /* Pub / Sub */
diff --git a/src/t_hash.c b/src/t_hash.c
index 21083a901e..0d62641fac 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -317,15 +317,20 @@ int hashTypeHasStringRef(robj *o, sds field) {
     void **entry_ref = hashtableFindRef(ht, field);
     return (entryHasStringRef(*entry_ref));
 }
-/* Set a reference to the value.
- * Returns C_ERR on error, C_OK on update.
+/* Update a hash field value with a string reference value.
+ * Returns C_ERR if:
+ * 1. The hash field value not found.
+ * 2. The provided buffer doesn't match the hash field value.
+ * 3. The hash field value is already a string reference.
+ * Otherwise, returns C_OK.
  */
-int hashTypeSetStringRef(robj *o, sds field, const char *buf, size_t len) {
+int hashTypeUpdateAsStringRef(robj *o, sds field, const char *buf, size_t len) {
     unsigned char *vstr = NULL;
     unsigned int vlen = UINT_MAX;
     long long vll = LLONG_MAX;
 
     if (hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) != C_OK || !vstr) return C_ERR;
+    // For safety, the provided buffer must match the entry field value
     if (len != vlen || memcmp(buf, vstr, len) != 0) return C_ERR;
     // require HASHTABLE encoding due to aux bits and pointer storage.
     if (o->encoding == OBJ_ENCODING_LISTPACK) hashTypeConvert(o, OBJ_ENCODING_HASHTABLE);
@@ -335,7 +340,7 @@ int hashTypeSetStringRef(robj *o, sds field, const char *buf, size_t len) {
     entry *entry = *entry_ref;
     if (entryHasStringRef(entry)) return C_ERR;
     long long expiry = entryGetExpiry(entry);
-    void *new_entry = entrySetStringRef(entry, buf, len, expiry);
+    void *new_entry = entryUpdateAsStringRef(entry, buf, len, expiry);
     serverAssert(hashtableReplaceReallocatedEntry(ht, entry, new_entry));
     hashTypeTrackUpdateEntry(o, entry, new_entry, expiry, expiry);
     return C_OK;
@@ -551,7 +556,7 @@ static expiryModificationResult hashTypePersist(robj *o, sds field) {
         long long current_expire = entryGetExpiry(current_entry);
         if (current_expire != EXPIRY_NONE) {
             hashTypeUntrackEntry(o, current_entry);
-            *entry_ref = entryUpdate(current_entry, NULL, EXPIRY_NONE);
+            *entry_ref = entrySetExpiry(current_entry, EXPIRY_NONE);
             return EXPIRATION_MODIFICATION_SUCCESSFUL;
         }
         return EXPIRATION_MODIFICATION_FAILED; // If the found element has no expiration set, return -1
@@ -728,9 +733,12 @@ sds hashTypeCurrentObjectNewSds(hashTypeIterator *hi, int what) {
         if (vstr) return sdsnewlen(vstr, vlen);
         return sdsfromlonglong(vll);
     }
-    size_t vlen = 0;
-    vstr = (unsigned char *)hashTypeCurrentFromHashTable(hi, what, &vlen);
-    return sdsnewlen(vstr, vlen);
+    if (hi->encoding == OBJ_ENCODING_HASHTABLE) {
+	size_t vlen = 0;
+	vstr = (unsigned char *)hashTypeCurrentFromHashTable(hi, what, &vlen);
+	return sdsnewlen(vstr, vlen);
+    }
+    serverPanic("Unknown hash encoding");
 }
 
 robj *hashTypeLookupWriteOrCreate(client *c, robj *key) {
diff --git a/src/unit/test_entry.c b/src/unit/test_entry.c
index d70b0b6cdf..04a9b6f69b 100644
--- a/src/unit/test_entry.c
+++ b/src/unit/test_entry.c
@@ -358,7 +358,7 @@ int test_entryIsExpired(int argc, char **argv, int flags) {
  *      * To smaller value (should decrease memory usage)
  *      * To bigger value (should increase memory usage)
  */
-int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int flags) {
+int test_entryMemUsage_entrySetExpiry_entryUpdate(int argc, char **argv, int flags) {
     UNUSED(argc);
     UNUSED(argv);
     UNUSED(flags);
@@ -395,7 +395,7 @@ int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int f
     // Memory usage should decrease by the difference in value size (2 bytes)
     sds value4 = sdsnew("x");
     sds value_copy4 = sdsdup(value4);
-    entry *e4 = entrySetValue(e3, value4);
+    entry *e4 = entryUpdate(e3, value4, entryGetExpiry(e3));
     size_t e4_entryMemUsage = entryMemUsage(e4);
     verify_entry_properties(e4, field1, value_copy4, expiry3, true, false);
     TEST_ASSERT(zmalloc_usable_size((char *)e4 - sizeof(long long) - 3) == e4_entryMemUsage);
@@ -404,7 +404,7 @@ int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int f
     // Memory usage should increase by the difference in value size (1 byte)
     sds value5 = sdsnew("xx");
     sds value_copy5 = sdsdup(value5);
-    entry *e5 = entrySetValue(e4, value5);
+    entry *e5 = entryUpdate(e4, value5, entryGetExpiry(e4));
     size_t e5_entryMemUsage = entryMemUsage(e5);
     verify_entry_properties(e5, field1, value_copy5, expiry3, true, false);
     TEST_ASSERT(zmalloc_usable_size((char *)e5 - sizeof(long long) - 3) == e5_entryMemUsage);
@@ -442,7 +442,7 @@ int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int f
     // Memory usage should increase by at least the difference between LONG_VALUE and "x" (143)
     sds value9 = sdsnew("x");
     sds value_copy9 = sdsdup(value9);
-    entry *e9 = entrySetValue(e8, value9);
+    entry *e9 = entryUpdate(e8, value9, entryGetExpiry(e8));
     size_t e9_entryMemUsage = entryMemUsage(e9);
     verify_entry_properties(e9, field6, value_copy9, expiry8, true, true);
     size_t expected_e9_entry_mem = zmalloc_usable_size((char *)e9 - sizeof(long long) - sizeof(sds) - 3) + sdsAllocSize(value9);
@@ -452,7 +452,7 @@ int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int f
     // Memory usage increases by the difference in value size (1 byte)
     sds value10 = sdsnew("xx");
     sds value_copy10 = sdsdup(value10);
-    entry *e10 = entrySetValue(e9, value10);
+    entry *e10 = entryUpdate(e9, value10, entryGetExpiry(e9));
     size_t e10_entryMemUsage = entryMemUsage(e10);
     size_t expected_10_entry_mem = zmalloc_usable_size((char *)e10 - sizeof(long long) - sizeof(sds) - 3) + sdsAllocSize(value10);
     TEST_ASSERT(expected_10_entry_mem == e10_entryMemUsage);
@@ -480,18 +480,18 @@ int test_entryStringRef(int argc, char **argv, int flags) {
     sds value_copy1 = sdsdup(value1);
     long long expiry1 = EXPIRY_NONE;
     entry *e1 = entryCreate(field1, value1, expiry1);
-    entry *e2 = entrySetStringRef(e1, value_copy1, sdslen(value_copy1), entryGetExpiry(e1));
+    entry *e2 = entryUpdateAsStringRef(e1, value_copy1, sdslen(value_copy1), entryGetExpiry(e1));
     verify_entry_properties(e2, field1, value_copy1, expiry1, false, true);
     TEST_ASSERT(entryHasStringRef(e2) == true);
 
     long long expiry2 = 100;
-    entry *e3 = entrySetStringRef(e2, value_copy1, sdslen(value_copy1), expiry2);
+    entry *e3 = entryUpdateAsStringRef(e2, value_copy1, sdslen(value_copy1), expiry2);
     TEST_ASSERT(e2 != e3);
     verify_entry_properties(e3, field1, value_copy1, expiry2, true, true);
     TEST_ASSERT(entryHasStringRef(e3) == true);
 
     long long expiry3 = 200;
-    entry *e4 = entrySetStringRef(e3, value_copy1, sdslen(value_copy1), expiry3);
+    entry *e4 = entryUpdateAsStringRef(e3, value_copy1, sdslen(value_copy1), expiry3);
     TEST_ASSERT(e3 == e4);
     verify_entry_properties(e4, field1, value_copy1, expiry3, true, true);
     TEST_ASSERT(entryHasStringRef(e4) == true);
@@ -502,7 +502,7 @@ int test_entryStringRef(int argc, char **argv, int flags) {
     verify_entry_properties(e5, field1, value_copy2, expiry3, true, false);
     TEST_ASSERT(entryHasStringRef(e5) == false);
 
-    entry *e6 = entrySetStringRef(e5, value_copy1, sdslen(value_copy1), expiry2);
+    entry *e6 = entryUpdateAsStringRef(e5, value_copy1, sdslen(value_copy1), expiry2);
     TEST_ASSERT(e5 != e6);
     verify_entry_properties(e6, field1, value_copy1, expiry2, true, true);
     TEST_ASSERT(entryHasStringRef(e6) == true);
diff --git a/src/unit/test_files.h b/src/unit/test_files.h
index 6d5f06e9e1..a14e475906 100644
--- a/src/unit/test_files.h
+++ b/src/unit/test_files.h
@@ -24,7 +24,7 @@ int test_entryCreate(int argc, char **argv, int flags);
 int test_entryUpdate(int argc, char **argv, int flags);
 int test_entryHasexpiry_entrySetExpiry(int argc, char **argv, int flags);
 int test_entryIsExpired(int argc, char **argv, int flags);
-int test_entryMemUsage_entrySetExpiry_entrySetValue(int argc, char **argv, int flags);
+int test_entryMemUsage_entrySetExpiry_entryUpdate(int argc, char **argv, int flags);
 int test_entryStringRef(int argc, char **argv, int flags);
 int test_cursor(int argc, char **argv, int flags);
 int test_set_hash_function_seed(int argc, char **argv, int flags);
@@ -258,7 +258,7 @@ unitTest __test_crc64_c[] = {{"test_crc64", test_crc64}, {NULL, NULL}};
 unitTest __test_crc64combine_c[] = {{"test_crc64combine", test_crc64combine}, {NULL, NULL}};
 unitTest __test_dict_c[] = {{"test_dictCreate", test_dictCreate}, {"test_dictAdd16Keys", test_dictAdd16Keys}, {"test_dictDisableResize", test_dictDisableResize}, {"test_dictAddOneKeyTriggerResize", test_dictAddOneKeyTriggerResize}, {"test_dictDeleteKeys", test_dictDeleteKeys}, {"test_dictDeleteOneKeyTriggerResize", test_dictDeleteOneKeyTriggerResize}, {"test_dictEmptyDirAdd128Keys", test_dictEmptyDirAdd128Keys}, {"test_dictDisableResizeReduceTo3", test_dictDisableResizeReduceTo3}, {"test_dictDeleteOneKeyTriggerResizeAgain", test_dictDeleteOneKeyTriggerResizeAgain}, {"test_dictBenchmark", test_dictBenchmark}, {NULL, NULL}};
 unitTest __test_endianconv_c[] = {{"test_endianconv", test_endianconv}, {NULL, NULL}};
-unitTest __test_entry_c[] = {{"test_entryCreate", test_entryCreate}, {"test_entryUpdate", test_entryUpdate}, {"test_entryHasexpiry_entrySetExpiry", test_entryHasexpiry_entrySetExpiry}, {"test_entryIsExpired", test_entryIsExpired}, {"test_entryMemUsage_entrySetExpiry_entrySetValue", test_entryMemUsage_entrySetExpiry_entrySetValue}, {"test_entryStringRef", test_entryStringRef}, {NULL, NULL}};
+unitTest __test_entry_c[] = {{"test_entryCreate", test_entryCreate}, {"test_entryUpdate", test_entryUpdate}, {"test_entryHasexpiry_entrySetExpiry", test_entryHasexpiry_entrySetExpiry}, {"test_entryIsExpired", test_entryIsExpired}, {"test_entryMemUsage_entrySetExpiry_entryUpdate", test_entryMemUsage_entrySetExpiry_entryUpdate}, {"test_entryStringRef", test_entryStringRef}, {NULL, NULL}};
 unitTest __test_hashtable_c[] = {{"test_cursor", test_cursor}, {"test_set_hash_function_seed", test_set_hash_function_seed}, {"test_add_find_delete", test_add_find_delete}, {"test_add_find_delete_avoid_resize", test_add_find_delete_avoid_resize}, {"test_instant_rehashing", test_instant_rehashing}, {"test_bucket_chain_length", test_bucket_chain_length}, {"test_two_phase_insert_and_pop", test_two_phase_insert_and_pop}, {"test_replace_reallocated_entry", test_replace_reallocated_entry}, {"test_incremental_find", test_incremental_find}, {"test_scan", test_scan}, {"test_iterator", test_iterator}, {"test_safe_iterator", test_safe_iterator}, {"test_compact_bucket_chain", test_compact_bucket_chain}, {"test_random_entry", test_random_entry}, {"test_random_entry_with_long_chain", test_random_entry_with_long_chain}, {"test_random_entry_sparse_table", test_random_entry_sparse_table}, {NULL, NULL}};
 unitTest __test_intset_c[] = {{"test_intsetValueEncodings", test_intsetValueEncodings}, {"test_intsetBasicAdding", test_intsetBasicAdding}, {"test_intsetLargeNumberRandomAdd", test_intsetLargeNumberRandomAdd}, {"test_intsetUpgradeFromint16Toint32", test_intsetUpgradeFromint16Toint32}, {"test_intsetUpgradeFromint16Toint64", test_intsetUpgradeFromint16Toint64}, {"test_intsetUpgradeFromint32Toint64", test_intsetUpgradeFromint32Toint64}, {"test_intsetStressLookups", test_intsetStressLookups}, {"test_intsetStressAddDelete", test_intsetStressAddDelete}, {NULL, NULL}};
 unitTest __test_kvstore_c[] = {{"test_kvstoreAdd16Keys", test_kvstoreAdd16Keys}, {"test_kvstoreIteratorRemoveAllKeysNoDeleteEmptyHashtable", test_kvstoreIteratorRemoveAllKeysNoDeleteEmptyHashtable}, {"test_kvstoreIteratorRemoveAllKeysDeleteEmptyHashtable", test_kvstoreIteratorRemoveAllKeysDeleteEmptyHashtable}, {"test_kvstoreHashtableIteratorRemoveAllKeysNoDeleteEmptyHashtable", test_kvstoreHashtableIteratorRemoveAllKeysNoDeleteEmptyHashtable}, {"test_kvstoreHashtableIteratorRemoveAllKeysDeleteEmptyHashtable", test_kvstoreHashtableIteratorRemoveAllKeysDeleteEmptyHashtable}, {"test_kvstoreHashtableExpand", test_kvstoreHashtableExpand}, {NULL, NULL}};

From 82ef7bd3af7c70d2c860bd206d2d0d7a90cc489c Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Fri, 5 Sep 2025 14:28:25 +0000
Subject: [PATCH 13/30] addressing lint

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/t_hash.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/t_hash.c b/src/t_hash.c
index 0d62641fac..eeffb1ec4b 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -734,9 +734,9 @@ sds hashTypeCurrentObjectNewSds(hashTypeIterator *hi, int what) {
         return sdsfromlonglong(vll);
     }
     if (hi->encoding == OBJ_ENCODING_HASHTABLE) {
-	size_t vlen = 0;
-	vstr = (unsigned char *)hashTypeCurrentFromHashTable(hi, what, &vlen);
-	return sdsnewlen(vstr, vlen);
+        size_t vlen = 0;
+        vstr = (unsigned char *)hashTypeCurrentFromHashTable(hi, what, &vlen);
+        return sdsnewlen(vstr, vlen);
     }
     serverPanic("Unknown hash encoding");
 }

From 48193e8c90d48bccac9c7589c9de162fcb43afb8 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Fri, 5 Sep 2025 15:13:11 +0000
Subject: [PATCH 14/30] addressing unit test failure

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/entry.c           | 2 +-
 src/unit/test_entry.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/entry.c b/src/entry.c
index 6ae987a2bf..19d8dfef39 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -323,7 +323,7 @@ entry *entryUpdateAsStringRef(entry *entry, const char *buf, size_t len, long lo
 
     size_t expiry_size = 0;
     if (expiry != EXPIRY_NONE) expiry_size = sizeof(expiry);
-    sds new_entry = entryConstruct(alloc_size, field, value, expiry, false, SDS_TYPE_8, expiry_size, field_size, sizeof(value));
+    sds new_entry = entryConstruct(alloc_size, field, value, expiry, false, SDS_TYPE_8, expiry_size, sizeof(value), field_size);
     entryFree(entry);
 
     sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
diff --git a/src/unit/test_entry.c b/src/unit/test_entry.c
index 04a9b6f69b..bf673a786e 100644
--- a/src/unit/test_entry.c
+++ b/src/unit/test_entry.c
@@ -475,6 +475,7 @@ int test_entryStringRef(int argc, char **argv, int flags) {
     UNUSED(argc);
     UNUSED(argv);
     UNUSED(flags);
+
     sds field1 = sdsnew(SHORT_FIELD);
     sds value1 = sdsnew(SHORT_VALUE);
     sds value_copy1 = sdsdup(value1);

From 38aae4a481e0568d23d8fc15c741bb99b3f88129 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairgott@gmail.com>
Date: Tue, 9 Sep 2025 14:55:33 -0400
Subject: [PATCH 15/30] Update src/t_hash.c

Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: Yair Gottdenker <yairgott@gmail.com>
---
 src/t_hash.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/t_hash.c b/src/t_hash.c
index eeffb1ec4b..9dfb578da1 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -317,6 +317,7 @@ int hashTypeHasStringRef(robj *o, sds field) {
     void **entry_ref = hashtableFindRef(ht, field);
     return (entryHasStringRef(*entry_ref));
 }
+
 /* Update a hash field value with a string reference value.
  * Returns C_ERR if:
  * 1. The hash field value not found.

From 5f65be773886100ade5573de6197631efc9b44f7 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairgott@gmail.com>
Date: Tue, 9 Sep 2025 14:55:45 -0400
Subject: [PATCH 16/30] Update src/t_hash.c

Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: Yair Gottdenker <yairgott@gmail.com>
---
 src/t_hash.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/t_hash.c b/src/t_hash.c
index 9dfb578da1..ebc3379004 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -323,8 +323,7 @@ int hashTypeHasStringRef(robj *o, sds field) {
  * 1. The hash field value not found.
  * 2. The provided buffer doesn't match the hash field value.
  * 3. The hash field value is already a string reference.
- * Otherwise, returns C_OK.
- */
+ * Otherwise, returns C_OK. */
 int hashTypeUpdateAsStringRef(robj *o, sds field, const char *buf, size_t len) {
     unsigned char *vstr = NULL;
     unsigned int vlen = UINT_MAX;

From 8224ea6b8c867a5e18363e50063b89b7a1b78eed Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairgott@gmail.com>
Date: Tue, 9 Sep 2025 14:55:58 -0400
Subject: [PATCH 17/30] Update src/server.h

Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: Yair Gottdenker <yairgott@gmail.com>
---
 src/server.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/server.h b/src/server.h
index 79f158a1b0..db042b88b8 100644
--- a/src/server.h
+++ b/src/server.h
@@ -627,8 +627,7 @@ typedef enum { RDB_VERSION_CHECK_STRICT = 0,
 
 /* Structure representing a non-owning view of a buffer.
  * A stringRef struct does not manage the underlying memory, so its destruction
- * will not free the buffer.
- */
+ * will not free the buffer. */
 typedef struct stringRef {
     const char *buf; /* Pointer to the externalized buffer */
     size_t len;      /* Length of the buffer */

From ed86fa09ec0346dc2cde19d740fcb44c1a730cf8 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairgott@gmail.com>
Date: Tue, 9 Sep 2025 14:56:08 -0400
Subject: [PATCH 18/30] Update src/entry.c

Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: Yair Gottdenker <yairgott@gmail.com>
---
 src/entry.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/entry.c b/src/entry.c
index 19d8dfef39..286d9aad60 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -404,7 +404,7 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
         if (!update_value) {
             /* Check if the value can be reused. In case the original entry value is
              * embedded WE WILL HAVE TO DUPLICATE IT if not we have to duplicate it,
-             * remove it from the original entry since we are going to delete it.*/
+             * remove it from the original entry since we are going to delete it. */
             if (entryHasEmbeddedValue(e)) {
                 value = sdsdup(value);
             } else {

From 6d5a45eb616239ad156062f1db1f6453ef194344 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Tue, 9 Sep 2025 19:59:56 +0000
Subject: [PATCH 19/30] addressing comments

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/entry.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/entry.c b/src/entry.c
index 286d9aad60..92d58a1a8c 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -436,7 +436,7 @@ size_t entryMemUsage(entry *entry) {
          * header could be too small for holding the real allocation size. */
         mem += zmalloc_usable_size(entryGetAllocPtr(entry));
     }
-    if (!entryHasStringRef(entry)) mem += sdsAllocSize((sds)entryGetValue(entry, NULL));
+    mem += sdsAllocSize((sds)entryGetValue(entry, NULL));
     return mem;
 }
 
@@ -448,7 +448,11 @@ size_t entryMemUsage(entry *entry) {
  * If the location of the entry changed we return the new location,
  * otherwise we return NULL. */
 entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) {
-    if (entryHasValuePtr(entry) && !entryHasStringRef(entry)) {
+    if (entryHasStringRef(entry)) {
+        stringRef **value_ref = (stringRef **)entryGetValueRef(entry);
+        stringRef *new_value = activeDefragAlloc(*value_ref);
+        if (new_value) *value_ref = new_value;
+    } else if (entryHasValuePtr(entry)) {
         sds *value_ref = (sds *)entryGetValueRef(entry);
         sds new_value = sdsdefragfn(*value_ref);
         if (new_value) *value_ref = new_value;

From 2aea44d0f28676bf7a5d3d03fe2331c129a917a6 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Tue, 9 Sep 2025 20:27:18 +0000
Subject: [PATCH 20/30] addressing code review comments

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/t_hash.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/t_hash.c b/src/t_hash.c
index ebc3379004..c6c8f7107d 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -321,8 +321,7 @@ int hashTypeHasStringRef(robj *o, sds field) {
 /* Update a hash field value with a string reference value.
  * Returns C_ERR if:
  * 1. The hash field value not found.
- * 2. The provided buffer doesn't match the hash field value.
- * 3. The hash field value is already a string reference.
+ * 2. The hash field value is already a string reference.
  * Otherwise, returns C_OK. */
 int hashTypeUpdateAsStringRef(robj *o, sds field, const char *buf, size_t len) {
     unsigned char *vstr = NULL;
@@ -330,8 +329,6 @@ int hashTypeUpdateAsStringRef(robj *o, sds field, const char *buf, size_t len) {
     long long vll = LLONG_MAX;
 
     if (hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) != C_OK || !vstr) return C_ERR;
-    // For safety, the provided buffer must match the entry field value
-    if (len != vlen || memcmp(buf, vstr, len) != 0) return C_ERR;
     // require HASHTABLE encoding due to aux bits and pointer storage.
     if (o->encoding == OBJ_ENCODING_LISTPACK) hashTypeConvert(o, OBJ_ENCODING_HASHTABLE);
 

From 37a006c2bfc6a03d9c1f2cc83d8aaecce38c25d6 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Wed, 10 Sep 2025 20:14:49 +0000
Subject: [PATCH 21/30] fixing module test

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 tests/unit/moduleapi/hash_stringref.tcl | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/unit/moduleapi/hash_stringref.tcl b/tests/unit/moduleapi/hash_stringref.tcl
index 938865d4b8..a2efb23cc8 100644
--- a/tests/unit/moduleapi/hash_stringref.tcl
+++ b/tests/unit/moduleapi/hash_stringref.tcl
@@ -9,8 +9,6 @@ start_server {tags {"modules"}} {
         assert {$status == 1}
         r hset k f hello1
         assert_equal "0" [r hash.has_stringref k f]
-        set status [catch {r hash.set_stringref k f hello2} errmsg]
-        assert {$status == 1}
         r hash.set_stringref k f hello1
         assert_equal "hello1" [r hget k f]
         assert_equal "1" [r hash.has_stringref k f]

From b97667a182b2a7dfde033ba327ac2158d447e5fc Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Tue, 16 Sep 2025 19:28:31 +0000
Subject: [PATCH 22/30] addressing code review comments

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/entry.c  | 2 +-
 src/t_hash.c | 9 ++++-----
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/entry.c b/src/entry.c
index 92d58a1a8c..07bd6d6340 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -149,7 +149,7 @@ long long entryGetExpiry(const entry *entry) {
 
 /* Modify the expiration time of this entry and return a pointer to the (potentially new) entry. */
 entry *entrySetExpiry(entry *e, long long expiry) {
-    if (entryHasExpiry(e)) {
+    if (expiry != EXPIRY_NONE && entryHasExpiry(e)) {
         *entryGetExpiryRef(e) = expiry;
         return e;
     }
diff --git a/src/t_hash.c b/src/t_hash.c
index c6c8f7107d..0f071cc131 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -319,10 +319,9 @@ int hashTypeHasStringRef(robj *o, sds field) {
 }
 
 /* Update a hash field value with a string reference value.
- * Returns C_ERR if:
- * 1. The hash field value not found.
- * 2. The hash field value is already a string reference.
- * Otherwise, returns C_OK. */
+ * Returns C_ERR if the hash field value not found. Otherwise, returns C_OK.
+ * This function assumes, asserts, that the hash field value is not already
+ * a string reference. */
 int hashTypeUpdateAsStringRef(robj *o, sds field, const char *buf, size_t len) {
     unsigned char *vstr = NULL;
     unsigned int vlen = UINT_MAX;
@@ -335,7 +334,7 @@ int hashTypeUpdateAsStringRef(robj *o, sds field, const char *buf, size_t len) {
     hashtable *ht = o->ptr;
     void **entry_ref = hashtableFindRef(ht, field);
     entry *entry = *entry_ref;
-    if (entryHasStringRef(entry)) return C_ERR;
+    serverAssert(!entryHasStringRef(entry));
     long long expiry = entryGetExpiry(entry);
     void *new_entry = entryUpdateAsStringRef(entry, buf, len, expiry);
     serverAssert(hashtableReplaceReallocatedEntry(ht, entry, new_entry));

From bf6d7c7a36697f951f2c902d9c090dfca47f0f75 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Wed, 17 Sep 2025 15:34:05 +0000
Subject: [PATCH 23/30] addressing code review comments

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/t_hash.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/t_hash.c b/src/t_hash.c
index 0f071cc131..068c5beb13 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -319,22 +319,19 @@ int hashTypeHasStringRef(robj *o, sds field) {
 }
 
 /* Update a hash field value with a string reference value.
- * Returns C_ERR if the hash field value not found. Otherwise, returns C_OK.
- * This function assumes, asserts, that the hash field value is not already
- * a string reference. */
+ * Returns C_ERR if the hash field value not found. Otherwise, returns C_OK. */
 int hashTypeUpdateAsStringRef(robj *o, sds field, const char *buf, size_t len) {
     unsigned char *vstr = NULL;
     unsigned int vlen = UINT_MAX;
     long long vll = LLONG_MAX;
 
-    if (hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) != C_OK || !vstr) return C_ERR;
+    if (hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) != C_OK) return C_ERR;
     // require HASHTABLE encoding due to aux bits and pointer storage.
     if (o->encoding == OBJ_ENCODING_LISTPACK) hashTypeConvert(o, OBJ_ENCODING_HASHTABLE);
 
     hashtable *ht = o->ptr;
     void **entry_ref = hashtableFindRef(ht, field);
     entry *entry = *entry_ref;
-    serverAssert(!entryHasStringRef(entry));
     long long expiry = entryGetExpiry(entry);
     void *new_entry = entryUpdateAsStringRef(entry, buf, len, expiry);
     serverAssert(hashtableReplaceReallocatedEntry(ht, entry, new_entry));

From ce0e3a99cbc24ddb9a05ebe857dd0673da0b8254 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Wed, 19 Nov 2025 00:58:50 +0000
Subject: [PATCH 24/30] addressing review comments

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/entry.c  | 108 ++++++++++++++++++++++++++++++---------------------
 src/entry.h  |   3 ++
 src/module.c |  13 ++++++-
 src/server.h |   2 +-
 src/t_hash.c |   5 ++-
 5 files changed, 82 insertions(+), 49 deletions(-)

diff --git a/src/entry.c b/src/entry.c
index 07bd6d6340..d5315539ed 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -34,6 +34,17 @@
  *                        |
  *                        |
  *                        value pointer = value sds
+ *
+ * Entry with string reference, used when the value is not owned by entry. The field is SDS
+ * type 8 or higher.
+ *
+ *     +--------------+-----------+--------------+
+ *     | Expiration   | string    | field        |
+ *     | 1234567890LL | reference | hdr "foo" \0 |
+ *     +--------------+-----^-----+--------------+
+ *                          |
+ *                          |
+ *                          string reference = value is stringRef type.
  */
 
 enum {
@@ -82,13 +93,23 @@ sds entryGetField(const entry *entry) {
 
 /* Returns the location of a pointer to a separately allocated value. Only for
  * an entry without an embedded value. */
-static inline void **entryGetValueRef(const entry *entry) {
+static void **entryGetValueRef(const entry *entry) {
     serverAssert(entryHasValuePtr(entry));
     char *field_data = sdsAllocPtr(entry);
     field_data -= sizeof(void *);
     return (void **)field_data;
 }
 
+static sds *entryGetSdsValueRef(const entry *entry) {
+    return (sds *)entryGetValueRef(entry);    
+}
+
+static stringRef *entryGetStringRefRef(const entry *entry) {
+    serverAssert(entryHasStringRef(entry));
+    return (stringRef *)*entryGetValueRef(entry);    
+}
+
+
 /* Returns the entry's value. */
 char *entryGetValue(const entry *entry, size_t *len) {
     if (entryHasEmbeddedValue(entry)) {
@@ -98,27 +119,37 @@ char *entryGetValue(const entry *entry, size_t *len) {
         if (len) *len = sdslen(value);
         return value;
     }
-    void **value_ref = entryGetValueRef(entry);
-    if (!value_ref) return NULL;
     if (entryHasStringRef(entry)) {
-        stringRef *string_ref = (stringRef *)*value_ref;
+        stringRef *string_ref = entryGetStringRefRef(entry);
+        if (!string_ref) return NULL;
         if (len) *len = string_ref->len;
         return (char *)string_ref->buf;
     }
+    sds *value_ref = entryGetSdsValueRef(entry);
     if (len) *len = sdslen(*value_ref);
     return *value_ref;
 }
 
-static void entrySetValueSds(entry *e, sds value) {
-    serverAssert(entryHasValuePtr(e));
-    void **value_ref = entryGetValueRef(e);
-    if (entryHasStringRef(e)) {
+/* Frees the entry's non-embedded value.
+ * If the value is a string reference (stringRef), only the entry's pointer 
+ * is freed, as the underlying string is not owned by this entry.
+ * Otherwise, the value is a standard SDS and is fully freed. */
+static void entryFreeValuePtr(entry *entry) {
+    serverAssert(entryHasValuePtr(entry));
+    void **value_ref = entryGetValueRef(entry);
+    if (entryHasStringRef(entry)) {
         zfree(*value_ref);
-        *value_ref = (stringRef *)value;
-        sdsSetAuxBit(e, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 0);
     } else {
         sdsfree(*value_ref);
     }
+    *value_ref = NULL;
+}
+
+static void entrySetValueSds(entry *e, sds value) {
+    serverAssert(entryHasValuePtr(e));
+    entryFreeValuePtr(e);
+    if (entryHasStringRef(e)) sdsSetAuxBit(e, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 0);
+    sds *value_ref = entryGetSdsValueRef(e);
     *value_ref = value;
 }
 /* Returns the address of the entry allocation. */
@@ -131,12 +162,9 @@ static void *entryGetAllocPtr(const entry *entry) {
 
 /**************************************** Entry Expiry API *****************************************/
 /* Returns the location of a pointer to the expiry */
-static inline long long *entryGetExpiryRef(const entry *entry) {
+static long long *entryGetExpiryRef(const entry *entry) {
     debugServerAssert(entryHasExpiry(entry));
     char *buf = entryGetAllocPtr(entry);
-    debugServerAssert((((uintptr_t)buf & 0x7) == 0)); /* Test that the allocation is indeed 8 bytes aligned
-                                                       * This is needed since we access the expiry as with pointer casting
-                                                       * which require the access to be 8 bytes aligned. */
     return (long long *)buf;
 }
 
@@ -154,7 +182,7 @@ entry *entrySetExpiry(entry *e, long long expiry) {
         return e;
     }
     if (entryHasStringRef(e)) {
-        stringRef *value = (stringRef *)*entryGetValueRef(e);
+        stringRef *value = entryGetStringRefRef(e);
         return entryUpdateAsStringRef(e, value->buf, value->len, expiry);
     }
     return entryUpdate(e, NULL, expiry);
@@ -165,17 +193,6 @@ bool entryIsExpired(entry *entry) {
     return timestampIsExpired(entryGetExpiry(entry));
 }
 /**************************************** Entry Expiry API - End *****************************************/
-/* Frees the entry non-embedded value */
-static inline void entryFreeValuePtr(entry *entry) {
-    serverAssert(entryHasValuePtr(entry));
-    void **value_ref = entryGetValueRef(entry);
-    if (entryHasStringRef(entry)) {
-        zfree(*value_ref);
-        return;
-    }
-    sdsfree(*value_ref);
-}
-
 void entryFree(entry *entry) {
     if (entryHasValuePtr(entry)) entryFreeValuePtr(entry);
     zfree(entryGetAllocPtr(entry));
@@ -293,29 +310,34 @@ entry *entryCreate(const_sds field, sds value, long long expiry) {
     return entryConstruct(alloc_size, field, value, expiry, embed_value, embedded_field_sds_type, expiry_size, embedded_value_sds_size, embedded_field_sds_size);
 }
 
-
+/* Sets the entry's value to a string reference object. 
+ * The reference points to the provided `buf` but does not assume ownership. 
+ * It is assumed that an external mechanism will handle releasing any memory which
+ * may have been associated with value->buf */
 entry *entryUpdateAsStringRef(entry *entry, const char *buf, size_t len, long long expiry) {
     long long entry_expiry = entryGetExpiry(entry);
     // Check for toggling expiration
     bool expiry_add_remove = (expiry != entry_expiry) && (entry_expiry == EXPIRY_NONE || expiry == EXPIRY_NONE);
-    if (entryHasStringRef(entry) && !expiry_add_remove) {
-        stringRef *value = *entryGetValueRef(entry);
-        value->buf = buf;
-        value->len = len;
+    if (entryHasValuePtr(entry) && !expiry_add_remove) {
+        if (entryHasStringRef(entry)) {
+            stringRef *value = entryGetStringRefRef(entry);
+            value->buf = buf;
+            value->len = len;
+        } else {
+            stringRef *value = zmalloc(sizeof(stringRef));
+            value->buf = buf;
+            value->len = len;
+            sds *value_ref = entryGetSdsValueRef(entry);
+            sdsfree(*value_ref);
+            *value_ref = (sds)value;
+            sdsSetAuxBit(entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
+        }
         if (expiry != EXPIRY_NONE) *entryGetExpiryRef(entry) = expiry;
         return entry;
     }
     stringRef *value = zmalloc(sizeof(stringRef));
     value->buf = buf;
     value->len = len;
-    if (entryHasValuePtr(entry) && !expiry_add_remove) {
-        sds *value_ref = (sds *)entryGetValueRef(entry);
-        sdsfree(*value_ref);
-        *value_ref = (sds)value;
-        sdsSetAuxBit(entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
-        if (expiry != EXPIRY_NONE) *entryGetExpiryRef(entry) = expiry;
-        return entry;
-    }
     sds field = entryGetField(entry);
     size_t field_size = sdsReqSize(sdslen(field), SDS_TYPE_8);
     size_t alloc_size = field_size + sizeof(void *);
@@ -327,8 +349,6 @@ entry *entryUpdateAsStringRef(entry *entry, const char *buf, size_t len, long lo
     entryFree(entry);
 
     sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
-    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_VALUE_PTR, 1);
-    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_EXPIRY, expiry != EXPIRY_NONE);
     return new_entry;
 }
 /* Modify the entry's value and/or expiration time.
@@ -341,7 +361,7 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
 
     /* Update just the expiry field, no value change, of a string ref entry */
     if (entryHasStringRef(e) && !value) {
-        stringRef *value = (stringRef *)*entryGetValueRef(e);
+        stringRef *value = entryGetStringRefRef(e);
         return entryUpdateAsStringRef(e, value->buf, value->len, expiry);
     }
     bool update_value = value ? true : false;
@@ -436,7 +456,7 @@ size_t entryMemUsage(entry *entry) {
          * header could be too small for holding the real allocation size. */
         mem += zmalloc_usable_size(entryGetAllocPtr(entry));
     }
-    mem += sdsAllocSize((sds)entryGetValue(entry, NULL));
+    if (!entryHasStringRef(entry)) mem += sdsAllocSize((sds)entryGetValue(entry, NULL));
     return mem;
 }
 
@@ -450,7 +470,7 @@ size_t entryMemUsage(entry *entry) {
 entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) {
     if (entryHasStringRef(entry)) {
         stringRef **value_ref = (stringRef **)entryGetValueRef(entry);
-        stringRef *new_value = activeDefragAlloc(*value_ref);
+        stringRef *new_value = defragfn(*value_ref);
         if (new_value) *value_ref = new_value;
     } else if (entryHasValuePtr(entry)) {
         sds *value_ref = (sds *)entryGetValueRef(entry);
diff --git a/src/entry.h b/src/entry.h
index 1aa0281dbe..9a64d78aa5 100644
--- a/src/entry.h
+++ b/src/entry.h
@@ -73,6 +73,9 @@ void entryFree(entry *entry);
 
 /* Creates a new entry with the given field, value, and optional expiry. */
 entry *entryCreate(const_sds field, sds value, long long expiry);
+/* Sets the entry's value to a string reference object. 
+ * The reference points to the provided `buf` but does not assume ownership. 
+ * An external mechanism must handle the eventual memory deallocation of `buf`. */
 entry *entryUpdateAsStringRef(entry *entry, const char *buf, size_t len, long long expiry);
 
 /* Updates the value and/or expiry of an existing entry.
diff --git a/src/module.c b/src/module.c
index d0786869df..47d7a0ab05 100644
--- a/src/module.c
+++ b/src/module.c
@@ -5251,13 +5251,22 @@ int VM_ZsetRangePrev(ValkeyModuleKey *key) {
  * See also VM_ValueLength(), which returns the number of fields in a hash.
  * -------------------------------------------------------------------------- */
 
-/* Sets a reference to the value.
- * The function takes the hash key, hash field, and a buffer along with its length. */
+/* Sets the value of a hash field to a non-owning string reference (stringRef) 
+ * pointing to the buffer parameter, which remains owned by the module.
+ *
+ * NOTE: This API is designed for memory efficiency by avoiding memory duplication
+ * between the module and the core engine, which is critical when the buffer size is large. 
+ * For example, valkey-search uses this interface to avoid maintaining two copies of the
+ * indexed vectors. 
+ *
+ * The function receives the hash key, field name, buffer to share along with its size. */
 int VM_HashSetStringRef(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) {
     if (!key || !key->value || key->value->type != OBJ_HASH || !field || !buf) return VALKEYMODULE_ERR;
     return hashTypeUpdateAsStringRef(key->value, field->ptr, buf, len);
 }
 
+/* Checks if the value of a hash entry is a shared string reference (stringRef).
+ * The function receives the hash key and field name to perform the check against. */
 int VM_HashHasStringRef(ValkeyModuleKey *key, ValkeyModuleString *field) {
     if (!key || !key->value || key->value->type != OBJ_HASH) return VALKEYMODULE_ERR;
     return hashTypeHasStringRef(key->value, field->ptr);
diff --git a/src/server.h b/src/server.h
index db042b88b8..5b82fd9955 100644
--- a/src/server.h
+++ b/src/server.h
@@ -3428,7 +3428,7 @@ int hashTypeSet(robj *o, sds field, sds value, long long expiry, int flags);
 robj *hashTypeDup(robj *o);
 bool hashTypeHasVolatileFields(robj *o);
 int hashTypeUpdateAsStringRef(robj *o, sds field, const char *buf, size_t len);
-int hashTypeHasStringRef(robj *o, sds field);
+bool hashTypeHasStringRef(robj *o, sds field);
 
 /* Pub / Sub */
 int pubsubUnsubscribeAllChannels(client *c, int notify);
diff --git a/src/t_hash.c b/src/t_hash.c
index 068c5beb13..feaedf639f 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -311,7 +311,7 @@ int hashTypeExists(robj *o, sds field) {
     return hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) == C_OK;
 }
 
-int hashTypeHasStringRef(robj *o, sds field) {
+bool hashTypeHasStringRef(robj *o, sds field) {
     if (o->encoding == OBJ_ENCODING_LISTPACK) return 0;
     hashtable *ht = o->ptr;
     void **entry_ref = hashtableFindRef(ht, field);
@@ -334,7 +334,8 @@ int hashTypeUpdateAsStringRef(robj *o, sds field, const char *buf, size_t len) {
     entry *entry = *entry_ref;
     long long expiry = entryGetExpiry(entry);
     void *new_entry = entryUpdateAsStringRef(entry, buf, len, expiry);
-    serverAssert(hashtableReplaceReallocatedEntry(ht, entry, new_entry));
+    bool replaced = hashtableReplaceReallocatedEntry(ht, entry, new_entry);
+    serverAssert(replaced);
     hashTypeTrackUpdateEntry(o, entry, new_entry, expiry, expiry);
     return C_OK;
 }

From 96a4530238239f40cfe7243faa7046d3a208843e Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairgott@gmail.com>
Date: Wed, 19 Nov 2025 10:59:03 -0500
Subject: [PATCH 25/30] Update src/entry.c

Co-authored-by: Ran Shidlansik <ranshid@amazon.com>
Signed-off-by: Yair Gottdenker <yairgott@gmail.com>
---
 src/entry.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/entry.c b/src/entry.c
index d5315539ed..b8447e1989 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -109,7 +109,6 @@ static stringRef *entryGetStringRefRef(const entry *entry) {
     return (stringRef *)*entryGetValueRef(entry);    
 }
 
-
 /* Returns the entry's value. */
 char *entryGetValue(const entry *entry, size_t *len) {
     if (entryHasEmbeddedValue(entry)) {

From a0f541da47688261a262d3ae2e1067df08f144ba Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Thu, 20 Nov 2025 01:04:28 +0000
Subject: [PATCH 26/30] addressing review comments

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/anet.c                     |  4 ++--
 src/defrag.c                   |  2 +-
 src/entry.c                    | 38 +++++++++++++++++-----------------
 src/entry.h                    |  4 ++--
 src/module.c                   |  6 +++---
 src/object.c                   |  4 ++--
 src/rdb.h                      |  2 +-
 src/server.c                   |  6 +++---
 src/vset.c                     |  2 +-
 tests/modules/hash_stringref.c | 24 ++++++++++++++++++++-
 10 files changed, 57 insertions(+), 35 deletions(-)

diff --git a/src/anet.c b/src/anet.c
index 5524e9cf4c..65af9a10d2 100644
--- a/src/anet.c
+++ b/src/anet.c
@@ -201,8 +201,8 @@ int anetKeepAlive(char *err, int fd, int interval) {
     if (idle < 10) idle = 10;                               // kernel expects at least 10 seconds
     if (idle > 10 * 24 * 60 * 60) idle = 10 * 24 * 60 * 60; // kernel expects at most 10 days
 
-        /* `TCP_KEEPIDLE`, `TCP_KEEPINTVL`, and `TCP_KEEPCNT` were not available on Solaris
-         * until version 11.4, but let's take a chance here. */
+    /* `TCP_KEEPIDLE`, `TCP_KEEPINTVL`, and `TCP_KEEPCNT` were not available on Solaris
+     * until version 11.4, but let's take a chance here. */
 #if defined(TCP_KEEPIDLE) && defined(TCP_KEEPINTVL) && defined(TCP_KEEPCNT)
     if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &idle, sizeof(idle))) {
         anetSetError(err, "setsockopt TCP_KEEPIDLE: %s\n", strerror(errno));
diff --git a/src/defrag.c b/src/defrag.c
index 2acb18a12f..6a5bbd8e42 100644
--- a/src/defrag.c
+++ b/src/defrag.c
@@ -576,7 +576,7 @@ static int scanLaterStreamListpacks(robj *ob, unsigned long *cursor, monotime en
 }
 
 /* optional callback used defrag each rax element (not including the element pointer itself) */
-typedef void *(raxDefragFunction)(raxIterator *ri, void *privdata);
+typedef void *(raxDefragFunction)(raxIterator * ri, void *privdata);
 
 /* defrag radix tree including:
  * 1) rax struct
diff --git a/src/entry.c b/src/entry.c
index b8447e1989..538cd88d6b 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -101,12 +101,12 @@ static void **entryGetValueRef(const entry *entry) {
 }
 
 static sds *entryGetSdsValueRef(const entry *entry) {
-    return (sds *)entryGetValueRef(entry);    
+    return (sds *)entryGetValueRef(entry);
 }
 
 static stringRef *entryGetStringRefRef(const entry *entry) {
     serverAssert(entryHasStringRef(entry));
-    return (stringRef *)*entryGetValueRef(entry);    
+    return (stringRef *)*entryGetValueRef(entry);
 }
 
 /* Returns the entry's value. */
@@ -130,7 +130,7 @@ char *entryGetValue(const entry *entry, size_t *len) {
 }
 
 /* Frees the entry's non-embedded value.
- * If the value is a string reference (stringRef), only the entry's pointer 
+ * If the value is a string reference (stringRef), only the entry's pointer
  * is freed, as the underlying string is not owned by this entry.
  * Otherwise, the value is a standard SDS and is fully freed. */
 static void entryFreeValuePtr(entry *entry) {
@@ -258,13 +258,15 @@ static inline size_t entryReqSize(size_t field_len,
  * Note that this function will take ownership of the value so user should not assume it is valid after this call. */
 static entry *entryConstruct(size_t alloc_size,
                              const_sds field,
-                             void *value,
+                             sds sds_value,
+                             stringRef *stringref_value,
                              long long expiry,
                              bool embed_value,
                              int embedded_field_sds_type,
                              size_t expiry_size,
                              size_t embedded_value_sds_size,
                              size_t embedded_field_sds_size) {
+    serverAssert((sds_value == NULL && stringref_value == NULL && embed_value) || (sds_value != NULL && stringref_value == NULL) || (sds_value == NULL && stringref_value != NULL && !embed_value));
     size_t buf_size;
     /* allocate the buffer */
     char *buf = zmalloc_usable(alloc_size, &buf_size);
@@ -275,16 +277,14 @@ static entry *entryConstruct(size_t alloc_size,
         buf += expiry_size;
         buf_size -= expiry_size;
     }
-    if (value) {
-        if (!embed_value) {
-            *(void **)buf = value;
-            buf += sizeof(value);
-            buf_size -= sizeof(value);
-        } else {
-            sdswrite(buf + embedded_field_sds_size, buf_size - embedded_field_sds_size, SDS_TYPE_8, value, sdslen(value));
-            sdsfree(value);
-            buf_size -= embedded_value_sds_size;
-        }
+    if (!embed_value) {
+        *(void **)buf = sds_value ? (void *)sds_value : (void *)stringref_value;
+        buf += sizeof(void *);
+        buf_size -= sizeof(void *);
+    } else if (sds_value) {
+        sdswrite(buf + embedded_field_sds_size, buf_size - embedded_field_sds_size, SDS_TYPE_8, sds_value, sdslen(sds_value));
+        sdsfree(sds_value);
+        buf_size -= embedded_value_sds_size;
     }
     /* Set the field data */
     entry *new_entry = sdswrite(buf, embedded_field_sds_size, embedded_field_sds_type, field, sdslen(field));
@@ -306,11 +306,11 @@ entry *entryCreate(const_sds field, sds value, long long expiry) {
     size_t expiry_size, embedded_value_sds_size, embedded_field_sds_size;
     size_t value_len = value ? sdslen(value) : SIZE_MAX;
     size_t alloc_size = entryReqSize(sdslen(field), value_len, expiry, &embed_value, &embedded_field_sds_type, &embedded_field_sds_size, &expiry_size, &embedded_value_sds_size);
-    return entryConstruct(alloc_size, field, value, expiry, embed_value, embedded_field_sds_type, expiry_size, embedded_value_sds_size, embedded_field_sds_size);
+    return entryConstruct(alloc_size, field, value, NULL, expiry, embed_value, embedded_field_sds_type, expiry_size, embedded_value_sds_size, embedded_field_sds_size);
 }
 
-/* Sets the entry's value to a string reference object. 
- * The reference points to the provided `buf` but does not assume ownership. 
+/* Sets the entry's value to a string reference object.
+ * The reference points to the provided `buf` but does not assume ownership.
  * It is assumed that an external mechanism will handle releasing any memory which
  * may have been associated with value->buf */
 entry *entryUpdateAsStringRef(entry *entry, const char *buf, size_t len, long long expiry) {
@@ -344,7 +344,7 @@ entry *entryUpdateAsStringRef(entry *entry, const char *buf, size_t len, long lo
 
     size_t expiry_size = 0;
     if (expiry != EXPIRY_NONE) expiry_size = sizeof(expiry);
-    sds new_entry = entryConstruct(alloc_size, field, value, expiry, false, SDS_TYPE_8, expiry_size, sizeof(value), field_size);
+    sds new_entry = entryConstruct(alloc_size, field, NULL, value, expiry, false, SDS_TYPE_8, expiry_size, sizeof(value), field_size);
     entryFree(entry);
 
     sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
@@ -432,7 +432,7 @@ entry *entryUpdate(entry *e, sds value, long long expiry) {
             }
         }
         /* allocate the buffer for a new entry */
-        new_entry = entryConstruct(required_entry_size, field, value, expiry, embed_value, embedded_field_sds_type, expiry_size, embedded_value_size, embedded_field_size);
+        new_entry = entryConstruct(required_entry_size, field, value, NULL, expiry, embed_value, embedded_field_sds_type, expiry_size, embedded_value_size, embedded_field_size);
         entryFree(e);
     }
     /* Check that the new entry was built correctly */
diff --git a/src/entry.h b/src/entry.h
index 9a64d78aa5..c43fa90fa6 100644
--- a/src/entry.h
+++ b/src/entry.h
@@ -73,8 +73,8 @@ void entryFree(entry *entry);
 
 /* Creates a new entry with the given field, value, and optional expiry. */
 entry *entryCreate(const_sds field, sds value, long long expiry);
-/* Sets the entry's value to a string reference object. 
- * The reference points to the provided `buf` but does not assume ownership. 
+/* Sets the entry's value to a string reference object.
+ * The reference points to the provided `buf` but does not assume ownership.
  * An external mechanism must handle the eventual memory deallocation of `buf`. */
 entry *entryUpdateAsStringRef(entry *entry, const char *buf, size_t len, long long expiry);
 
diff --git a/src/module.c b/src/module.c
index 47d7a0ab05..93fcc2f0c2 100644
--- a/src/module.c
+++ b/src/module.c
@@ -5251,13 +5251,13 @@ int VM_ZsetRangePrev(ValkeyModuleKey *key) {
  * See also VM_ValueLength(), which returns the number of fields in a hash.
  * -------------------------------------------------------------------------- */
 
-/* Sets the value of a hash field to a non-owning string reference (stringRef) 
+/* Sets the value of a hash field to a non-owning string reference (stringRef)
  * pointing to the buffer parameter, which remains owned by the module.
  *
  * NOTE: This API is designed for memory efficiency by avoiding memory duplication
- * between the module and the core engine, which is critical when the buffer size is large. 
+ * between the module and the core engine, which is critical when the buffer size is large.
  * For example, valkey-search uses this interface to avoid maintaining two copies of the
- * indexed vectors. 
+ * indexed vectors.
  *
  * The function receives the hash key, field name, buffer to share along with its size. */
 int VM_HashSetStringRef(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) {
diff --git a/src/object.c b/src/object.c
index 33fd4c752f..b6fd846f14 100644
--- a/src/object.c
+++ b/src/object.c
@@ -742,8 +742,8 @@ void dismissObject(robj *o, size_t size_hint) {
     /* madvise(MADV_DONTNEED) may not work if Transparent Huge Pages is enabled. */
     if (server.thp_enabled) return;
 
-        /* Currently we use zmadvise_dontneed only when we use jemalloc with Linux.
-         * so we avoid these pointless loops when they're not going to do anything. */
+    /* Currently we use zmadvise_dontneed only when we use jemalloc with Linux.
+     * so we avoid these pointless loops when they're not going to do anything. */
 #if defined(USE_JEMALLOC) && defined(__linux__)
     if (o->refcount != 1) return;
     switch (o->type) {
diff --git a/src/rdb.h b/src/rdb.h
index 56bb74e9f4..be0feebd29 100644
--- a/src/rdb.h
+++ b/src/rdb.h
@@ -54,7 +54,7 @@
 /* Reserved range for foreign (unsupported, non-OSS) RDB format. */
 #define RDB_FOREIGN_VERSION_MIN 12
 #define RDB_FOREIGN_VERSION_MAX 79
-static_assert(RDB_VERSION < RDB_FOREIGN_VERSION_MIN || RDB_VERSION > RDB_FOREIGN_VERSION_MAX,
+static_assert(RDB_VERSION<RDB_FOREIGN_VERSION_MIN || RDB_VERSION> RDB_FOREIGN_VERSION_MAX,
               "RDB version in foreign version range");
 
 /* Defines related to the dump file format. To store 32 bits lengths for short
diff --git a/src/server.c b/src/server.c
index 91b4513c5c..3f4623f90e 100644
--- a/src/server.c
+++ b/src/server.c
@@ -6813,8 +6813,8 @@ void dismissMemoryInChild(void) {
     /* madvise(MADV_DONTNEED) may not work if Transparent Huge Pages is enabled. */
     if (server.thp_enabled) return;
 
-        /* Currently we use zmadvise_dontneed only when we use jemalloc with Linux.
-         * so we avoid these pointless loops when they're not going to do anything. */
+    /* Currently we use zmadvise_dontneed only when we use jemalloc with Linux.
+     * so we avoid these pointless loops when they're not going to do anything. */
 #if defined(USE_JEMALLOC) && defined(__linux__)
     listIter li;
     listNode *ln;
@@ -7259,7 +7259,7 @@ __attribute__((weak)) int main(int argc, char **argv) {
     }
     if (server.sentinel_mode) sentinelCheckConfigFile();
 
-        /* Do system checks */
+    /* Do system checks */
 #ifdef __linux__
     linuxMemoryWarnings();
     sds err_msg = NULL;
diff --git a/src/vset.c b/src/vset.c
index ec49d1fa0c..68fc966c7f 100644
--- a/src/vset.c
+++ b/src/vset.c
@@ -675,7 +675,7 @@ void pvSort(pVector *pv, int (*compare)(const void *a, const void *b)) {
 
 #define VOLATILESET_VECTOR_BUCKET_MAX_SIZE 127
 
-#define VSET_NONE_BUCKET_PTR ((void *)(uintptr_t) - 1)
+#define VSET_NONE_BUCKET_PTR ((void *)(uintptr_t)-1)
 #define VSET_BUCKET_NONE -1      // matching the NULL case
 #define VSET_BUCKET_SINGLE 0x1UL // xx1 (assuming sds)
 #define VSET_BUCKET_VECTOR 0x2UL // 010
diff --git a/tests/modules/hash_stringref.c b/tests/modules/hash_stringref.c
index 54cf5701ae..f6de1b325c 100644
--- a/tests/modules/hash_stringref.c
+++ b/tests/modules/hash_stringref.c
@@ -1,3 +1,5 @@
+/* Module Test: Verifies the module's capability to share an owned buffer with the core, 
+ * which is then stored in a hash key field using a non-owning string reference (stringRef). */
 #include "valkeymodule.h"
 #include <string.h>
 
@@ -31,6 +33,18 @@ void freeBufferList(void) {
     }
 }
 
+/* HASH.HAS_STRINGREF key field
+ *
+ * Returns 1 if all of the following conditions are met for the hash field:
+ * 1. The key exists.
+ * 2. The key's value is a HASH type.
+ * 3. The field's value is a string reference (stringRef) type.
+ * Otherwise, returns 0.
+ *
+ * Parameters:
+ * 1. The hash entry key.
+ * 2. The hahs entry field.
+ */
 int hashHasStringRef(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {
     if (argc != 3) return ValkeyModule_WrongArity(ctx);
 
@@ -41,6 +55,14 @@ int hashHasStringRef(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc)
     return ValkeyModule_ReplyWithLongLong(ctx, result);
 }
 
+/* HASH.SET_STRINGREF key field buffer
+ *
+ * Sets hash entry value of a given key and field to an external owned buffer.
+ * Parameters:
+ * 1. The hash entry key.
+ * 2. The hahs entry field.
+ * 3. The buffer to share with the core.
+ */
 int hashSetStringRef(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int argc) {
     if (argc != 4) return ValkeyModule_WrongArity(ctx);
 
@@ -63,7 +85,7 @@ int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int arg
         VALKEYMODULE_OK &&
         ValkeyModule_CreateCommand(ctx, "hash.set_stringref", hashSetStringRef, "write",
                                   1, 1, 1) == VALKEYMODULE_OK &&
-        ValkeyModule_CreateCommand(ctx, "hash.has_stringref", hashHasStringRef, "write",
+        ValkeyModule_CreateCommand(ctx, "hash.has_stringref", hashHasStringRef, "read",
                                   1, 1, 1) == VALKEYMODULE_OK) {
         return VALKEYMODULE_OK;
     }

From 2569ea234074a22e5ca2d691eddf94caec16a0ab Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Thu, 20 Nov 2025 15:58:11 +0000
Subject: [PATCH 27/30] fixing rebase issues

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/entry.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/entry.c b/src/entry.c
index d682acd6a3..94cbdd7b97 100644
--- a/src/entry.c
+++ b/src/entry.c
@@ -110,7 +110,7 @@ bool entryHasEmbeddedValue(const entry *entry) {
 /* Returns true in case the entry holds a reference of the value.
  * Returns false otherwise. */
 bool entryHasStringRef(const entry *entry) {
-    return entryHasValuePtr(entry) && sdsGetAuxBit(entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF);
+    return entryHasValuePtr(entry) && sdsGetAuxBit(entryGetField(entry), FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF);
 }
 /* Returns true in case the entry has expiration timestamp.
  * Returns false otherwise. */
@@ -174,7 +174,7 @@ static void entryFreeValuePtr(entry *entry) {
 static void entrySetValueSds(entry *e, sds value) {
     serverAssert(entryHasValuePtr(e));
     entryFreeValuePtr(e);
-    if (entryHasStringRef(e)) sdsSetAuxBit(e, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 0);
+    if (entryHasStringRef(e)) sdsSetAuxBit(entryGetField(e), FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 0);
     sds *value_ref = entryGetSdsValueRef(e);
     *value_ref = value;
 }
@@ -341,41 +341,41 @@ entry *entryCreate(const_sds field, sds value, long long expiry) {
  * The reference points to the provided `buf` but does not assume ownership.
  * It is assumed that an external mechanism will handle releasing any memory which
  * may have been associated with value->buf */
-entry *entryUpdateAsStringRef(entry *entry, const char *buf, size_t len, long long expiry) {
-    long long entry_expiry = entryGetExpiry(entry);
+entry *entryUpdateAsStringRef(entry *e, const char *buf, size_t len, long long expiry) {
+    long long entry_expiry = entryGetExpiry(e);
     // Check for toggling expiration
     bool expiry_add_remove = (expiry != entry_expiry) && (entry_expiry == EXPIRY_NONE || expiry == EXPIRY_NONE);
-    if (entryHasValuePtr(entry) && !expiry_add_remove) {
-        if (entryHasStringRef(entry)) {
-            stringRef *value = entryGetStringRefRef(entry);
+    if (entryHasValuePtr(e) && !expiry_add_remove) {
+        if (entryHasStringRef(e)) {
+            stringRef *value = entryGetStringRefRef(e);
             value->buf = buf;
             value->len = len;
         } else {
             stringRef *value = zmalloc(sizeof(stringRef));
             value->buf = buf;
             value->len = len;
-            sds *value_ref = entryGetSdsValueRef(entry);
+            sds *value_ref = entryGetSdsValueRef(e);
             sdsfree(*value_ref);
             *value_ref = (sds)value;
-            sdsSetAuxBit(entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
+            sdsSetAuxBit(entryGetField(e), FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
         }
-        if (expiry != EXPIRY_NONE) *entryGetExpiryRef(entry) = expiry;
-        return entry;
+        if (expiry != EXPIRY_NONE) *entryGetExpiryRef(e) = expiry;
+        return e;
     }
     stringRef *value = zmalloc(sizeof(stringRef));
     value->buf = buf;
     value->len = len;
-    sds field = entryGetField(entry);
+    sds field = entryGetField(e);
     size_t field_size = sdsReqSize(sdslen(field), SDS_TYPE_8);
     size_t alloc_size = field_size + sizeof(void *);
     alloc_size += (expiry == EXPIRY_NONE) ? 0 : sizeof(expiry);
 
     size_t expiry_size = 0;
     if (expiry != EXPIRY_NONE) expiry_size = sizeof(expiry);
-    sds new_entry = entryConstruct(alloc_size, field, NULL, value, expiry, false, SDS_TYPE_8, expiry_size, sizeof(value), field_size);
-    entryFree(entry);
+    entry *new_entry = entryConstruct(alloc_size, field, NULL, value, expiry, false, SDS_TYPE_8, expiry_size, sizeof(value), field_size);
+    entryFree(e);
 
-    sdsSetAuxBit(new_entry, FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
+    sdsSetAuxBit(entryGetField(new_entry), FIELD_SDS_AUX_BIT_ENTRY_HAS_STRING_REF, 1);
     return new_entry;
 }
 /* Modify the entry's value and/or expiration time.
@@ -494,13 +494,13 @@ size_t entryMemUsage(entry *entry) {
  * of sds strings.
  * If the location of the entry changed we return the new location,
  * otherwise we return NULL. */
-entry *entryDefrag(entry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) {
-    if (entryHasStringRef(entry)) {
-        stringRef **value_ref = (stringRef **)entryGetValueRef(entry);
+entry *entryDefrag(entry *e, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) {
+    if (entryHasStringRef(e)) {
+        stringRef **value_ref = (stringRef **)entryGetValueRef(e);
         stringRef *new_value = defragfn(*value_ref);
         if (new_value) *value_ref = new_value;
-    } else if (entryHasValuePtr(entry)) {
-        sds *value_ref = (sds *)entryGetValueRef(entry);
+    } else if (entryHasValuePtr(e)) {
+        sds *value_ref = (sds *)entryGetValueRef(e);
         sds new_value = sdsdefragfn(*value_ref);
         if (new_value) *value_ref = new_value;
     }

From 2423aa60d0b58b9c2478efe180813a39805f42b2 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Thu, 20 Nov 2025 16:14:11 +0000
Subject: [PATCH 28/30] lint fixes

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/anet.c   | 4 ++--
 src/defrag.c | 2 +-
 src/object.c | 4 ++--
 src/rdb.h    | 2 +-
 src/server.c | 4 ++--
 src/vset.c   | 4 ++--
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/anet.c b/src/anet.c
index 65af9a10d2..3be052a6e7 100644
--- a/src/anet.c
+++ b/src/anet.c
@@ -201,8 +201,8 @@ int anetKeepAlive(char *err, int fd, int interval) {
     if (idle < 10) idle = 10;                               // kernel expects at least 10 seconds
     if (idle > 10 * 24 * 60 * 60) idle = 10 * 24 * 60 * 60; // kernel expects at most 10 days
 
-    /* `TCP_KEEPIDLE`, `TCP_KEEPINTVL`, and `TCP_KEEPCNT` were not available on Solaris
-     * until version 11.4, but let's take a chance here. */
+       /* `TCP_KEEPIDLE`, `TCP_KEEPINTVL`, and `TCP_KEEPCNT` were not available on Solaris
+         * until version 11.4, but let's take a chance here. */
 #if defined(TCP_KEEPIDLE) && defined(TCP_KEEPINTVL) && defined(TCP_KEEPCNT)
     if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &idle, sizeof(idle))) {
         anetSetError(err, "setsockopt TCP_KEEPIDLE: %s\n", strerror(errno));
diff --git a/src/defrag.c b/src/defrag.c
index bd55572c62..f4cb5f0b2d 100644
--- a/src/defrag.c
+++ b/src/defrag.c
@@ -570,7 +570,7 @@ static int scanLaterStreamListpacks(robj *ob, unsigned long *cursor, monotime en
 }
 
 /* optional callback used defrag each rax element (not including the element pointer itself) */
-typedef void *(raxDefragFunction)(raxIterator * ri, void *privdata);
+typedef void *(raxDefragFunction)(raxIterator *ri, void *privdata);
 
 /* defrag radix tree including:
  * 1) rax struct
diff --git a/src/object.c b/src/object.c
index 46b4314f56..13efeb5023 100644
--- a/src/object.c
+++ b/src/object.c
@@ -743,8 +743,8 @@ void dismissObject(robj *o, size_t size_hint) {
     /* madvise(MADV_DONTNEED) may not work if Transparent Huge Pages is enabled. */
     if (server.thp_enabled) return;
 
-    /* Currently we use zmadvise_dontneed only when we use jemalloc with Linux.
-     * so we avoid these pointless loops when they're not going to do anything. */
+        /* Currently we use zmadvise_dontneed only when we use jemalloc with Linux.
+         * so we avoid these pointless loops when they're not going to do anything. */
 #if defined(USE_JEMALLOC) && defined(__linux__)
     if (o->refcount != 1) return;
     switch (o->type) {
diff --git a/src/rdb.h b/src/rdb.h
index 7f2e1a17b2..2aed096cc5 100644
--- a/src/rdb.h
+++ b/src/rdb.h
@@ -54,7 +54,7 @@
 /* Reserved range for foreign (unsupported, non-OSS) RDB format. */
 #define RDB_FOREIGN_VERSION_MIN 12
 #define RDB_FOREIGN_VERSION_MAX 79
-static_assert(RDB_VERSION<RDB_FOREIGN_VERSION_MIN || RDB_VERSION> RDB_FOREIGN_VERSION_MAX,
+static_assert(RDB_VERSION < RDB_FOREIGN_VERSION_MIN || RDB_VERSION > RDB_FOREIGN_VERSION_MAX,
               "RDB version in foreign version range");
 
 static inline bool rdbIsForeignVersion(int rdbver) {
diff --git a/src/server.c b/src/server.c
index cfa7bb9e70..44bd09ad4e 100644
--- a/src/server.c
+++ b/src/server.c
@@ -6929,8 +6929,8 @@ void dismissMemoryInChild(void) {
     /* madvise(MADV_DONTNEED) may not work if Transparent Huge Pages is enabled. */
     if (server.thp_enabled) return;
 
-    /* Currently we use zmadvise_dontneed only when we use jemalloc with Linux.
-     * so we avoid these pointless loops when they're not going to do anything. */
+        /* Currently we use zmadvise_dontneed only when we use jemalloc with Linux.
+         * so we avoid these pointless loops when they're not going to do anything. */
 #if defined(USE_JEMALLOC) && defined(__linux__)
     listIter li;
     listNode *ln;
diff --git a/src/vset.c b/src/vset.c
index 9d3985afcd..1b0b44d387 100644
--- a/src/vset.c
+++ b/src/vset.c
@@ -671,8 +671,8 @@ void pvSort(pVector *pv, int (*compare)(const void *a, const void *b)) {
 
 #define VOLATILESET_VECTOR_BUCKET_MAX_SIZE 127
 
-#define VSET_NONE_BUCKET_PTR ((void *)(uintptr_t)-1)
-#define VSET_BUCKET_NONE -1      // matching the NULL case
+#define VSET_NONE_BUCKET_PTR ((void *)(uintptr_t) - 1)
+#define VSET_BUCKET_NONE - 1      // matching the NULL case
 #define VSET_BUCKET_SINGLE 0x1UL // xx1 (assuming sds)
 #define VSET_BUCKET_VECTOR 0x2UL // 010
 #define VSET_BUCKET_HT 0x4UL     // 100

From f03c9ad6ed5c652718b7618c5d46db2760c5f16c Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Thu, 20 Nov 2025 16:25:54 +0000
Subject: [PATCH 29/30] lint fixes

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 src/anet.c | 2 +-
 src/vset.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/anet.c b/src/anet.c
index 3be052a6e7..5524e9cf4c 100644
--- a/src/anet.c
+++ b/src/anet.c
@@ -201,7 +201,7 @@ int anetKeepAlive(char *err, int fd, int interval) {
     if (idle < 10) idle = 10;                               // kernel expects at least 10 seconds
     if (idle > 10 * 24 * 60 * 60) idle = 10 * 24 * 60 * 60; // kernel expects at most 10 days
 
-       /* `TCP_KEEPIDLE`, `TCP_KEEPINTVL`, and `TCP_KEEPCNT` were not available on Solaris
+        /* `TCP_KEEPIDLE`, `TCP_KEEPINTVL`, and `TCP_KEEPCNT` were not available on Solaris
          * until version 11.4, but let's take a chance here. */
 #if defined(TCP_KEEPIDLE) && defined(TCP_KEEPINTVL) && defined(TCP_KEEPCNT)
     if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &idle, sizeof(idle))) {
diff --git a/src/vset.c b/src/vset.c
index 1b0b44d387..1527df8334 100644
--- a/src/vset.c
+++ b/src/vset.c
@@ -672,7 +672,7 @@ void pvSort(pVector *pv, int (*compare)(const void *a, const void *b)) {
 #define VOLATILESET_VECTOR_BUCKET_MAX_SIZE 127
 
 #define VSET_NONE_BUCKET_PTR ((void *)(uintptr_t) - 1)
-#define VSET_BUCKET_NONE - 1      // matching the NULL case
+#define VSET_BUCKET_NONE -1      // matching the NULL case
 #define VSET_BUCKET_SINGLE 0x1UL // xx1 (assuming sds)
 #define VSET_BUCKET_VECTOR 0x2UL // 010
 #define VSET_BUCKET_HT 0x4UL     // 100

From f2f7f8081774cf76ca95970c530c5b849e6d0823 Mon Sep 17 00:00:00 2001
From: Yair Gottdenker <yairg@google.com>
Date: Thu, 20 Nov 2025 17:14:48 +0000
Subject: [PATCH 30/30] fixing the test module hash_stringref initialization

Signed-off-by: Yair Gottdenker <yairg@google.com>
---
 tests/modules/hash_stringref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/modules/hash_stringref.c b/tests/modules/hash_stringref.c
index f6de1b325c..6daefd60f4 100644
--- a/tests/modules/hash_stringref.c
+++ b/tests/modules/hash_stringref.c
@@ -85,7 +85,7 @@ int ValkeyModule_OnLoad(ValkeyModuleCtx *ctx, ValkeyModuleString **argv, int arg
         VALKEYMODULE_OK &&
         ValkeyModule_CreateCommand(ctx, "hash.set_stringref", hashSetStringRef, "write",
                                   1, 1, 1) == VALKEYMODULE_OK &&
-        ValkeyModule_CreateCommand(ctx, "hash.has_stringref", hashHasStringRef, "read",
+        ValkeyModule_CreateCommand(ctx, "hash.has_stringref", hashHasStringRef, "readonly",
                                   1, 1, 1) == VALKEYMODULE_OK) {
         return VALKEYMODULE_OK;
     }