dice icon indicating copy to clipboard operation
dice copied to clipboard

Inconsistent `SET`: Handling of Binary Keys Differs Between DiceDB and Redis

Open meetghodasara opened this issue 1 year ago • 5 comments

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

Execute Command - SET key value (Where key is an binary data) E.g. key = "\x00\x01\x02" 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> SET "\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> SET "\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 absent of double-quotes)

To summarise -

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.

meetghodasara avatar Sep 23 '24 16:09 meetghodasara

@JyotinderSingh can you assign this to me, I want to give it a try.

Shimanshu83 avatar Sep 23 '24 18:09 Shimanshu83

@Shimanshu83 assigned, thanks for contributing.

lucifercr07 avatar Sep 23 '24 20:09 lucifercr07

Hello @Shimanshu83,

There has been no activity on this issue for the past 5 days. It would be awesome if you keep posting updates to this issue so that we know you are actively working on it.

We are really eager to close this issue at the earliest, hence if we continue to see the inactivity, we will have to reassign the issue to someone else. We are doing this to ensure that the project maintains its momentum and others are not blocked on this work.

Just drop a comment with the current status of the work or share any issues you are facing. We can always chip in to help you out.

Thanks again.

arpitbbhayani avatar Oct 03 '24 14:10 arpitbbhayani

Hello @arpitbbhayani , Noted was bit busy but I will wrap the issue this weekend.

Shimanshu83 avatar Oct 03 '24 15:10 Shimanshu83

Hello @arpitbbhayani, @lucifercr07

I was able to recreate the above issue while using redis-cli for redis and dice-cli for dicedb. Interestingly, when using redis-cli for both Redis and DiceDB, the behavior remains consistent.

Kindly refer below screenshot for your reference.

Screenshot from 2024-10-06 11-51-55

Upon further investigation, I observed that the issue was rooted in the difference between the CLI implementations: Redis-CLI: When binary values are enclosed within double quotes, Redis-CLI interprets and sends the data in its binary form. DiceDB-CLI: No matter if binary data is enclosed in double quotes, it is sent as a string instead of its binary representation.

Below is the tcp request made from redis-cli where you can see \x00\x01\x02\x03 represented as 00 01 02 03 not in string format Screenshot from 2024-10-06 11-55-15

But the same request made from dicedb-cli the binary key is represented as string value itself \x00\x01\x02\x03

Screenshot from 2024-10-06 11-56-57

In conclusion the issue is with difference in cli implementations not with the dicedb set command.

Shimanshu83 avatar Oct 06 '24 06:10 Shimanshu83

Hello @lucifercr07 we can close this issue as this is related to differnece in implementations between dicedb-cli and redis-cli not with dicedb set command.

Shimanshu83 avatar Oct 12 '24 08:10 Shimanshu83