arbha1erao commented 2 months ago

The command MSET is not consistent with Redis implementation. Here are the steps to reproduce the issue

Execute Command - MSET key value (Where key is an binary data) Excute Command - GET key

Here's the output I observed in Redis v7.2.5

# On Redis v7.2.5:
127.0.0.1:6379> MSET "\x00\x01\x02" "value"
OK
127.0.0.1:6379> get \x00\x01\x02
(nil)
127.0.0.1:6379> get "\x00\x01\x02"
"value"

and here's the output I observed in DiceDB's latest commit of the master branch

# On DiceDB:
127.0.0.1:7379> MSET "\x00\x01\x02" "value"
OK
127.0.0.1:7379> get \x00\x01\x02
"value"
127.0.0.1:7379> get "\x00\x01\x02"
"value"

(Observe the value printed by GET in absense of double-quotes)

To summarize -

In DiceDB, both the raw binary key (\x00\x01\x02) and the escaped string representation ("\x00\x01\x02") retrieve the value correctly.
In Redis, the raw binary key (\x00\x01\x02) returns (nil), while the escaped string version ("\x00\x01\x02") correctly retrieves the value.

Make the implementation consistent with the Redis implementation. Make sure you are using Redis version 7.2.5 as a reference for the command implementation and to setup Redis

from source code, or
use Docker.

swarajrb7 commented 2 months ago

Hi @arpitbbhayani I would like to work on this issue.

arbha1erao commented 2 months ago

I came across this issue while working on https://github.com/DiceDB/dice/issues/378

cc: @JyotinderSingh

AshwinKul28 commented 2 months ago

@swarajrb7 go for it. Thanks for picking it up!

arbha1erao commented 2 months ago

Few more additions to this -

This is occurring with SET command as well.

# On DiceDB
127.0.0.1:7379> SET "\x00\x01" "value"
OK
127.0.0.1:7379> get "\x00\x01"
"value"
127.0.0.1:7379> get \x00\x01
"value"

# On Redis
127.0.0.1:6379> SET "\x00\x01" "value"
OK
127.0.0.1:6379> get "\x00\x01"
"value"
127.0.0.1:6379> get \x00\x01
(nil)

swarajrb7 commented 2 months ago

In Redis, the key is treated as a binary-safe string, while DiceDB seems to be interpreting unquoted binary sequences as valid keys, leading to the discrepancy. Redis used RESP(Redis Serialization Protocol). In RESP, strings are represented as a $ character, followed by the length of the string in decimal format, followed by \r\n. The string itself can contain arbitrary characters including \0, \r and \n and can hence be used to represent binary blobs

swarajrb7 commented 1 month ago

Hello might not be able to work on this. Can we un-assign me so that we have someone else pick this up for now? @arpitbbhayani / @AshwinKul28