feat: [GH-203] Add support for PXAT in SET command #237
Conversation
|
|
||
| exDurationUnixMs, err := strconv.ParseInt(args[i], 10, 64) | ||
| if err != nil { | ||
| return Encode(errors.New("ERR value is not an integer or out of range"), false) |
There was a problem hiding this comment.
Not a requirement for this PR, but it may be a good time to start consolidating/reusing the error messages for easier maintainability and ensuring that the error messages are exactly as that of REDIS.
@arpitbbhayani @JyotinderSingh if you guys feel there is a need i can open a PR.
There was a problem hiding this comment.
@soumya-codes please let me know I can create an issue and work on this if required.
There was a problem hiding this comment.
You can create the task for tracking purposes. Let's wait for Arpit and Jyotinder to see if this task needs to prioritized now over adding newer features. I am new to the project and I not aware of the current priorities.
There was a problem hiding this comment.
@soumya-codes Good point. We should start consolidating error messages. High time :)
There was a problem hiding this comment.
@lucifercr07 the changes look good and the test cases are exhaustive. I have left a couple of comments. Please let me know what you think about the same.
|
|
||
| exDurationMs = exDurationUnixMs - time.Now().UnixMilli() | ||
| // If the expiry time is in the past, set exDurationMs to 0 | ||
| // This will be used to signal immediate expiration |
There was a problem hiding this comment.
nit: Instead of creating a new object with expiry time set to zero, should we delete the key instead? How is it handled in Redis?
There was a problem hiding this comment.
Redis does not reject the key, instead it sets the key and immediately expires it if the timestamp has already passed.
| @@ -45,7 +45,7 @@ func NewObj(value interface{}, expDurationMs int64, oType uint8, oEnc uint8) *Ob | |||
| TypeEncoding: oType | oEnc, | |||
| LastAccessedAt: getCurrentClock(), | |||
| } | |||
| if expDurationMs > 0 { | |||
| if expDurationMs >= 0 { | |||
There was a problem hiding this comment.
yes. that is a good catch.
Change Details:
PXATinSETcommand similar to the PXAT option in Redis.SET key val PXAT unix-time-millisecondssetExpiryto be set to0 mswhen creatingNewObj.Test scenarios:
PXATe.g:str,-ve intetc.