dice icon indicating copy to clipboard operation
dice copied to clipboard

Add support for Commands Interacting with Byte Array datatype

Open arpitbbhayani opened this issue 1 year ago • 10 comments

Add support for the byte array manipulation commands in DiceDB similar to the following commands 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 Jul 12 '24 09:07 arpitbbhayani

Shall i work on this?

VipinRaiP avatar Jul 21 '24 11:07 VipinRaiP

@VipinRaiP https://github.com/DiceDB/dice/issues/142 is a pre-requisite for this. @yashs360 is working on it. Once that is done, you can pick this up.

@yashs360 Keep this issue in mind while coming up with a design. Make sure you do not implement all commands :) Go with your gut feeling.

arpitbbhayani avatar Jul 21 '24 12:07 arpitbbhayani

Good to go @VipinRaiP . I had to implement Get/set/count to verify that the underlying data structure works. You can take this on and work on the other data types

yashs360 avatar Jul 22 '24 21:07 yashs360

Hey @arpitbbhayani / @yashs360

Implementing all these commands under one PR will result in a very large PR. Is this acceptable for you, or would you prefer that we break it down into smaller, more manageable PRs for easier review?

Please let me know your preferences.

MohitVachhani avatar Jul 24 '24 03:07 MohitVachhani

Smaller PRs are recommended for efficient reviews. Don't overdo however. Use your judgement.

yashs360 avatar Jul 24 '24 08:07 yashs360

I will start with BITCOUNT implementation 👍🏾

MohitVachhani avatar Jul 24 '24 09:07 MohitVachhani

Hey @arpitbbhayani / @yashs360 I have tried to raise PR for BITCOUNT, please can you guys go through it and let me know if any changes required!

Thanks 🙇🏾

MohitVachhani avatar Jul 24 '24 17:07 MohitVachhani

Shall I take up BITPOS ?

VipinRaiP avatar Jul 27 '24 06:07 VipinRaiP

Hey @VipinRaiP , need to introduce byteArray new datatypes in this itself [Using @yashs360 's PR], so planning to implement all the commands in single PR..

MohitVachhani avatar Jul 27 '24 09:07 MohitVachhani

To close this issue fully, need to implement: BITPOS, I will working on this and will be raising PR accordingly.

MohitVachhani avatar Aug 04 '24 16:08 MohitVachhani