dice icon indicating copy to clipboard operation
dice copied to clipboard

Add support for command `JSON.ARRAPPEND`

Open arpitbbhayani opened this issue 1 year ago • 6 comments

Add support for the JSON.ARRAPPEND command in DiceDB similar to the JSON.ARRAPPEND command in Redis. Please refer to the following commit in Redis to understand the implementation specifics - source.

Write unit and integration tests for the command referring to the tests written in the Redis codebase 7.2.5. For integration tests, you can refer to the tests folder. Note: they have used TCL for the test suite, and we need to port that to our way of writing integration tests using the relevant helper methods. Please refer to our tests directory.

For the command, benchmark the code and measure the time taken and memory allocs using benchmem and try to keep them to the bare minimum.

arpitbbhayani avatar Sep 08 '24 18:09 arpitbbhayani

could you please assign this to me @arpitbbhayani

raghavbabbar avatar Sep 08 '24 18:09 raghavbabbar

@raghavbabbar you already have a few on your plate, once you wrap that you can pick this one. trying to be fair here to others :) but thank you so much for being so eager.

arpitbbhayani avatar Sep 08 '24 18:09 arpitbbhayani

@arpitbbhayani, I can pick this up. could you assign this to me?

srivastava-yash avatar Sep 09 '24 03:09 srivastava-yash

@srivastava-yash assigned, thanks for picking this up.

lucifercr07 avatar Sep 09 '24 04:09 lucifercr07

@lucifercr07 @JyotinderSingh Quick question: So in JSON.ARRAPPEND we want to append to a list at a given path, so should I check the type of the fields that are already there in the array and then convert this value in the argument to the same type and only then append, right? Or should I just append it assuming the user knows what they are appending? Either I can just run a sonic.UnmarshalString on the arg and just append. What do you suggest? Nothing was mentioned in the redis doc about it, maybe I can try finding it in their code

srivastava-yash avatar Sep 11 '24 00:09 srivastava-yash

@lucifercr07 @JyotinderSingh Quick question: So in JSON.ARRAPPEND we want to append to a list at a given path, so should I check the type of the fields that are already there in the array and then convert this value in the argument to the same type and only then append, right?

Or should I just append it assuming the user knows what they are appending? Either I can just run a sonic.UnmarshalString on the arg and just append. What do you suggest? Nothing was mentioned in the redis doc about it, maybe I can try finding it in their code

You need to check the type of object already present at any path which you'd like to modify. If the object doesn't match the expected type you'll return an error. Ensure that test cases validating this behavior are also added. Additionally, run the same commands on a redis instance (redis docs pages have an embedded cli to run redis commands) and ensure the results (including error responses) are consistent.

JyotinderSingh avatar Sep 11 '24 13:09 JyotinderSingh