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

Code before:
        addReplyError(c, "Invalid TTL value, must be >= 0");
        return;
    }

    /* Verify RDB version and data checksum. */
    if (verifyDumpPayload(objectGetVal(c->argv[3]), sdslen(objectGetVal(c->argv[3])), &rdbver) == C_ERR) {
        addReplyError(c, "DUMP payload version or checksum are wrong");
        return;
    }

    rioInitWithBuffer(&payload, objectGetVal(c->argv[3]));
    type = rdbLoadObjectType(&payload);
    if (type == -1) {
        addReplyError(c, "Bad data format");
        return;
    }

    /* If it's a foreign RDB format, only accept old data types that we know
     * existed in the past and that don't clash with new types added later. */
    if (rdbIsForeignVersion(rdbver) && type >= RDB_FOREIGN_TYPE_MIN) {
        addReplyErrorFormat(c, "Unsupported foreign data type: %d", type);
        return;
    }

    long long now = commandTimeSnapshot();
    obj = rdbLoadObject(type, &payload, objectGetVal(key), c->db->id, NULL, RDBFLAGS_NONE, now);
    if (obj == NULL) {
        addReplyError(c, "Bad data format");
        return;
    }

    /* Remove the old key if needed. */
    int deleted = 0;
    if (replace) deleted = dbDelete(c->db, key);

    if (ttl && !absttl) ttl += commandTimeSnapshot();
    if (ttl && checkAlreadyExpired(ttl)) {
        if (deleted) {
            /* Here we don't use deleteExpiredKeyFromOverwriteAndPropagate because
             * strictly speaking, the `delete` is triggered by the `replace`. */
            robj *aux = server.lazyfree_lazy_server_del ? shared.unlink : shared.del;
            rewriteClientCommandVector(c, 2, aux, key);
            signalModifiedKey(c, c->db, key);
            notifyKeyspaceEvent(NOTIFY_GENERIC, "del", key, c->db->id);
            server.dirty++;
        }
        decrRefCount(obj);
        addReply(c, shared.ok);
        return;
    }


Review comment (suggestion):
I have small concern that if we drop some elements during the load we might not be able to verify the fact that these fields have been expired. For example, imagine the restore is applying these elements before they expired and when the command is processed they are already expired... in such case we will drop them, but we will not issue any keyspace event and they will just silently disappear. Also we have no logic inside rdbLoadObject which takes into account import-mode/replication etc... 
For example, see how the restore command handles expired keys being restored:
```
if (ttl && !absttl) ttl += commandTimeSnapshot();
    if (ttl && checkAlreadyExpired(ttl)) {
        if (deleted) {
            /* Here we don't use deleteExpiredKeyFromOverwriteAndPropagate because
             * strictly speaking, the `delete` is triggered by the `replace`. */
            robj *aux = server.lazyfree_lazy_server_del ? shared.unlink : shared.del;
            rewriteClientCommandVector(c, 2, aux, key);
            signalModifiedKey(c, c->db, key);
            notifyKeyspaceEvent(NOTIFY_GENERIC, "del", key, c->db->id);
            server.dirty++;
        }
        decrRefCount(obj);
        addReply(c, shared.ok);
        return;
    }
```
I think maybe we should just restore all fields here even if they expired?
I also think we should add a test for that case...


Code after:
        addReplyError(c, "Invalid TTL value, must be >= 0");
        return;
    }

    /* Verify RDB version and data checksum. */
    if (verifyDumpPayload(objectGetVal(c->argv[3]), sdslen(objectGetVal(c->argv[3])), &rdbver) == C_ERR) {
        addReplyError(c, "DUMP payload version or checksum are wrong");
        return;
    }

    rioInitWithBuffer(&payload, objectGetVal(c->argv[3]));
    type = rdbLoadObjectType(&payload);
    if (type == -1) {
        addReplyError(c, "Bad data format");
        return;
    }

    /* If it's a foreign RDB format, only accept old data types that we know
     * existed in the past and that don't clash with new types added later. */
    if (rdbIsForeignVersion(rdbver) && type >= RDB_FOREIGN_TYPE_MIN) {
        addReplyErrorFormat(c, "Unsupported foreign data type: %d", type);
        return;
    }

    obj = rdbLoadObject(type, &payload, objectGetVal(key), c->db->id, NULL, RDBFLAGS_NONE, 0);
    if (obj == NULL) {
        addReplyError(c, "Bad data format");
        return;
    }

    /* Remove the old key if needed. */
    int deleted = 0;
    if (replace) deleted = dbDelete(c->db, key);

    if (ttl && !absttl) ttl += commandTimeSnapshot();
    if (ttl && checkAlreadyExpired(ttl)) {
        if (deleted) {
            /* Here we don't use deleteExpiredKeyFromOverwriteAndPropagate because
             * strictly speaking, the `delete` is triggered by the `replace`. */
            robj *aux = server.lazyfree_lazy_server_del ? shared.unlink : shared.del;
            rewriteClientCommandVector(c, 2, aux, key);
            signalModifiedKey(c, c->db, key);
            notifyKeyspaceEvent(NOTIFY_GENERIC, "del", key, c->db->id);
            server.dirty++;
        }
        decrRefCount(obj);
        addReply(c, shared.ok);
        return;
    }

    /* Create the key and set the TTL if any */