Repository: valkey-io/valkey
File: src/t_hash.c (C)

Code before:
        serverAssert(lpRandomPairsUnique(hash->ptr, count, fields, vals) == count);
        hrandfieldReplyWithListpack(wpc, count, fields, vals);
        zfree(fields);
        zfree(vals);
        return;
    }

    /* CASE 3:
     * The number of elements inside the hash is not greater than
     * HRANDFIELD_SUB_STRATEGY_MUL times the number of requested elements.
     * In this case we create a hash from scratch with all the elements, and
     * subtract random elements to reach the requested number of elements.
     *
     * This is done because if the number of requested elements is just
     * a bit less than the number of elements in the hash, the natural approach
     * used into CASE 4 is highly inefficient. */
    if (count * HRANDFIELD_SUB_STRATEGY_MUL > size) {
        /* Hashtable encoding (generic implementation) */
        hashtable *ht = hashtableCreate(&sdsReplyHashtableType);
        hashtableExpand(ht, size);
        hashTypeIterator hi;
        hashTypeInitIterator(hash, &hi);

        /* Add all the elements into the temporary hashtable. */
        while (hashTypeNext(&hi) != C_ERR) {
            serverAssert(hashtableAdd(ht, (&hi)->next));
        }
        serverAssert(hashtableSize(ht) == size);
        hashTypeResetIterator(&hi);

        /* Remove random elements to reach the right count. */
        while (size > count) {
            void *element;
            hashtableFairRandomEntry(ht, &element);
            hashtableDelete(ht, element);
            size--;
        }

        /* Reply with what's in the temporary hashtable and release memory */
        hashtableIterator iter;
        hashtableInitIterator(&iter, ht, 0);
        void *next;
        while (hashtableNext(&iter, &next)) {
            sds field = hashTypeEntryGetField(next);
            sds value = hashTypeEntryGetValue(next);
            if (withvalues && c->resp > 2) addWritePreparedReplyArrayLen(wpc, 2);
            addReplyBulkCBuffer(c, field, sdslen(field));
            if (withvalues) addReplyBulkCBuffer(c, value, sdslen(value));
        }

        hashtableResetIterator(&iter);

Review comment (suggestion):
`hi.next` looks like it's the next element but it's actually the current element. Maybe we should rename the `void *next` iterator field to `hashTypeEntry *currentHashtableEntry`. It might make the code a bit more clear.

Or maybe better: we could treat it as internals of the iterator and not access it directly here. The hashTypeIterator is supposed to work regardless of encoding and here rely on it being a hashtable. But since we know it's a hashtable, we can use a hashtableIterator directly instead of a hashTypeIterator:

```C
        hashtableIterator iter;
        hashtableInitIterator(&iter, hash->ptr, 0);
        void *entry;
        while (hashtableNext(&iter, &entry)) {
            hashtableAdd(ht, entry);
        }
        hashtableResetIterator(&iter);
```

Regarding the assert: In general, it's not good practice to do work inside asserts, because if we ever want to compile the code with assert disabled, the entry will not be added. I know we don't follow this very well, but still. We can just check the result in the assert, like

```C
            int added = hashtableAdd(ht, entry);
            serverAssert(added);
```

Code after:
        serverAssert(lpRandomPairsUnique(hash->ptr, count, fields, vals) == count);
        hrandfieldReplyWithListpack(wpc, count, fields, vals);
        zfree(fields);
        zfree(vals);
        return;
    }

    /* CASE 3:
     * The number of elements inside the hash is not greater than
     * HRANDFIELD_SUB_STRATEGY_MUL times the number of requested elements.
     * In this case we create a hash from scratch with all the elements, and
     * subtract random elements to reach the requested number of elements.
     *
     * This is done because if the number of requested elements is just
     * a bit less than the number of elements in the hash, the natural approach
     * used into CASE 4 is highly inefficient. */
    if (count * HRANDFIELD_SUB_STRATEGY_MUL > size) {
        /* Hashtable encoding (generic implementation) */
        hashtable *ht = hashtableCreate(&sdsReplyHashtableType);
        hashtableExpand(ht, size);
        hashtableIterator iter;
        hashtableInitIterator(&iter, hash->ptr, 0);
        void *entry;

        /* Add all the elements into the temporary hashtable. */
        while (hashtableNext(&iter, &entry)) {
            int res = hashtableAdd(ht, entry);
            serverAssert(res);
        }
        serverAssert(hashtableSize(ht) == size);
        hashtableResetIterator(&iter);

        /* Remove random elements to reach the right count. */
        while (size > count) {
            void *element;
            hashtableFairRandomEntry(ht, &element);
            hashtableDelete(ht, element);
            size--;
        }

        /* Reply with what's in the temporary hashtable and release memory */
        hashtableInitIterator(&iter, ht, 0);
        void *next;
        while (hashtableNext(&iter, &next)) {
            sds field = hashTypeEntryGetField(next);
            sds value = hashTypeEntryGetValue(next);
            if (withvalues && c->resp > 2) addWritePreparedReplyArrayLen(wpc, 2);
            addWritePreparedReplyBulkCBuffer(wpc, field, sdslen(field));
            if (withvalues) addWritePreparedReplyBulkCBuffer(wpc, value, sdslen(value));
        }