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

fix: add type.tcl and quit.tcl files, fix some bug #398

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 1 addition & 1 deletion src/base_cmd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ BaseCmd* BaseCmdGroup::GetSubCmd(const std::string& cmdName) {
bool BaseCmdGroup::DoInitial(PClient* client) {
client->SetSubCmdName(client->argv_[1]);
if (!subCmds_.contains(client->SubCmdName())) {
client->SetRes(CmdRes::kSyntaxErr, client->argv_[0] + " unknown subcommand for '" + client->SubCmdName() + "'");
client->SetRes(CmdRes::kErrOther, client->argv_[0] + " unknown subcommand for '" + client->SubCmdName() + "'");
return false;
}
return true;
Expand Down
2 changes: 2 additions & 0 deletions src/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class CmdRes {
kInvalidDB,
kInconsistentHashTag,
kErrOther,
kUnknownCmd,
kUnknownSubCmd,
KIncrByOverFlow,
kInvalidCursor,
kWrongLeader,
Expand Down
2 changes: 1 addition & 1 deletion src/cmd_keys.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ bool TypeCmd::DoInitial(PClient* client) {
}

void TypeCmd::DoCmd(PClient* client) {
storage::DataType type;
storage::DataType type = storage::DataType::kNones;
rocksdb::Status s = PSTORE.GetBackend(client->GetCurrentDB())->GetStorage()->GetType(client->Key(), type);
if (s.ok()) {
client->AppendContent("+" + std::string(storage::DataTypeToString(type)));
Expand Down
7 changes: 4 additions & 3 deletions src/cmd_table_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "cmd_raft.h"
#include "cmd_set.h"
#include "cmd_zset.h"
#include "pstd_string.h"

namespace pikiwidb {

Expand Down Expand Up @@ -190,16 +191,16 @@ std::pair<BaseCmd*, CmdRes::CmdRet> CmdTableManager::GetCommand(const std::strin
auto cmd = cmds_->find(cmdName);

if (cmd == cmds_->end()) {
return std::pair(nullptr, CmdRes::kSyntaxErr);
return std::pair(nullptr, CmdRes::kUnknownCmd);
}

if (cmd->second->HasSubCommand()) {
if (client->argv_.size() < 2) {
return std::pair(nullptr, CmdRes::kInvalidParameter);
}
return std::pair(cmd->second->GetSubCmd(client->argv_[1]), CmdRes::kSyntaxErr);
return std::pair(cmd->second->GetSubCmd(pstd::StringToLower(client->argv_[1])), CmdRes::kUnknownSubCmd);
}
return std::pair(cmd->second.get(), CmdRes::kSyntaxErr);
return std::pair(cmd->second.get(), CmdRes::kOK);
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里为什么返回改为OK了,是不是原来的逻辑是错的

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这个方法的逻辑,前面把所有的异常逻辑都 retuen,如果走到最后,应该是正常的 case

}

bool CmdTableManager::CmdExist(const std::string& cmd) const {
Expand Down
8 changes: 5 additions & 3 deletions src/cmd_thread_pool_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ void CmdWorkThreadPoolWorker::Work() {
auto [cmdPtr, ret] = cmd_table_manager_.GetCommand(task->CmdName(), task->Client().get());

if (!cmdPtr) {
if (ret == CmdRes::kInvalidParameter) {
task->Client()->SetRes(CmdRes::kInvalidParameter);
if (ret == CmdRes::kUnknownCmd) {
task->Client()->SetRes(CmdRes::kErrOther, "unknown command '" + task->CmdName() + "'");
} else if (ret == CmdRes::kUnknownSubCmd) {
task->Client()->SetRes(CmdRes::kErrOther, "unknown sub command '" + task->Client().get()->argv_[1] + "'");
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里是不是应该需要检查一下task->Client().get()中argv[]的大小,避免访问非法内存地址

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image
如果满足 ret == CmdRes::kUnknownSubCmd ,说明一定有多个参数

} else {
task->Client()->SetRes(CmdRes::kSyntaxErr, "unknown command '" + task->CmdName() + "'");
task->Client()->SetRes(CmdRes::kInvalidParameter);
}
g_pikiwidb->PushWriteTask(task->Client());
continue;
Expand Down
4 changes: 2 additions & 2 deletions tests/test_helper.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ set ::all_tests {
# unit/basic
# unit/scan
# unit/multi
# unit/quit
unit/quit
# unit/type/list
# unit/pubsub
# unit/slowlog
# unit/maxmemory
# unit/bitops
# unit/hyperloglog
# unit/type
unit/type
# unit/acl
# unit/type/list-2
# unit/type/list-3
Expand Down
5 changes: 1 addition & 4 deletions tests/unit/command.tcl
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
# Copyright (c) 2023-present, Qihoo, Inc. All rights reserved.
# This source code is licensed under the BSD-style license found in the
# LICENSE file in the root directory of this source tree. An additional grant
# of patent rights can be found in the PATENTS file in the same directory.
# Pikiwidb does not support the docs command

start_server {tags {"command"}} {
test "Command docs supported." {
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/dump.tcl
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Pikiwidb does not support the restore command

start_server {tags {"dump"}} {
test {DUMP / RESTORE are able to serialize / unserialize a simple key} {
r set foo bar
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/geo.tcl
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Pikiwidb does not support the geo command

# Helper functions to simulate search-in-radius in the Tcl side in order to
# verify the Redis implementation with a fuzzy test.
proc geo_degrad deg {expr {$deg*atan(1)*8/360}}
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/hyperloglog.tcl
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Pikiwidb does not support the pfadd command

start_server {tags {"hll"}} {
# test {HyperLogLog self test passes} {
# catch {r pfselftest} e
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/introspection.tcl
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# Pikiwidb does not support the client command

start_server {tags {"introspection"}} {
test {CLIENT LIST} {
r client list
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/latency-monitor.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ start_server {tags {"latency-monitor"}} {
r config set latency-monitor-threshold 200
r latency reset

# This parameter is not available in Pika
test {Test latency events logging} {
r debug sleep 0.3
after 1100
Expand All @@ -12,6 +13,7 @@ start_server {tags {"latency-monitor"}} {
assert {[r latency history command] >= 3}
}

# This parameter is not available in Pika
test {LATENCY HISTORY output is ok} {
set min 250
set max 450
Expand All @@ -24,6 +26,7 @@ start_server {tags {"latency-monitor"}} {
}
}

# This parameter is not available in Pika
test {LATENCY LATEST output is ok} {
foreach event [r latency latest] {
lassign $event eventname time latency max
Expand All @@ -34,15 +37,18 @@ start_server {tags {"latency-monitor"}} {
}
}

# This parameter is not available in Pika
test {LATENCY HISTORY / RESET with wrong event name is fine} {
assert {[llength [r latency history blabla]] == 0}
assert {[r latency reset blabla] == 0}
}

# This parameter is not available in Pika
test {LATENCY DOCTOR produces some output} {
assert {[string length [r latency doctor]] > 0}
}

# This parameter is not available in Pika
test {LATENCY RESET is able to reset events} {
assert {[r latency reset] > 0}
assert {[r latency latest] eq {}}
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/maxmemory.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,22 @@ start_server {tags {"maxmemory"}} {
# The current maxmemory command does not support config set and policy.
# For a complete list of commands, refer to the wiki: https://github.com/OpenAtomFoundation/pika/wiki/pika-%E5%B7%AE%E5%BC%82%E5%8C%96%E5%91%BD%E4%BB%A4

# This parameter is not available in Pika
# test "Without maxmemory small integers are shared" {
# r config set maxmemory 0
# r set a 1
# assert {[r object refcount a] > 1}
# }

# This parameter is not available in Pika
# test "With maxmemory and non-LRU policy integers are still shared" {
# r config set maxmemory 1073741824
# r config set maxmemory-policy allkeys-random
# r set a 1
# assert {[r object refcount a] > 1}
# }

# This parameter is not available in Pika
# test "With maxmemory and LRU policy integers are not shared" {
# r config set maxmemory 1073741824
# r config set maxmemory-policy allkeys-lru
Expand All @@ -31,6 +34,7 @@ start_server {tags {"maxmemory"}} {
# r config set maxmemory 0
# }

# This parameter is not available in Pika
# foreach policy {
# allkeys-random allkeys-lru volatile-lru volatile-random volatile-ttl
# } {
Expand Down Expand Up @@ -63,6 +67,7 @@ start_server {tags {"maxmemory"}} {
# }
# }

# This parameter is not available in Pika
# foreach policy {
# allkeys-random allkeys-lru volatile-lru volatile-random volatile-ttl
# } {
Expand Down Expand Up @@ -105,6 +110,7 @@ start_server {tags {"maxmemory"}} {
# }
# }

# This parameter is not available in Pika
# foreach policy {
# volatile-lru volatile-random volatile-ttl
# } {
Expand Down
48 changes: 25 additions & 23 deletions tests/unit/type.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -24,27 +24,29 @@ start_server {tags {"type"}} {
assert_equal set [r type key5]
}

test "ptype none" {
r flushdb
assert_equal {} [r ptype key]
}

test "ptype command" {
r flushdb

r set key1 key1
assert_equal string [r ptype key1]

r hset key1 key key1
assert_equal {string hash} [r ptype key1]

r lpush key1 key1
assert_equal {string hash list} [r ptype key1]

r zadd key1 100 key1
assert_equal {string hash list zset} [r ptype key1]

r sadd key1 key1
assert_equal {string hash list zset set} [r ptype key1]
}
#Pikiwidb does not support the ptype command
Copy link
Collaborator

Choose a reason for hiding this comment

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

ptype这个可以直接删掉了,这个是之前多key版本留下来的测试

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

#test "ptype none" {
# r flushdb
# assert_equal {} [r ptype key]
#}

#Pikiwidb does not support the ptype command
#test "ptype command" {
# r flushdb
#
# r set key1 key1
# assert_equal string [r ptype key1]
#
# r hset key1 key key1
# assert_equal {string hash} [r ptype key1]
#
# r lpush key1 key1
# assert_equal {string hash list} [r ptype key1]
#
# r zadd key1 100 key1
# assert_equal {string hash list zset} [r ptype key1]
#
# r sadd key1 key1
# assert_equal {string hash list zset set} [r ptype key1]
#}
}
Loading