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

Code before:
                       robj *key,
                       robj *val,
                       robj *expire,
                       int unit,
                       robj *ok_reply,
                       robj *abort_reply,
                       robj *comparison) {
    long long milliseconds = 0; /* initialized to avoid any harmness warning */
    int found = 0;
    int setkey_flags = 0;

    if (expire && getExpireMillisecondsOrReply(c, expire, flags, unit, &milliseconds) != C_OK) {
        return;
    }

    if (flags & OBJ_SET_GET) {
        if (getGenericCommand(c) == C_ERR) return;
    }

    const robj *value = lookupKeyWrite(c->db, key);
    found = value != NULL;

    /* Handle the IFEQ conditional check */
    if (flags & OBJ_SET_IFEQ && found) {

        if (value->type != OBJ_STRING) {
            if (!(flags & OBJ_SET_GET)) {
                addReplyError(c, "value(s) must be present or string");
            }
            return;
        }

        if (compareStringObjects(value, comparison) != 0) {
            if (!(flags & OBJ_SET_GET)) {
                addReply(c, abort_reply ? abort_reply : shared.null[c->resp]);
            }
            return;
        }

    } else if (flags & OBJ_SET_IFEQ && !found) {
        if (!(flags & OBJ_SET_GET)) {
            addReply(c, abort_reply ? abort_reply : shared.null[c->resp]);
        }
        return;
    }

    if ((flags & OBJ_SET_NX && found) || (flags & OBJ_SET_XX && !found)) {
        if (!(flags & OBJ_SET_GET)) {
            addReply(c, abort_reply ? abort_reply : shared.null[c->resp]);
        }
        return;

Review comment (suggestion):
Using the `checkType` method adds an error message to the client buffer and the error message will remain the same.

Code after:
                       robj *key,
                       robj *val,
                       robj *expire,
                       int unit,
                       robj *ok_reply,
                       robj *abort_reply,
                       robj *comparison) {
    long long milliseconds = 0; /* initialized to avoid any harmness warning */
    int found = 0;
    int setkey_flags = 0;

    if (expire && getExpireMillisecondsOrReply(c, expire, flags, unit, &milliseconds) != C_OK) {
        return;
    }

    if (flags & OBJ_SET_GET) {
        if (getGenericCommand(c) == C_ERR) return;
    }

    robj *existing_value = lookupKeyWrite(c->db, key);
    found = existing_value != NULL;

    /* Handle the IFEQ conditional check */
    if (flags & OBJ_SET_IFEQ && found) {
        if (!(flags & OBJ_SET_GET) && checkType(c, existing_value, OBJ_STRING)) {
            return;
        }

        if (compareStringObjects(existing_value, comparison) != 0) {
            if (!(flags & OBJ_SET_GET)) {
                addReply(c, abort_reply ? abort_reply : shared.null[c->resp]);
            }
            return;
        }
    } else if (flags & OBJ_SET_IFEQ && !found) {
        if (!(flags & OBJ_SET_GET)) {
            addReply(c, abort_reply ? abort_reply : shared.null[c->resp]);
        }
        return;
    }

    if ((flags & OBJ_SET_NX && found) || (flags & OBJ_SET_XX && !found)) {
        if (!(flags & OBJ_SET_GET)) {
            addReply(c, abort_reply ? abort_reply : shared.null[c->resp]);
        }
        return;
    }

    /* If the `milliseconds` have expired, then we don't need to set it into the
     * database, and then wait for the active expire to delete it, it is wasteful.
     * If the key already exists, delete it. */