Skip to content

Commit

Permalink
Allow HSETNX to receive multiple field-value pairs like HSET
Browse files Browse the repository at this point in the history 
Close #1294
  • Loading branch information
@torwig
torwig committed Mar 9, 2023
1 parent 80260d7 commit b69e72c
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 41 deletions.
16 changes: 14 additions & 2 deletions src/commands/cmd_hash.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,22 @@ class CommandHGet : public Commander {

class CommandHSetNX : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
if (args.size() % 2 != 0) {
return {Status::RedisParseErr, errWrongNumOfArguments};
}
return Commander::Parse(args);
}

Status Execute(Server *svr, Connection *conn, std::string *output) override {
std::vector<FieldValue> field_values;
for (size_t i = 2; i < args_.size(); i += 2) {
field_values.emplace_back(FieldValue{args_[i], args_[i + 1]});
}

int ret = 0;
Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
auto s = hash_db.SetNX(args_[1], args_[2], args_[3], &ret);
auto s = hash_db.MSet(args_[1], field_values, true, &ret);
if (!s.ok()) {
return {Status::RedisExecErr, s.ToString()};
}
Expand Down Expand Up @@ -366,7 +378,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandHGet>("hget", 3, "read-only", 1, 1, 1
MakeCmdAttr<CommandHIncrBy>("hincrby", 4, "write", 1, 1, 1),
MakeCmdAttr<CommandHIncrByFloat>("hincrbyfloat", 4, "write", 1, 1, 1),
MakeCmdAttr<CommandHMSet>("hset", -4, "write", 1, 1, 1),
MakeCmdAttr<CommandHSetNX>("hsetnx", 4, "write", 1, 1, 1),
MakeCmdAttr<CommandHSetNX>("hsetnx", -4, "write", 1, 1, 1),
MakeCmdAttr<CommandHDel>("hdel", -3, "write", 1, 1, 1),
MakeCmdAttr<CommandHStrlen>("hstrlen", 3, "read-only", 1, 1, 1),
MakeCmdAttr<CommandHExists>("hexists", 3, "read-only", 1, 1, 1),
Expand Down
27 changes: 16 additions & 11 deletions src/types/redis_hash.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@
#include <algorithm>
#include <cctype>
#include <cmath>
#include <limits>
#include <utility>

#include "db_util.h"
#include "parse_util.h"

namespace Redis {

rocksdb::Status Hash::GetMetadata(const Slice &ns_key, HashMetadata *metadata) {
return Database::GetMetadata(kRedisHash, ns_key, metadata);
}
Expand Down Expand Up @@ -173,12 +173,15 @@ rocksdb::Status Hash::MGet(const Slice &user_key, const std::vector<Slice> &fiel
LatestSnapShot ss(storage_);
rocksdb::ReadOptions read_options;
read_options.snapshot = ss.GetSnapShot();
storage_->SetReadOptions(read_options);

std::string sub_key, value;
for (const auto &field : fields) {
InternalKey(ns_key, field, metadata.version, storage_->IsSlotIdEncoded()).Encode(&sub_key);
value.clear();
auto s = storage_->Get(read_options, sub_key, &value);
s = storage_->Get(read_options, sub_key, &value);
if (!s.ok() && !s.IsNotFound()) return s;

values->emplace_back(value);
statuses->emplace_back(s);
}
Expand All @@ -192,13 +195,6 @@ rocksdb::Status Hash::Set(const Slice &user_key, const Slice &field, const Slice
return MSet(user_key, fvs, false, ret);
}

rocksdb::Status Hash::SetNX(const Slice &user_key, const Slice &field, Slice value, int *ret) {
FieldValue fv = {field.ToString(), value.ToString()};
std::vector<FieldValue> fvs;
fvs.emplace_back(std::move(fv));
return MSet(user_key, fvs, true, ret);
}

rocksdb::Status Hash::Delete(const Slice &user_key, const std::vector<Slice> &fields, int *ret) {
*ret = 0;
std::string ns_key;
Expand Down Expand Up @@ -246,29 +242,38 @@ rocksdb::Status Hash::MSet(const Slice &user_key, const std::vector<FieldValue>
auto batch = storage_->GetWriteBatchBase();
WriteBatchLogData log_data(kRedisHash);
batch->PutLogData(log_data.Encode());

for (const auto &fv : field_values) {
exists = false;

std::string sub_key;
InternalKey(ns_key, fv.field, metadata.version, storage_->IsSlotIdEncoded()).Encode(&sub_key);

if (metadata.size > 0) {
std::string fieldValue;
s = storage_->Get(rocksdb::ReadOptions(), sub_key, &fieldValue);
if (!s.ok() && !s.IsNotFound()) return s;

if (s.ok()) {
if (((fieldValue == fv.value) || nx)) continue;
if (nx || fieldValue == fv.value) continue;

exists = true;
}
}

if (!exists) added++;

batch->Put(sub_key, fv.value);
}

if (added > 0) {
*ret = added;
metadata.size += added;
std::string bytes;
metadata.Encode(&bytes);
batch->Put(metadata_cf_handle_, ns_key, bytes);
}

return storage_->Write(storage_->DefaultWriteOptions(), batch->GetWriteBatch());
}

Expand Down Expand Up @@ -330,7 +335,7 @@ rocksdb::Status Hash::RangeByLex(const Slice &user_key, const CommonRangeLexSpec
tmp_field_value.field = ikey.GetSubKey().ToString();
tmp_field_value.value = iter->value().ToString();
field_values->emplace_back(tmp_field_value);
if (spec.count > 0 && field_values && field_values->size() >= static_cast<unsigned>(spec.count)) break;
if (spec.count > 0 && field_values->size() >= static_cast<unsigned>(spec.count)) break;
}
return rocksdb::Status::OK();
}
Expand Down
4 changes: 3 additions & 1 deletion src/types/redis_hash.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ struct FieldValue {
enum class HashFetchType { kAll = 0, kOnlyKey = 1, kOnlyValue = 2 };

namespace Redis {

class Hash : public SubKeyScanner {
public:
Hash(Engine::Storage *storage, const std::string &ns) : SubKeyScanner(storage, ns) {}

rocksdb::Status Size(const Slice &user_key, uint32_t *ret);
rocksdb::Status Get(const Slice &user_key, const Slice &field, std::string *value);
rocksdb::Status Set(const Slice &user_key, const Slice &field, const Slice &value, int *ret);
rocksdb::Status SetNX(const Slice &user_key, const Slice &field, Slice value, int *ret);
rocksdb::Status Delete(const Slice &user_key, const std::vector<Slice> &fields, int *ret);
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);
Expand All @@ -62,4 +63,5 @@ class Hash : public SubKeyScanner {
private:
rocksdb::Status GetMetadata(const Slice &ns_key, HashMetadata *metadata);
};

} // namespace Redis
94 changes: 67 additions & 27 deletions tests/cppunit/t_hash_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@
#include "parse_util.h"
#include "test_base.h"
#include "types/redis_hash.h"

class RedisHashTest : public TestBase {
protected:
explicit RedisHashTest() : TestBase() { hash = std::make_unique<Redis::Hash>(storage_, "hash_ns"); }
~RedisHashTest() = default;
~RedisHashTest() override = default;

void SetUp() override {
key_ = "test_hash->key";
fields_ = {"test-hash-key-1", "test-hash-key-2", "test-hash-key-3"};
Expand All @@ -45,74 +47,111 @@ class RedisHashTest : public TestBase {
};

TEST_F(RedisHashTest, GetAndSet) {
int ret;
int ret = 0;
for (size_t i = 0; i < fields_.size(); i++) {
rocksdb::Status s = hash->Set(key_, fields_[i], values_[i], &ret);
auto s = hash->Set(key_, fields_[i], values_[i], &ret);
EXPECT_TRUE(s.ok() && ret == 1);
}
for (size_t i = 0; i < fields_.size(); i++) {
std::string got;
rocksdb::Status s = hash->Get(key_, fields_[i], &got);
auto s = hash->Get(key_, fields_[i], &got);
EXPECT_EQ(values_[i], got);
}
rocksdb::Status s = hash->Delete(key_, fields_, &ret);
auto s = hash->Delete(key_, fields_, &ret);
EXPECT_TRUE(s.ok() && static_cast<int>(fields_.size()) == ret);
hash->Del(key_);
}

TEST_F(RedisHashTest, MGetAndMSet) {
int ret;
int ret = 0;
std::vector<FieldValue> fvs;
for (size_t i = 0; i < fields_.size(); i++) {
fvs.emplace_back(FieldValue{fields_[i].ToString(), values_[i].ToString()});
}
rocksdb::Status s = hash->MSet(key_, fvs, false, &ret);
auto s = hash->MSet(key_, fvs, false, &ret);
EXPECT_TRUE(s.ok() && static_cast<int>(fvs.size()) == ret);
s = hash->MSet(key_, fvs, false, &ret);
EXPECT_TRUE(s.ok());
EXPECT_EQ(ret, 0);
std::vector<std::string> values;
std::vector<rocksdb::Status> statuses;
s = hash->MGet(key_, fields_, &values, &statuses);
EXPECT_TRUE(s.ok());
for (size_t i = 0; i < fields_.size(); i++) {
EXPECT_EQ(values[i], values_[i].ToString());
}
s = hash->Delete(key_, fields_, &ret);
EXPECT_TRUE(s.ok());
EXPECT_EQ(static_cast<int>(fields_.size()), ret);
hash->Del(key_);
}

TEST_F(RedisHashTest, SetNX) {
int ret;
Slice field("foo");
rocksdb::Status s = hash->Set(key_, field, "bar", &ret);
TEST_F(RedisHashTest, MSetSingleFieldAndNX) {
int ret = 0;
std::vector<FieldValue> values = {{"field-one", "value-one"}};
auto s = hash->MSet(key_, values, true, &ret);
EXPECT_TRUE(s.ok() && ret == 1);

std::string field2 = "field-two";
std::string initial_value = "value-two";
s = hash->Set(key_, field2, initial_value, &ret);
EXPECT_TRUE(s.ok() && ret == 1);
s = hash->Set(key_, field, "bar", &ret);

values = {{field2, "value-two-changed"}};
s = hash->MSet(key_, values, true, &ret);
EXPECT_TRUE(s.ok() && ret == 0);
std::vector<Slice> fields = {field};
s = hash->Delete(key_, fields, &ret);
EXPECT_EQ(fields.size(), ret);

std::string final_value;
s = hash->Get(key_, field2, &final_value);
EXPECT_TRUE(s.ok());
EXPECT_EQ(initial_value, final_value);

hash->Del(key_);
}

TEST_F(RedisHashTest, MSetMultipleFieldsAndNX) {
int ret = 0;
std::vector<FieldValue> values = {{"field-one", "value-one"}, {"field-two", "value-two"}};
auto s = hash->MSet(key_, values, true, &ret);
EXPECT_TRUE(s.ok() && ret == 2);

values = {{"field-one", "value-one"}, {"field-two", "value-two-changed"}, {"field-three", "value-three"}};
s = hash->MSet(key_, values, true, &ret);
EXPECT_TRUE(s.ok());
EXPECT_EQ(ret, 1);

std::string value;
s = hash->Get(key_, "field-one", &value);
EXPECT_TRUE(s.ok() && value == "value-one");

s = hash->Get(key_, "field-two", &value);
EXPECT_TRUE(s.ok() && value == "value-two");

s = hash->Get(key_, "field-three", &value);
EXPECT_TRUE(s.ok() && value == "value-three");

hash->Del(key_);
}

TEST_F(RedisHashTest, HGetAll) {
int ret;
int ret = 0;
for (size_t i = 0; i < fields_.size(); i++) {
rocksdb::Status s = hash->Set(key_, fields_[i], values_[i], &ret);
auto s = hash->Set(key_, fields_[i], values_[i], &ret);
EXPECT_TRUE(s.ok() && ret == 1);
}
std::vector<FieldValue> fvs;
rocksdb::Status s = hash->GetAll(key_, &fvs);
auto s = hash->GetAll(key_, &fvs);
EXPECT_TRUE(s.ok() && fvs.size() == fields_.size());
s = hash->Delete(key_, fields_, &ret);
EXPECT_TRUE(s.ok() && static_cast<int>(fields_.size()) == ret);
hash->Del(key_);
}

TEST_F(RedisHashTest, HIncr) {
int64_t value;
int64_t value = 0;
Slice field("hash-incrby-invalid-field");
for (int i = 0; i < 32; i++) {
rocksdb::Status s = hash->IncrBy(key_, field, 1, &value);
auto s = hash->IncrBy(key_, field, 1, &value);
EXPECT_TRUE(s.ok());
}
std::string bytes;
Expand All @@ -126,10 +165,10 @@ TEST_F(RedisHashTest, HIncr) {
}

TEST_F(RedisHashTest, HIncrInvalid) {
int ret;
int64_t value;
int ret = 0;
int64_t value = 0;
Slice field("hash-incrby-invalid-field");
rocksdb::Status s = hash->IncrBy(key_, field, 1, &value);
auto s = hash->IncrBy(key_, field, 1, &value);
EXPECT_TRUE(s.ok() && value == 1);

s = hash->IncrBy(key_, field, LLONG_MAX, &value);
Expand All @@ -148,10 +187,10 @@ TEST_F(RedisHashTest, HIncrInvalid) {
}

TEST_F(RedisHashTest, HIncrByFloat) {
double value;
double value = 0.0;
Slice field("hash-incrbyfloat-invalid-field");
for (int i = 0; i < 32; i++) {
rocksdb::Status s = hash->IncrByFloat(key_, field, 1.2, &value);
auto s = hash->IncrByFloat(key_, field, 1.2, &value);
EXPECT_TRUE(s.ok());
}
std::string bytes;
Expand All @@ -176,9 +215,10 @@ TEST_F(RedisHashTest, HRangeByLex) {
std::vector<FieldValue> tmp(fvs);
for (size_t i = 0; i < 100; i++) {
std::shuffle(tmp.begin(), tmp.end(), g);
rocksdb::Status s = hash->MSet(key_, tmp, false, &ret);
auto s = hash->MSet(key_, tmp, false, &ret);
EXPECT_TRUE(s.ok() && static_cast<int>(tmp.size()) == ret);
s = hash->MSet(key_, fvs, false, &ret);
EXPECT_TRUE(s.ok());
EXPECT_EQ(ret, 0);
std::vector<FieldValue> result;
CommonRangeLexSpec spec;
Expand All @@ -196,7 +236,7 @@ TEST_F(RedisHashTest, HRangeByLex) {
hash->Del(key_);
}

rocksdb::Status s = hash->MSet(key_, tmp, false, &ret);
auto 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;
Expand Down
Loading

0 comments on commit b69e72c

Please sign in to comment.