Enhancement: `Append` to support other datatypes

DiceDB / dice

DiceDB is a redis-compliant, reactive, scalable, highly-available, unified cache optimized for modern hardware.

https://dicedb.io/

Other

6.83k stars 1.08k forks source link

Enhancement: `Append` to support other datatypes #1193

Closed apoorvyadav1111 closed 1 week ago

apoorvyadav1111 commented 4 weeks ago

Hi team,

Command Append has recently been migrated and missed support for all available datatypes. We need to make it consistent with Redis. For more context, please go through #1095 comments and unresolved review comments.

Requirements:

[ ] All datatypes are supported and error handling must be consistent.
[ ] updating unit tests to cover new enhancement
[ ] updating all integration test to test new enhancement
[ ] documentation updates with new enhancement

Thanks

shashi-sah2003 commented 4 weeks ago

@apoorvyadav1111 if noone is assigned on this then I would like to take this thanks

apoorvyadav1111 commented 4 weeks ago

Thanks for picking this up @shashi-sah2003.

shashi-sah2003 commented 3 weeks ago

@apoorvyadav1111 the command APPEND works only with string datatype and it will throw error when used with other datatype you want me to check for all the other datatypes if it's throwing error or not?

apoorvyadav1111 commented 3 weeks ago

Hi @shashi-sah2003 , yes please, we need to handle all datatypes which should be stringified. We need to be consistent with redis, hence, please check the behavior on redis.

shashi-sah2003 commented 3 weeks ago

@apoorvyadav1111 when I'm trying to append in sorted set then it should give relevant error but the server is being closed immediately and the below error is appeared on terminal, rest all other datatypes is giving relevant error/output

apoorvyadav1111 commented 3 weeks ago

Hi @shashi-sah2003 , Can you please create a draft pr with the code throwing this error? Thanks

shashi-sah2003 commented 2 weeks ago

@apoorvyadav1111 I have created draft PR for the above issue with APPEND command pls review it https://github.com/DiceDB/dice/pull/1233 also how should I go about fixing this issue or from where it is arising from? thanks

c-harish commented 2 weeks ago

@apoorvyadav1111 I have created draft PR for the above issue with APPEND command pls review it #1233 also how should I go about fixing this issue or from where it is arising from? thanks

Hi @shashi-sah2003. Are you working on this panic exit? If not, I can raise a fix for it.

apoorvyadav1111 commented 2 weeks ago

Hi @shashi-sah2003 , I ll take a look today afternoon and we can move ahead from there.

Thanks, Apoorv

shashi-sah2003 commented 2 weeks ago

@apoorvyadav1111 I have created draft PR for the above issue with APPEND command pls review it #1233 also how should I go about fixing this issue or from where it is arising from? thanks

Hi @shashi-sah2003. Are you working on this panic exit? If not, I can raise a fix for it.

Hey @c-harish I have been assigned this issue and working on it. Instead it would be helpful if you can give your insight on solving this issue?