DiceDB
Inconsistent `JSON.ARRLEN`: Mismatching DiceCLI output when compared with RedisCLI output
Steps to reproduce
I was going through JSON.ARRLEN command and compared the outputs in both redis-cli and dice-cli. Almost all usecases are not giving the expected outputs. I am sharing some screenshots for your reference.
Total 4 mismatches found.
Here are the keys with 2 different JSONs
-
JSON with object in root
-
JSON with array in root
Mismatch 1
- Redis
- Dice
Mismatch 2
- Redis
- Dice
Mismatch 3
- Redis
- Dice
Mismatch 4
- Redis
- Dice
Expected output
The expected output when the above set of commands (maybe when run on Redis)
Expectations for resolution
This issue will be considered resolved when the following things are done
- changes in the
dicecode to meet the expected behavior - addition of relevant test case to ensure we catch the regression
You can find the tests under the integration_tests directory of the dice repository and the steps to run are in the README file. Refer to the following links to set up DiceDB and Redis 7.2.5 locally
@lucifercr07 @JyotinderSingh I would like to work on this issue myself
Assigned
Hi @JyotinderSingh @arpitbbhayani, @lucifercr07.
The current implementation of JSON.ARRLEN is limited to handling straightforward JSONPaths, such as $.key1.key2.... Given that we are directly replicating this command from Redis, it is essential to ensure that its functionality aligns completely with that of Redis. However, the existing implementation lacks coverage for numerous use cases.
To address this, I will compile a comprehensive list of these use cases and work on enhancing JSON.ARRLEN accordingly. Additionally, I will develop test cases to validate the new implementations.
Hi @JyotinderSingh @arpitbbhayani, @lucifercr07.
The current implementation of
JSON.ARRLENis limited to handling straightforward JSONPaths, such as$.key1.key2.... Given that we are directly replicating this command from Redis, it is essential to ensure that its functionality aligns completely with that of Redis. However, the existing implementation lacks coverage for numerous use cases.To address this, I will compile a comprehensive list of these use cases and work on enhancing
JSON.ARRLENaccordingly. Additionally, I will develop test cases to validate the new implementations.
We have a list of inconsistencies already that @psrvere had prepared a while back.
https://github.com/DiceDB/dice/issues/493#issuecomment-2350824277
I'll actually create a new issue for this so we have a proper thread to talk about it.
@JyotinderSingh. Ok, for now I will fix the issues in the basic implementation of JSON.ARRLEN and send a PR for this issue. We can then have a new issue for solving inconsistencies related to JSONPaths when compared with redis. Will this work?
@JyotinderSingh. Ok, for now I will fix the issues in the basic implementation of JSON.ARRLEN and send a PR for this issue. We can then have a new issue for solving inconsistencies related to JSONPaths when compared with redis. Will this work?
Sounds good!