Added support for GET in SET command #246
Conversation
| @@ -187,6 +187,19 @@ func evalSET(args []string) []byte { | |||
| if obj != nil { | |||
| return RESP_NIL | |||
| } | |||
| case "GET": | |||
| obj := Get(key) | |||
There was a problem hiding this comment.
You can just call evalGET() method before the value has been updated and return the o/p of evalGET() as is. wdyt?
There was a problem hiding this comment.
Okay, so, like, I've to use evalGET() function instead of Get(), am I right?
There was a problem hiding this comment.
@soumya-codes , I've a doubt, that since by using the evalGET function, we need to pass an array
So, first, we've to push the key to an array and then pass it to the evalGET func, which will increase the fetching time, so can we just let it be Get() only? because in other cases also, the Get() function is only used!
What say?
| tempObj := obj | ||
|
|
||
| if obj == nil { | ||
| return RESP_NIL |
There was a problem hiding this comment.
IMO it is better to just update the response here. Let us invoke PUT at just one place.
There was a problem hiding this comment.
+1. We don't have to call PUT in 2 places. Passing GET will just change the return value of the SET command.
There was a problem hiding this comment.
Yes, actually, previously I did by using Put in the end only, but there was a review from someone which mentioned that the value won't get updated if I didn't add Put at 2 places, this comment.
NVM, I'll make the changes 👍
There was a problem hiding this comment.
you don't need to add Put in two places but you will need a variable for a response and then you can return that in the end if it was of subtype GET instead of RESP_OK. If you just return it from here only then PUT never gets called and SET command doesn't execute. That is what I meant
There was a problem hiding this comment.
Hey! srivastav-yash, apologies for the miscommunication. Now I have got your point. But, actually, before doing that, I'm first trying to fix the issue with XX and NX options. If we're giving both xx and get, then the tests are failing.
| @@ -187,6 +187,19 @@ func evalSET(args []string) []byte { | |||
| if obj != nil { | |||
| return RESP_NIL | |||
| } | |||
| case "GET": | |||
There was a problem hiding this comment.
Since we are adding GET, we cannot now blindly return from XX and NX.
There was a problem hiding this comment.
Actually, I'm not able to get it completely, could you please elaborate more on this
There was a problem hiding this comment.
Hey @soumya-codes , I was just now running the tests with the XX option with the GET one. The test is failing. I think do we need to change the complete evalSET() func? I'm thinking of an approach for this, will update soon about it here 👍
Fixes #207
This PR adds a support for the
GETcommand in theSETcommand as in redis - https://redis.io/docs/latest/commands/set/