feat(set_family): Add support for KEEPTTL to SAddEx #4712
Conversation
Arg parsing is done by parser in SAddEx. A test is added to check invalid expiry handling.
abhijat
force-pushed
the
abhijat/feat/add_keepttl_to_saddex
branch
from
b2b4823 to
d83147c
Compare
When KEEPTTL is supplied with args, any existing keys in the set will preserve their TTL values. Only new members will get TTL applied to them.
abhijat
force-pushed
the
abhijat/feat/add_keepttl_to_saddex
branch
from
d83147c to
210fc76
Compare
src/server/set_family.cc
Outdated
| constexpr uint32_t kMaxTtl = (1UL << 26); | ||
| CmdArgParser parser(args); | ||
|
|
||
| auto key = parser.Next<std::string_view>(); |
There was a problem hiding this comment.
remember to use explicit types for simple types
abhijat
changed the title
src/server/set_family_test.cc
Outdated
|
|
||
| // KEEPTTL support. add field orig with TTL=10 | ||
| EXPECT_THAT(Run({"saddex", "key", "10", "orig"}), IntArg(1)); | ||
| // add fields new and orig with TTL=5 and KEEPTTL=true. orig ttl should be preserved |
There was a problem hiding this comment.
the comment says ttl=5 but in this command I see ttl=1
There was a problem hiding this comment.
Yeah I mixed up the comments, updated now.
| EXPECT_THAT(Run({"saddex", "key", "10", "orig"}), IntArg(1)); | ||
| // add fields new and orig with TTL=5 and KEEPTTL=true. orig ttl should be preserved | ||
| EXPECT_THAT(Run({"saddex", "key", "KEEPTTL", "1", "orig", "new"}), IntArg(1)); | ||
| EXPECT_GT(CheckedInt({"fieldttl", "key", "orig"}), 5); |
There was a problem hiding this comment.
I am confused, why is the fieldttl here is not 10?
There was a problem hiding this comment.
also lets check the CheckedInt({"fieldttl", "key", "new"}) which should be set to 1
There was a problem hiding this comment.
I am confused, why is the fieldttl here is not 10?
Oh you are checking EXPECT_GT I now understand
There was a problem hiding this comment.
Yeah, since some time passed since we set it to 10, I used 5 as an approximation to compare, so the test isn't flaky.
also lets check the CheckedInt({"fieldttl", "key", "new"}) which should be set to 1
Good point, added EXPECT_LE for 1.
|
please also update our documentaion for the command here - https://github.com/dragonflydb/documentation |
Support is added for KEEPTTL to the SAddEx command as described in #4701