Fix LPOP to support multiple arguments #1100 #1105
Conversation
Thanks :), Yep I am on it. |
|
this is the unit-tests this is the integration tests any changes to make? |
|
Hey, |
whoops, didn't realise the comments were addressed. Going through this PR. |
There was a problem hiding this comment.
Thanks for these changes @tren03! The implementation in itself looks solid, have left a few minor comments.
internal/eval/eval_test.go
Outdated
| @@ -120,6 +120,7 @@ func TestEval(t *testing.T) { | |||
| testEvalSINTER(t, store) | |||
| testEvalOBJECTENCODING(t, store) | |||
| testEvalJSONSTRAPPEND(t, store) | |||
| testEvalLPOP(t, store) | |||
There was a problem hiding this comment.
Looks like we are calling testEvalLPOP twice here.
There was a problem hiding this comment.
We need to add similar tests to HTTP and Websocket directory as well. As a part of this exercise it would be great if you could help verify that the rest of the tests in this file are also replicated there.
There was a problem hiding this comment.
Hey,
I was working on porting the tests to http and websockets directory.
I noticed that the http server returns a float for LPUSH, where as localhost operation returns an integer.
Is this expected behaviour?
There was a problem hiding this comment.
Hey, I was working on porting the tests to http and websockets directory. I noticed that the http server returns a float for LPUSH, where as localhost operation returns an integer. Is this expected behaviour?
There was a problem hiding this comment.
I did a mistake, in merging the branches,
This is my first so a bit nervous
I still have all the needed changes,
I have made another PR #1231 to continue the issue
Sorry for the inconvenience.
tren03
force-pushed
the
lpop-multiple-args-fix-1100
branch
from
5c75e85 to
d9c41f2
Compare
This pull request addresses issue #1100, where the LPOP command needed to support an additional argument to specify the number of elements to pop from the left side of the list.
Changes:
Modified the LPOP command to accept either 1 or 2 arguments:
LPOP key: Pops a single element from the list (default behavior).
LPOP key count: Pops up to count elements from the list.
Validation:
If count is 0, an empty response is returned.
If count is negative, an "out of range" error is returned.
If count exceeds the number of available elements, it pops as many elements as available without error.
Invalid count (non-integer values) returns an appropriate error.
Popping from a non-existent or empty list returns nil.
Behavior:
Returns a single element if one element is popped.
Returns a list of elements if multiple elements are popped.
This is the dicedb output
This is the output for redis over the same commands
