Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HrangebyLex supports specify intervals #1120

Merged
merged 26 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4044c05
Support more hrange options
tanruixiang Nov 13, 2022
cd7f46d
Merge branch 'unstable' into support_more_hrange_opitons
tanruixiang Nov 14, 2022
1c9e289
Merge branch 'unstable' into support_more_hrange_opitons
tanruixiang Nov 15, 2022
7be715e
Merge branch 'unstable' into support_more_hrange_opitons
tanruixiang Dec 2, 2022
ee9a2fe
rename hrange
tanruixiang Dec 3, 2022
4be2812
”fix“
tanruixiang Dec 4, 2022
5d743ea
Merge branch 'unstable' into support_more_hrange_opitons
tanruixiang Dec 4, 2022
3566014
remove redundant variables
tanruixiang Dec 4, 2022
f51b6d7
Merge branch 'support_more_hrange_opitons' of github.com:tanruixiang/…
tanruixiang Dec 4, 2022
a2a48d9
fix typo
tanruixiang Dec 6, 2022
3cf648e
Merge branch 'unstable' into support_more_hrange_opitons
tanruixiang Dec 6, 2022
d47b335
Merge branch 'unstable' into support_more_hrange_opitons
PragmaTwice Dec 6, 2022
4b4ac09
fix code style
tanruixiang Dec 10, 2022
4c6fb4b
Merge branch 'support_more_hrange_opitons' of github.com:tanruixiang/…
tanruixiang Dec 10, 2022
bfd774d
Merge branch 'unstable' into support_more_hrange_opitons
tanruixiang Dec 10, 2022
6dc81ad
Merge branch 'unstable' into support_more_hrange_opitons
PragmaTwice Dec 12, 2022
b027d43
Merge branch 'unstable' into support_more_hrange_opitons
tanruixiang Jan 8, 2023
850a1c8
use common lex range struct
tanruixiang Jan 11, 2023
3fe6c40
add lisence
tanruixiang Jan 11, 2023
8b8485d
format
tanruixiang Jan 11, 2023
a2f53a8
move ParseRangeLexSpec
tanruixiang Jan 13, 2023
5073fad
Merge branch 'unstable' into support_more_hrange_opitons
tanruixiang Jan 13, 2023
427f816
rename
tanruixiang Jan 13, 2023
581597d
add empty line
tanruixiang Jan 13, 2023
644298b
Merge branch 'unstable' into support_more_hrange_opitons
PragmaTwice Jan 17, 2023
37bc82b
Merge branch 'unstable' into support_more_hrange_opitons
PragmaTwice Jan 17, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 22 additions & 17 deletions src/commands/redis_cmd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1614,34 +1614,39 @@ class CommandHGetAll : public Commander {
}
};

