LINDEX implementation added #1395
Conversation
|
@apoorvyadav1111 @lucifercr07 Please review the PR . Issue : #1339 |
|
Hi @Arijit6258, apologies for the delayed review. The team is now returning from vacations and will review this soon. In the meantime could you please resolve the merge conflicts that are happening? |
|
Hi @JyotinderSingh , sure I will check. |
|
@JyotinderSingh I have merged the latest master. There is a failure in integration test,which is due to some commit in master. You can check master branch has also test-failed commits. |
|
|
||
| obj := store.Get(key) | ||
|
|
||
| if obj == nil { |
There was a problem hiding this comment.
If a key doesn't exist we should return nil similar to redis.
> LINDEX test 1
(nil)
There was a problem hiding this comment.
@lucifercr07 does "(nil)" works ?? (nil) with string quotation.
There was a problem hiding this comment.
Do we need to make the behavior exactly like Redis? I feel the user might not be able to understand why (nil) is being returned in this case. Showing users a proper error message seems to be a better solution to me.
There was a problem hiding this comment.
@lucifercr07 @JyotinderSingh waiting for your replies. Accordingly I will change and update the PR.
| index, err := strconv.ParseInt(args[1], 10, 64) | ||
|
|
||
| if err != nil { | ||
| return makeEvalError(errors.New("-ERR value is not an integer or out of range")) |
There was a problem hiding this comment.
Shall we reuse this var here? IntOrOutOfRangeErr
We can create similar in migrated_errors.go and reuse it.
No description provided.