Align the expire option behavior of redis

[tanruixiang]

Search before asking

• I had searched in the issues and found no similar issues.

Version

unstable branch.

Minimal reproduce step

127.0.0.1:6666> cas key old new ex 1 ex 10
(integer) 0

What did you expect to see?

(error) ERR syntax error

What did you see instead?

Executed successfully.

Anything Else?

Maybe other commands have this problem too. I'll fix them after double-checking. Even if the command was coded to prevent this issue (such as CommandSet), the code was written with no generality. I will provide a more general parsing function in this pr.

Are you willing to submit a PR?

• I'm willing to submit a PR!

[PragmaTwice]

I notice that actually it is ok in redis to repeat the same keyword and arguments, e.g.

127.0.0.1:6379> set key value ex 22 ex 333
OK
127.0.0.1:6379> ttl key
(integer) 331

[tanruixiang]

I notice that actually it is ok in redis to repeat the same keyword and arguments, e.g.

127.0.0.1:6379> set key value ex 22 ex 333
OK
127.0.0.1:6379> ttl key
(integer) 331

Yes, I've tested redis too, but I think duplication is a very confusing behavior. I think this is also a bug of redis, because in redis, theset key value ex 10 px 1 will get ERR syntax error

127.0.0.1:6379> set key value px 1
OK
127.0.0.1:6379> set key value ex 10 px 1
(error) ERR syntax error
127.0.0.1:6379> set key value ex 10 ex 1
OK

[git-hulk]

It can't tell whether it's a bug or not from the Redis code. But I guess Redis won't regard this behavior as a bug since most arguments like LIMIT and COUNT are allowed duplicate. Perhaps can raise an issue to ask the Redis community.

[tanruixiang]

It can't tell whether it's a bug or not from the Redis code. But I guess Redis won't regard this behavior as a bug since most arguments like LIMIT and COUNT are allowed duplicate. Perhaps can raise an issue to ask the Redis community.

If aligning redis is the starting point, we should at least guarantee cas key old new ex 1 px 10 will throw an error. In fact, our code is also inconsistent in the implementation of whether there can be repeated arguments, which also makes me more confused. For example, the current CommandSet logic does not support repeated commands. Do we need to change the CommandSet?

[git-hulk]

Yes, you're right that Redis also don't allow using ex|px|exat at the same time.

[PragmaTwice]

+1. There should be a syntax error while different options in same functionality is encountered to match the behavior of redis : )

[tisonkun]

@tanruixiang shall we close this issue as #1017 merged?

[tanruixiang]

@tanruixiang shall we close this issue as #1017 merged?

Ok. There are still some unresolved discussions that will be resolved after twice proposes a parsing-design's issue.