class CommandHRange : public Commander {
class CommandHRangeByLex : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
if (args.size() != 6 && args.size() != 4) {
return {Status::RedisParseErr, errWrongNumOfArguments};
CommandParser parser(args, 4);
while (parser.Good()) {
if (parser.EatEqICase("REV")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why support REV option?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be aligned with zrange and makes sense, for example if you need to take the largest data.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About aligning zrange I will deal with it in the new pr.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think hrangebylex is enough currently since i heard there is such a demand. Developers may not be used to that hash support range operations, we can implement when someone wants it, WDYT?

spec_.reversed = true;
} else if (parser.EatEqICase("LIMIT")) {
spec_.offset = GET_OR_RET(parser.TakeInt());
spec_.count = GET_OR_RET(parser.TakeInt());
} else {
return parser.InvalidSyntax();
}
}

if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
return {Status::RedisInvalidCmd, errInvalidSyntax};
Status s;
if (spec_.reversed) {
s = Redis::Hash::ParseRangeLexSpec(args[3], args[2], &spec_);
} else {
s = Redis::Hash::ParseRangeLexSpec(args[2], args[3], &spec_);
}

if (args.size() == 6) {
auto parse_result = ParseInt<int64_t>(args_[5], 10);
if (!parse_result) return {Status::RedisParseErr, errValueNotInteger};

limit_ = *parse_result;
if (!s.IsOK()) {
return Status(Status::RedisParseErr, s.Msg());
}
return Commander::Parse(args);
return Status::OK();
}

Status Execute(Server *svr, Connection *conn, std::string *output) override {
Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
std::vector<FieldValue> field_values;
auto s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
rocksdb::Status s = hash_db.RangeByLex(args_[1], spec_, &field_values);
if (!s.ok()) {
return {Status::RedisExecErr, s.ToString()};
}

std::vector<std::string> kv_pairs;
for (const auto &p : field_values) {
kv_pairs.emplace_back(p.field);
Expand All @@ -1653,7 +1658,7 @@ class CommandHRange : public Commander {
}

private:
int64_t limit_ = LONG_MAX;
HashRangeSpec spec_;
tanruixiang marked this conversation as resolved.
Show resolved Hide resolved
};

class CommandPush : public Commander {
Expand Down Expand Up @@ -6334,7 +6339,7 @@ REDIS_REGISTER_COMMANDS(
MakeCmdAttr<CommandHVals>("hvals", 2, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHGetAll>("hgetall", 2, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHScan>("hscan", -3, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHRange>("hrange", -4, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHRangeByLex>("hrangebylex", -4, "read-only", 1, 1, 1),

MakeCmdAttr<CommandLPush>("lpush", -3, "write", 1, 1, 1), MakeCmdAttr<CommandRPush>("rpush", -3, "write", 1, 1, 1),
MakeCmdAttr<CommandLPushX>("lpushx", -3, "write", 1, 1, 1),
Expand Down
82 changes: 71 additions & 11 deletions src/types/redis_hash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,37 +276,65 @@ rocksdb::Status Hash::MSet(const Slice &user_key, const std::vector<FieldValue>
return storage_->Write(storage_->DefaultWriteOptions(), &batch);
}

rocksdb::Status Hash::Range(const Slice &user_key, const Slice &start, const Slice &stop, int64_t limit,
std::vector<FieldValue> *field_values) {
rocksdb::Status Hash::RangeByLex(const Slice &user_key, const HashRangeSpec &spec,
std::vector<FieldValue> *field_values) {
field_values->clear();
if (start.compare(stop) >= 0 || limit <= 0) {
if (spec.count == 0) {
return rocksdb::Status::OK();
}
std::string ns_key;
AppendNamespacePrefix(user_key, &ns_key);
HashMetadata metadata(false);
rocksdb::Status s = GetMetadata(ns_key, &metadata);
if (!s.ok()) return s.IsNotFound() ? rocksdb::Status::OK() : s;
limit = std::min(static_cast<int64_t>(metadata.size), limit);
std::string start_key, stop_key;
InternalKey(ns_key, start, metadata.version, storage_->IsSlotIdEncoded()).Encode(&start_key);
InternalKey(ns_key, stop, metadata.version, storage_->IsSlotIdEncoded()).Encode(&stop_key);

std::string start_member = spec.reversed ? spec.max : spec.min;
std::string start_key, prefix_key, next_version_prefix_key;
InternalKey(ns_key, start_member, metadata.version, storage_->IsSlotIdEncoded()).Encode(&start_key);
InternalKey(ns_key, "", metadata.version, storage_->IsSlotIdEncoded()).Encode(&prefix_key);
InternalKey(ns_key, "", metadata.version + 1, storage_->IsSlotIdEncoded()).Encode(&next_version_prefix_key);
rocksdb::ReadOptions read_options;
LatestSnapShot ss(db_);
read_options.snapshot = ss.GetSnapShot();
rocksdb::Slice upper_bound(stop_key);
rocksdb::Slice upper_bound(next_version_prefix_key);
read_options.iterate_upper_bound = &upper_bound;
rocksdb::Slice lower_bound(prefix_key);
read_options.iterate_lower_bound = &lower_bound;
read_options.fill_cache = false;

auto iter = DBUtil::UniqueIterator(db_, read_options);
iter->Seek(start_key);
for (int64_t i = 0; iter->Valid() && i <= limit - 1; ++i) {
if (!spec.reversed) {
iter->Seek(start_key);
} else {
if (spec.max_infinite) {
iter->SeekToLast();
} else {
iter->SeekForPrev(start_key);
}
}
int64_t pos = 0;
for (; iter->Valid() && iter->key().starts_with(prefix_key); (!spec.reversed ? iter->Next() : iter->Prev())) {
FieldValue tmp_field_value;
InternalKey ikey(iter->key(), storage_->IsSlotIdEncoded());
if (spec.reversed) {
if (ikey.GetSubKey().ToString() < spec.min || (spec.minex && ikey.GetSubKey().ToString() == spec.min)) {
break;
}
if ((spec.maxex && ikey.GetSubKey().ToString() == spec.max) ||
(!spec.max_infinite && ikey.GetSubKey().ToString() > spec.max)) {
continue;
}
} else {
if (spec.minex && ikey.GetSubKey().ToString() == spec.min) continue; // the min member was exclusive
if ((spec.maxex && ikey.GetSubKey().ToString() == spec.max) ||
(!spec.max_infinite && ikey.GetSubKey().ToString() > spec.max))
break;
}
if (spec.offset >= 0 && pos++ < spec.offset) continue;
tmp_field_value.field = ikey.GetSubKey().ToString();
tmp_field_value.value = iter->value().ToString();
field_values->emplace_back(tmp_field_value);
iter->Next();
if (spec.count > 0 && field_values && field_values->size() >= static_cast<unsigned>(spec.count)) break;
}
return rocksdb::Status::OK();
}
Expand Down Expand Up @@ -355,4 +383,36 @@ rocksdb::Status Hash::Scan(const Slice &user_key, const std::string &cursor, uin
return SubKeyScanner::Scan(kRedisHash, user_key, cursor, limit, field_prefix, fields, values);
}

Status Hash::ParseRangeLexSpec(const std::string &min, const std::string &max, HashRangeSpec *spec) {
tanruixiang marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

@PragmaTwice PragmaTwice Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can merge these same structure and methods of HashRangeSpec and ZRangeSpec in next PR.

Copy link
Member

@ShooterIT ShooterIT Dec 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not this pr, i don't think we should import different function to deal with similar things

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no strong opinion for this. Could you merge these structures and functions in this PR? @tanruixiang

Copy link
Member Author

@tanruixiang tanruixiang Dec 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no strong opinion for this. Could you merge these structures and functions in this PR? @tanruixiang

Ok. Sorry to have missed this comment.

if (min == "+" || max == "-") {
return Status(Status::NotOK, "min > max");
}

if (min == "-") {
spec->min = "";
} else {
if (min[0] == '(') {
spec->minex = true;
} else if (min[0] == '[') {
spec->minex = false;
} else {
return Status(Status::NotOK, "the min is illegal");
}
spec->min = min.substr(1);
}

if (max == "+") {
spec->max_infinite = true;
} else {
if (max[0] == '(') {
spec->maxex = true;
} else if (max[0] == '[') {
spec->maxex = false;
} else {
return Status(Status::NotOK, "the max is illegal");
}
spec->max = max.substr(1);
}
return Status::OK();
}
} // namespace Redis
14 changes: 12 additions & 2 deletions src/types/redis_hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ struct FieldValue {

enum class HashFetchType { kAll = 0, kOnlyKey = 1, kOnlyValue = 2 };

struct HashRangeSpec {
tanruixiang marked this conversation as resolved.
Show resolved Hide resolved
std::string min, max;
bool minex, maxex; /* are min or max exclusive */
bool max_infinite; /* are max infinite */
int64_t offset, count;
bool removed, reversed;
HashRangeSpec()
: minex(false), maxex(false), max_infinite(false), offset(-1), count(-1), removed(false), reversed(false) {}
};

namespace Redis {
class Hash : public SubKeyScanner {
public:
Expand All @@ -48,8 +58,8 @@ class Hash : public SubKeyScanner {
rocksdb::Status IncrBy(const Slice &user_key, const Slice &field, int64_t increment, int64_t *ret);
rocksdb::Status IncrByFloat(const Slice &user_key, const Slice &field, double increment, double *ret);
rocksdb::Status MSet(const Slice &user_key, const std::vector<FieldValue> &field_values, bool nx, int *ret);
rocksdb::Status Range(const Slice &user_key, const Slice &start, const Slice &stop, int64_t limit,
std::vector<FieldValue> *field_values);
static Status ParseRangeLexSpec(const std::string &min, const std::string &max, HashRangeSpec *spec);
rocksdb::Status RangeByLex(const Slice &user_key, const HashRangeSpec &spec, std::vector<FieldValue> *field_values);
rocksdb::Status MGet(const Slice &user_key, const std::vector<Slice> &fields, std::vector<std::string> *values,
std::vector<rocksdb::Status> *statuses);
rocksdb::Status GetAll(const Slice &user_key, std::vector<FieldValue> *field_values,
Expand Down
105 changes: 100 additions & 5 deletions tests/cppunit/t_hash_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <gtest/gtest.h>

#include <algorithm>
#include <climits>
#include <memory>
#include <random>
#include <string>
Expand Down Expand Up @@ -160,8 +161,8 @@ TEST_F(RedisHashTest, HIncrByFloat) {
hash->Del(key_);
}

TEST_F(RedisHashTest, HRange) {
int ret;
TEST_F(RedisHashTest, HRangeByLex) {
int ret = 0;
std::vector<FieldValue> fvs;
for (size_t i = 0; i < 4; i++) {
fvs.emplace_back(FieldValue{"key" + std::to_string(i), "value" + std::to_string(i)});
Expand All @@ -180,19 +181,113 @@ TEST_F(RedisHashTest, HRange) {
s = hash->MSet(key_, fvs, false, &ret);
EXPECT_EQ(ret, 0);
std::vector<FieldValue> result;
s = hash->Range(key_, "key0", "key4", INT_MAX, &result);
HashRangeSpec spec;
spec.offset = 0;
spec.count = INT_MAX;
spec.min = "key0";
spec.max = "key3";
s = hash->RangeByLex(key_, spec, &result);
EXPECT_TRUE(s.ok());
EXPECT_EQ(4, result.size());
EXPECT_EQ("key0", result[0].field);
EXPECT_EQ("key1", result[1].field);
EXPECT_EQ("key2", result[2].field);
EXPECT_EQ("key3", result[3].field);
hash->Del(key_);
}

rocksdb::Status s = hash->MSet(key_, tmp, false, &ret);
EXPECT_TRUE(s.ok() && static_cast<int>(tmp.size()) == ret);
// use offset and count
std::vector<FieldValue> result;
HashRangeSpec spec;
spec.offset = 0;
spec.count = INT_MAX;
spec.min = "key0";
spec.max = "key3";
spec.offset = 1;
s = hash->RangeByLex(key_, spec, &result);
EXPECT_TRUE(s.ok());
EXPECT_EQ(3, result.size());
EXPECT_EQ("key1", result[0].field);
EXPECT_EQ("key2", result[1].field);
EXPECT_EQ("key3", result[2].field);

spec.offset = 1;
spec.count = 1;
s = hash->RangeByLex(key_, spec, &result);
EXPECT_TRUE(s.ok());
EXPECT_EQ(1, result.size());
EXPECT_EQ("key1", result[0].field);

spec.offset = 0;
spec.count = 0;
s = hash->RangeByLex(key_, spec, &result);
EXPECT_TRUE(s.ok());
EXPECT_EQ(0, result.size());

spec.offset = 1000;
spec.count = 1000;
s = hash->RangeByLex(key_, spec, &result);
EXPECT_TRUE(s.ok());
EXPECT_EQ(0, result.size());
// exclusive range
spec.offset = 0;
spec.count = -1;
spec.minex = true;
s = hash->RangeByLex(key_, spec, &result);
EXPECT_TRUE(s.ok());
EXPECT_EQ(3, result.size());
EXPECT_EQ("key1", result[0].field);
EXPECT_EQ("key2", result[1].field);
EXPECT_EQ("key3", result[2].field);

spec.offset = 0;
spec.count = -1;
spec.maxex = true;
spec.minex = false;
s = hash->RangeByLex(key_, spec, &result);
EXPECT_TRUE(s.ok());
EXPECT_EQ(3, result.size());
EXPECT_EQ("key0", result[0].field);
EXPECT_EQ("key1", result[1].field);
EXPECT_EQ("key2", result[2].field);

spec.offset = 0;
spec.count = -1;
spec.maxex = true;
spec.minex = true;
s = hash->RangeByLex(key_, spec, &result);
EXPECT_TRUE(s.ok());
EXPECT_EQ(2, result.size());
EXPECT_EQ("key1", result[0].field);
EXPECT_EQ("key2", result[1].field);

// inf and revered
spec.minex = false;
spec.maxex = false;
spec.min = "-";
spec.max = "+";
spec.max_infinite = true;
spec.reversed = true;
s = hash->RangeByLex(key_, spec, &result);
EXPECT_TRUE(s.ok());
EXPECT_EQ(4 + 26, result.size());
EXPECT_EQ("key3", result[0].field);
EXPECT_EQ("key2", result[1].field);
EXPECT_EQ("key1", result[2].field);
EXPECT_EQ("key0", result[3].field);
hash->Del(key_);
}

TEST_F(RedisHashTest, HRangeNonExistingKey) {
TEST_F(RedisHashTest, HRangeByLexNonExistingKey) {
std::vector<FieldValue> result;
auto s = hash->Range("non-existing-key", "any-start-key", "any-end-key", 10, &result);
HashRangeSpec spec;
spec.offset = 0;
spec.count = INT_MAX;
spec.min = "any-start-key";
spec.max = "any-end-key";
auto s = hash->RangeByLex("non-existing-key", spec, &result);
EXPECT_TRUE(s.ok());
EXPECT_EQ(result.size(), 0);
}
Loading