From a12ddfe108b72a134128d8060b95daa4aed78a30 Mon Sep 17 00:00:00 2001 From: Vladislav Date: Mon, 10 Apr 2023 14:14:52 +0300 Subject: [PATCH] Remove cmd name from args (#1057) chore: remove cmd name from the list of arguments Signed-off-by: Vladislav Oleshko Co-authored-by: Roman Gershman --- src/server/bitops_family.cc | 48 +++++----- src/server/command_registry.cc | 6 +- src/server/common.h | 12 +-- src/server/debugcmd.cc | 34 +++---- src/server/dflycmd.cc | 38 ++++---- src/server/generic_family.cc | 73 +++++++------- src/server/hset_family.cc | 78 ++++++++------- src/server/journal/serializer.cc | 24 ++--- src/server/journal/serializer.h | 5 +- src/server/journal/types.h | 6 +- src/server/journal_test.cc | 26 ++--- src/server/json_family.cc | 136 +++++++++++++-------------- src/server/json_family_test.cc | 9 ++ src/server/list_family.cc | 97 ++++++++++--------- src/server/main_service.cc | 79 +++++++--------- src/server/memory_cmd.cc | 2 +- src/server/multi_command_squasher.cc | 2 +- src/server/server_family.cc | 69 +++++++------- src/server/set_family.cc | 66 ++++++------- src/server/stream_family.cc | 66 ++++++------- src/server/string_family.cc | 82 ++++++++-------- src/server/test_utils.cc | 8 +- src/server/transaction.cc | 59 ++++++------ src/server/transaction.h | 2 +- src/server/zset_family.cc | 110 +++++++++++----------- 25 files changed, 564 insertions(+), 573 deletions(-) diff --git a/src/server/bitops_family.cc b/src/server/bitops_family.cc index e577a81d4..17a7dd4e5 100644 --- a/src/server/bitops_family.cc +++ b/src/server/bitops_family.cc @@ -508,32 +508,32 @@ void BitPos(CmdArgList args, ConnectionContext* cntx) { // Support for the command BITPOS // See details at https://redis.io/commands/bitpos/ - if (args.size() < 2 || args.size() > 6) { + if (args.size() < 1 || args.size() > 5) { return (*cntx)->SendError(kSyntaxErr); } - std::string_view key = ArgS(args, 1); + std::string_view key = ArgS(args, 0); int32_t value{0}; int64_t start = 0; int64_t end = std::numeric_limits::max(); bool as_bit = false; - if (!absl::SimpleAtoi(ArgS(args, 2), &value)) { + if (!absl::SimpleAtoi(ArgS(args, 1), &value)) { return (*cntx)->SendError(kInvalidIntErr); } - if (args.size() >= 4) { - if (!absl::SimpleAtoi(ArgS(args, 3), &start)) { + if (args.size() >= 3) { + if (!absl::SimpleAtoi(ArgS(args, 2), &start)) { return (*cntx)->SendError(kInvalidIntErr); } - if (args.size() >= 5) { - if (!absl::SimpleAtoi(ArgS(args, 4), &end)) { + if (args.size() >= 4) { + if (!absl::SimpleAtoi(ArgS(args, 3), &end)) { return (*cntx)->SendError(kInvalidIntErr); } - if (args.size() >= 6) { - if (!ToUpperAndGetAsBit(args, 5, &as_bit)) { + if (args.size() >= 5) { + if (!ToUpperAndGetAsBit(args, 4, &as_bit)) { return (*cntx)->SendError(kSyntaxErr); } } @@ -553,21 +553,21 @@ void BitCount(CmdArgList args, ConnectionContext* cntx) { // See details at https://redis.io/commands/bitcount/ // Please note that if the key don't exists, it would return 0 - if (args.size() == 3 || args.size() > 5) { + if (args.size() == 2 || args.size() > 4) { return (*cntx)->SendError(kSyntaxErr); } // return (*cntx)->SendLong(0); - std::string_view key = ArgS(args, 1); + std::string_view key = ArgS(args, 0); bool as_bit = false; int64_t start = 0; int64_t end = std::numeric_limits::max(); - if (args.size() >= 4) { - if (absl::SimpleAtoi(ArgS(args, 2), &start) == 0 || - absl::SimpleAtoi(ArgS(args, 3), &end) == 0) { + if (args.size() >= 3) { + if (absl::SimpleAtoi(ArgS(args, 1), &start) == 0 || + absl::SimpleAtoi(ArgS(args, 2), &end) == 0) { return (*cntx)->SendError(kInvalidIntErr); } - if (args.size() == 5) { - if (!ToUpperAndGetAsBit(args, 4, &as_bit)) { + if (args.size() == 4) { + if (!ToUpperAndGetAsBit(args, 3, &as_bit)) { return (*cntx)->SendError(kSyntaxErr); } } @@ -591,13 +591,13 @@ void BitFieldRo(CmdArgList args, ConnectionContext* cntx) { void BitOp(CmdArgList args, ConnectionContext* cntx) { static const std::array BITOP_OP_NAMES{OR_OP_NAME, XOR_OP_NAME, AND_OP_NAME, NOT_OP_NAME}; - ToUpper(&args[1]); - std::string_view op = ArgS(args, 1); - std::string_view dest_key = ArgS(args, 2); + ToUpper(&args[0]); + std::string_view op = ArgS(args, 0); + std::string_view dest_key = ArgS(args, 1); bool illegal = std::none_of(BITOP_OP_NAMES.begin(), BITOP_OP_NAMES.end(), [&op](auto val) { return op == val; }); - if (illegal || (op == NOT_OP_NAME && args.size() > 4)) { + if (illegal || (op == NOT_OP_NAME && args.size() > 3)) { return (*cntx)->SendError(kSyntaxErr); // too many arguments } @@ -658,9 +658,9 @@ void GetBit(CmdArgList args, ConnectionContext* cntx) { // see https://redis.io/commands/getbit/ uint32_t offset{0}; - std::string_view key = ArgS(args, 1); + std::string_view key = ArgS(args, 0); - if (!absl::SimpleAtoi(ArgS(args, 2), &offset)) { + if (!absl::SimpleAtoi(ArgS(args, 1), &offset)) { return (*cntx)->SendError(kInvalidIntErr); } auto cb = [&](Transaction* t, EngineShard* shard) { @@ -677,9 +677,9 @@ void SetBit(CmdArgList args, ConnectionContext* cntx) { uint32_t offset{0}; int32_t value{0}; - std::string_view key = ArgS(args, 1); + std::string_view key = ArgS(args, 0); - if (!absl::SimpleAtoi(ArgS(args, 2), &offset) || !absl::SimpleAtoi(ArgS(args, 3), &value)) { + if (!absl::SimpleAtoi(ArgS(args, 1), &offset) || !absl::SimpleAtoi(ArgS(args, 2), &value)) { return (*cntx)->SendError(kInvalidIntErr); } diff --git a/src/server/command_registry.cc b/src/server/command_registry.cc index c83d3138f..9315145f6 100644 --- a/src/server/command_registry.cc +++ b/src/server/command_registry.cc @@ -53,9 +53,9 @@ CommandRegistry::CommandRegistry() { } void CommandRegistry::Command(CmdArgList args, ConnectionContext* cntx) { - if (args.size() > 1) { - ToUpper(&args[1]); - string_view subcmd = ArgS(args, 1); + if (args.size() > 0) { + ToUpper(&args[0]); + string_view subcmd = ArgS(args, 0); if (subcmd == "COUNT") { return (*cntx)->SendLong(cmd_map_.size()); } else { diff --git a/src/server/common.h b/src/server/common.h index 1fba24bbc..785d95a2d 100644 --- a/src/server/common.h +++ b/src/server/common.h @@ -61,24 +61,24 @@ struct KeyIndex { unsigned end; // does not include this index (open limit). unsigned step; // 1 for commands like mget. 2 for commands like mset. - // if index is non-zero then adds another key index (usually 1). + // if index is non-zero then adds another key index (usually 0). // relevant for for commands like ZUNIONSTORE/ZINTERSTORE for destination key. - unsigned bonus = 0; + std::optional bonus{}; static KeyIndex Empty() { - return KeyIndex{0, 0, 0, 0}; + return KeyIndex{0, 0, 0, std::nullopt}; } static KeyIndex Range(unsigned start, unsigned end, unsigned step = 1) { - return KeyIndex{start, end, step, 0}; + return KeyIndex{start, end, step, std::nullopt}; } bool HasSingleKey() const { - return bonus == 0 && (start + step >= end); + return !bonus && (start + step >= end); } unsigned num_args() const { - return end - start + (bonus > 0); + return end - start + bool(bonus); } }; diff --git a/src/server/debugcmd.cc b/src/server/debugcmd.cc index 575a66b4f..20e951940 100644 --- a/src/server/debugcmd.cc +++ b/src/server/debugcmd.cc @@ -87,7 +87,7 @@ DebugCmd::DebugCmd(ServerFamily* owner, ConnectionContext* cntx) : sf_(*owner), } void DebugCmd::Run(CmdArgList args) { - string_view subcmd = ArgS(args, 1); + string_view subcmd = ArgS(args, 0); if (subcmd == "HELP") { string_view help_arr[] = { "DEBUG [ [value] [opt] ...]. Subcommands are:", @@ -128,7 +128,7 @@ void DebugCmd::Run(CmdArgList args) { return Reload(args); } - if (subcmd == "REPLICA" && args.size() == 3) { + if (subcmd == "REPLICA" && args.size() == 2) { return Replica(args); } @@ -136,12 +136,12 @@ void DebugCmd::Run(CmdArgList args) { return Watched(); } - if (subcmd == "LOAD" && args.size() == 3) { - return Load(ArgS(args, 2)); + if (subcmd == "LOAD" && args.size() == 2) { + return Load(ArgS(args, 1)); } - if (subcmd == "OBJECT" && args.size() == 3) { - string_view key = ArgS(args, 2); + if (subcmd == "OBJECT" && args.size() == 2) { + string_view key = ArgS(args, 1); return Inspect(key); } @@ -156,7 +156,7 @@ void DebugCmd::Run(CmdArgList args) { void DebugCmd::Reload(CmdArgList args) { bool save = true; - for (size_t i = 2; i < args.size(); ++i) { + for (size_t i = 1; i < args.size(); ++i) { ToUpper(&args[i]); string_view opt = ArgS(args, i); VLOG(1) << "opt " << opt; @@ -187,7 +187,7 @@ void DebugCmd::Reload(CmdArgList args) { } void DebugCmd::Replica(CmdArgList args) { - args.remove_prefix(2); + args.remove_prefix(1); ToUpper(&args[0]); string_view opt = ArgS(args, 0); @@ -254,29 +254,29 @@ void DebugCmd::Load(string_view filename) { } void DebugCmd::Populate(CmdArgList args) { - if (args.size() < 3 || args.size() > 6) { + if (args.size() < 2 || args.size() > 5) { return (*cntx_)->SendError(UnknownSubCmd("populate", "DEBUG")); } uint64_t total_count = 0; - if (!absl::SimpleAtoi(ArgS(args, 2), &total_count)) + if (!absl::SimpleAtoi(ArgS(args, 1), &total_count)) return (*cntx_)->SendError(kUintErr); std::string_view prefix{"key"}; - if (args.size() > 3) { - prefix = ArgS(args, 3); + if (args.size() > 2) { + prefix = ArgS(args, 2); } uint32_t val_size = 0; - if (args.size() > 4) { - std::string_view str = ArgS(args, 4); + if (args.size() > 3) { + std::string_view str = ArgS(args, 3); if (!absl::SimpleAtoi(str, &val_size)) return (*cntx_)->SendError(kUintErr); } bool populate_random_values = false; - if (args.size() > 5) { - ToUpper(&args[5]); - std::string_view str = ArgS(args, 5); + if (args.size() > 4) { + ToUpper(&args[4]); + std::string_view str = ArgS(args, 4); if (str != "RAND") { return (*cntx_)->SendError(kSyntaxErr); } diff --git a/src/server/dflycmd.cc b/src/server/dflycmd.cc index 91e100023..cb16da203 100644 --- a/src/server/dflycmd.cc +++ b/src/server/dflycmd.cc @@ -88,11 +88,11 @@ DflyCmd::DflyCmd(util::ListenerInterface* listener, ServerFamily* server_family) void DflyCmd::Run(CmdArgList args, ConnectionContext* cntx) { RedisReplyBuilder* rb = static_cast(cntx->reply_builder()); - DCHECK_GE(args.size(), 2u); - ToUpper(&args[1]); - string_view sub_cmd = ArgS(args, 1); + DCHECK_GE(args.size(), 1u); + ToUpper(&args[0]); + string_view sub_cmd = ArgS(args, 0); - if (sub_cmd == "JOURNAL" && args.size() >= 3) { + if (sub_cmd == "JOURNAL" && args.size() >= 2) { return Journal(args, cntx); } @@ -100,15 +100,15 @@ void DflyCmd::Run(CmdArgList args, ConnectionContext* cntx) { return Thread(args, cntx); } - if (sub_cmd == "FLOW" && args.size() == 5) { + if (sub_cmd == "FLOW" && args.size() == 4) { return Flow(args, cntx); } - if (sub_cmd == "SYNC" && args.size() == 3) { + if (sub_cmd == "SYNC" && args.size() == 2) { return Sync(args, cntx); } - if (sub_cmd == "STARTSTABLE" && args.size() == 3) { + if (sub_cmd == "STARTSTABLE" && args.size() == 2) { return StartStable(args, cntx); } @@ -116,7 +116,7 @@ void DflyCmd::Run(CmdArgList args, ConnectionContext* cntx) { return Expire(args, cntx); } - if (sub_cmd == "REPLICAOFFSET" && args.size() == 3) { + if (sub_cmd == "REPLICAOFFSET" && args.size() == 2) { return ReplicaOffset(args, cntx); } @@ -124,10 +124,10 @@ void DflyCmd::Run(CmdArgList args, ConnectionContext* cntx) { } void DflyCmd::Journal(CmdArgList args, ConnectionContext* cntx) { - DCHECK_GE(args.size(), 3u); - ToUpper(&args[2]); + DCHECK_GE(args.size(), 2u); + ToUpper(&args[1]); - std::string_view sub_cmd = ArgS(args, 2); + std::string_view sub_cmd = ArgS(args, 1); Transaction* trans = cntx->transaction; DCHECK(trans); RedisReplyBuilder* rb = static_cast(cntx->reply_builder()); @@ -190,7 +190,7 @@ void DflyCmd::Thread(CmdArgList args, ConnectionContext* cntx) { RedisReplyBuilder* rb = static_cast(cntx->reply_builder()); util::ProactorPool* pool = shard_set->pool(); - if (args.size() == 2) { // DFLY THREAD : returns connection thread index and number of threads. + if (args.size() == 1) { // DFLY THREAD : returns connection thread index and number of threads. rb->StartArray(2); rb->SendLong(ProactorBase::GetIndex()); rb->SendLong(long(pool->size())); @@ -198,7 +198,7 @@ void DflyCmd::Thread(CmdArgList args, ConnectionContext* cntx) { } // DFLY THREAD to_thread : migrates current connection to a different thread. - string_view arg = ArgS(args, 2); + string_view arg = ArgS(args, 1); unsigned num_thread; if (!absl::SimpleAtoi(arg, &num_thread)) { return rb->SendError(kSyntaxErr); @@ -217,9 +217,9 @@ void DflyCmd::Thread(CmdArgList args, ConnectionContext* cntx) { void DflyCmd::Flow(CmdArgList args, ConnectionContext* cntx) { RedisReplyBuilder* rb = static_cast(cntx->reply_builder()); - string_view master_id = ArgS(args, 2); - string_view sync_id_str = ArgS(args, 3); - string_view flow_id_str = ArgS(args, 4); + string_view master_id = ArgS(args, 1); + string_view sync_id_str = ArgS(args, 2); + string_view flow_id_str = ArgS(args, 3); VLOG(1) << "Got DFLY FLOW " << master_id << " " << sync_id_str << " " << flow_id_str; @@ -259,7 +259,7 @@ void DflyCmd::Flow(CmdArgList args, ConnectionContext* cntx) { void DflyCmd::Sync(CmdArgList args, ConnectionContext* cntx) { RedisReplyBuilder* rb = static_cast(cntx->reply_builder()); - string_view sync_id_str = ArgS(args, 2); + string_view sync_id_str = ArgS(args, 1); VLOG(1) << "Got DFLY SYNC " << sync_id_str; @@ -297,7 +297,7 @@ void DflyCmd::Sync(CmdArgList args, ConnectionContext* cntx) { void DflyCmd::StartStable(CmdArgList args, ConnectionContext* cntx) { RedisReplyBuilder* rb = static_cast(cntx->reply_builder()); - string_view sync_id_str = ArgS(args, 2); + string_view sync_id_str = ArgS(args, 1); VLOG(1) << "Got DFLY STARTSTABLE " << sync_id_str; @@ -346,7 +346,7 @@ void DflyCmd::Expire(CmdArgList args, ConnectionContext* cntx) { void DflyCmd::ReplicaOffset(CmdArgList args, ConnectionContext* cntx) { RedisReplyBuilder* rb = static_cast(cntx->reply_builder()); - string_view sync_id_str = ArgS(args, 2); + string_view sync_id_str = ArgS(args, 1); VLOG(1) << "Got DFLY REPLICAOFFSET " << sync_id_str; auto [sync_id, replica_ptr] = GetReplicaInfoOrReply(sync_id_str, rb); diff --git a/src/server/generic_family.cc b/src/server/generic_family.cc index 88f6dfd89..501ca5994 100644 --- a/src/server/generic_family.cc +++ b/src/server/generic_family.cc @@ -214,15 +214,14 @@ class RestoreArgs { } // The structure that we are expecting is: -// args[0] == "RESTORE" -// args[1] == "key" -// args[2] == "ttl" -// args[3] == serialized value (list of chars that are used for the actual restore). -// args[4] .. args[n]: optional arguments that can be [REPLACE] [ABSTTL] [IDLETIME seconds] +// args[0] == "key" +// args[1] == "ttl" +// args[2] == serialized value (list of chars that are used for the actual restore). +// args[3] .. args[n]: optional arguments that can be [REPLACE] [ABSTTL] [IDLETIME seconds] // [FREQ frequency], in any order OpResult RestoreArgs::TryFrom(const CmdArgList& args) { RestoreArgs out_args; - std::string_view cur_arg = ArgS(args, 2); // extract ttl + std::string_view cur_arg = ArgS(args, 1); // extract ttl if (!absl::SimpleAtoi(cur_arg, &out_args.expiration_) || (out_args.expiration_ < 0)) { return OpStatus::INVALID_INT; } @@ -234,7 +233,7 @@ OpResult RestoreArgs::TryFrom(const CmdArgList& args) { // we would parse them and ensure that they are correct, maybe later they will be used int64_t idle_time = 0; - for (size_t i = 4; i < args.size(); ++i) { + for (size_t i = 3; i < args.size(); ++i) { ToUpper(&args[i]); cur_arg = ArgS(args, i); bool additional = args.size() - i - 1 >= 1; @@ -635,7 +634,7 @@ void GenericFamily::Shutdown() { void GenericFamily::Del(CmdArgList args, ConnectionContext* cntx) { Transaction* transaction = cntx->transaction; - VLOG(1) << "Del " << ArgS(args, 1); + VLOG(1) << "Del " << ArgS(args, 0); atomic_uint32_t result{0}; bool is_mc = cntx->protocol() == Protocol::MEMCACHE; @@ -669,16 +668,16 @@ void GenericFamily::Del(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::Ping(CmdArgList args, ConnectionContext* cntx) { - if (args.size() > 2) { + if (args.size() > 1) { return (*cntx)->SendError(facade::WrongNumArgsError("ping"), kSyntaxErrType); } // We synchronously block here until the engine sends us the payload and notifies that // the I/O operation has been processed. - if (args.size() == 1) { + if (args.size() == 0) { return (*cntx)->SendSimpleString("PONG"); } else { - string_view arg = ArgS(args, 1); + string_view arg = ArgS(args, 0); DVLOG(2) << "Ping " << arg; return (*cntx)->SendBulkString(arg); @@ -687,7 +686,7 @@ void GenericFamily::Ping(CmdArgList args, ConnectionContext* cntx) { void GenericFamily::Exists(CmdArgList args, ConnectionContext* cntx) { Transaction* transaction = cntx->transaction; - VLOG(1) << "Exists " << ArgS(args, 1); + VLOG(1) << "Exists " << ArgS(args, 0); atomic_uint32_t result{0}; @@ -706,7 +705,7 @@ void GenericFamily::Exists(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::Persist(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) { return OpPersist(t->GetOpArgs(shard), key); }; @@ -715,8 +714,8 @@ void GenericFamily::Persist(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::Expire(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view sec = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view sec = ArgS(args, 1); int64_t int_arg; if (!absl::SimpleAtoi(sec, &int_arg)) { @@ -740,8 +739,8 @@ void GenericFamily::Expire(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::ExpireAt(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view sec = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view sec = ArgS(args, 1); int64_t int_arg; if (!absl::SimpleAtoi(sec, &int_arg)) { @@ -764,7 +763,7 @@ void GenericFamily::ExpireAt(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::Keys(CmdArgList args, ConnectionContext* cntx) { - string_view pattern(ArgS(args, 1)); + string_view pattern(ArgS(args, 0)); uint64_t cursor = 0; StringVec keys; @@ -785,8 +784,8 @@ void GenericFamily::Keys(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::PexpireAt(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view msec = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view msec = ArgS(args, 1); int64_t int_arg; if (!absl::SimpleAtoi(msec, &int_arg)) { @@ -808,8 +807,8 @@ void GenericFamily::PexpireAt(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::Pexpire(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view msec = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view msec = ArgS(args, 1); int64_t int_arg; if (!absl::SimpleAtoi(msec, &int_arg)) { @@ -832,7 +831,7 @@ void GenericFamily::Pexpire(CmdArgList args, ConnectionContext* cntx) { void GenericFamily::Stick(CmdArgList args, ConnectionContext* cntx) { Transaction* transaction = cntx->transaction; - VLOG(1) << "Stick " << ArgS(args, 1); + VLOG(1) << "Stick " << ArgS(args, 0); atomic_uint32_t result{0}; @@ -953,12 +952,12 @@ OpResultTyped OpFetchSortEntries(const OpArgs& op_args, std::stri } void GenericFamily::Sort(CmdArgList args, ConnectionContext* cntx) { - std::string_view key = ArgS(args, 1); + std::string_view key = ArgS(args, 0); bool alpha = false; bool reversed = false; std::optional> bounds; - for (size_t i = 2; i < args.size(); i++) { + for (size_t i = 1; i < args.size(); i++) { ToUpper(&args[i]); std::string_view arg = ArgS(args, i); @@ -1024,8 +1023,8 @@ void GenericFamily::Sort(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::Restore(CmdArgList args, ConnectionContext* cntx) { - std::string_view key = ArgS(args, 1); - std::string_view serialized_value = ArgS(args, 3); + std::string_view key = ArgS(args, 0); + std::string_view serialized_value = ArgS(args, 2); if (!VerifyFooter(serialized_value)) { return (*cntx)->SendError("ERR DUMP payload version or checksum are wrong"); @@ -1065,8 +1064,8 @@ void GenericFamily::Restore(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::Move(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view target_db_sv = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view target_db_sv = ArgS(args, 1); int64_t target_db; if (!absl::SimpleAtoi(target_db_sv, &target_db)) { @@ -1129,7 +1128,7 @@ void GenericFamily::Pttl(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::TtlGeneric(CmdArgList args, ConnectionContext* cntx, TimeUnit unit) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) { return OpTtl(t, shard, key); }; OpResult result = cntx->transaction->ScheduleSingleHopT(std::move(cb)); @@ -1153,7 +1152,7 @@ void GenericFamily::TtlGeneric(CmdArgList args, ConnectionContext* cntx, TimeUni } void GenericFamily::Select(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); int64_t index; if (!absl::SimpleAtoi(key, &index)) { return (*cntx)->SendError(kInvalidDbIndErr); @@ -1172,7 +1171,7 @@ void GenericFamily::Select(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::Dump(CmdArgList args, ConnectionContext* cntx) { - std::string_view key = ArgS(args, 1); + std::string_view key = ArgS(args, 0); DVLOG(1) << "Dumping before ::ScheduleSingleHopT " << key; auto cb = [&](Transaction* t, EngineShard* shard) { return OpDump(t->GetOpArgs(shard), key); }; @@ -1189,7 +1188,7 @@ void GenericFamily::Dump(CmdArgList args, ConnectionContext* cntx) { } void GenericFamily::Type(CmdArgList args, ConnectionContext* cntx) { - std::string_view key = ArgS(args, 1); + std::string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) -> OpResult { auto it = shard->db_slice().FindExt(t->GetDbContext(), key).first; @@ -1222,7 +1221,7 @@ void GenericFamily::Time(CmdArgList args, ConnectionContext* cntx) { OpResult GenericFamily::RenameGeneric(CmdArgList args, bool skip_exist_dest, ConnectionContext* cntx) { - string_view key[2] = {ArgS(args, 1), ArgS(args, 2)}; + string_view key[2] = {ArgS(args, 0), ArgS(args, 1)}; Transaction* transaction = cntx->transaction; @@ -1252,19 +1251,19 @@ OpResult GenericFamily::RenameGeneric(CmdArgList args, bool skip_exist_des } void GenericFamily::Echo(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); return (*cntx)->SendBulkString(key); } void GenericFamily::Scan(CmdArgList args, ConnectionContext* cntx) { - string_view token = ArgS(args, 1); + string_view token = ArgS(args, 0); uint64_t cursor = 0; if (!absl::SimpleAtoi(token, &cursor)) { return (*cntx)->SendError("invalid cursor"); } - OpResult ops = ScanOpts::TryFrom(args.subspan(2)); + OpResult ops = ScanOpts::TryFrom(args.subspan(1)); if (!ops) { DVLOG(1) << "Scan invalid args - return " << ops << " to the user"; return (*cntx)->SendError(ops.status()); diff --git a/src/server/hset_family.cc b/src/server/hset_family.cc index e8b669ed8..2f71837fb 100644 --- a/src/server/hset_family.cc +++ b/src/server/hset_family.cc @@ -688,7 +688,7 @@ OpResult OpSet(const OpArgs& op_args, string_view key, CmdArgList valu } void HGetGeneric(CmdArgList args, ConnectionContext* cntx, uint8_t getall_mask) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) { return OpGetAll(t->GetOpArgs(shard), key, getall_mask); @@ -707,16 +707,12 @@ void HGetGeneric(CmdArgList args, ConnectionContext* cntx, uint8_t getall_mask) // HSETEX key tll_sec field value field value ... void HSetEx(CmdArgList args, ConnectionContext* cntx) { - if (args.size() % 2 != 1) { - ToLower(&args[0]); - - string_view cmd = ArgS(args, 0); - - return (*cntx)->SendError(facade::WrongNumArgsError(cmd), kSyntaxErrType); + if (args.size() % 2 != 0) { + return (*cntx)->SendError(facade::WrongNumArgsError(cntx->cid->name()), kSyntaxErrType); } - string_view key = ArgS(args, 1); - string_view ttl_str = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view ttl_str = ArgS(args, 1); uint32_t ttl_sec; constexpr uint32_t kMaxTtl = (1UL << 26); @@ -724,7 +720,7 @@ void HSetEx(CmdArgList args, ConnectionContext* cntx) { return (*cntx)->SendError(kInvalidIntErr); } - args.remove_prefix(3); + args.remove_prefix(2); OpSetParams op_sp; op_sp.ttl = ttl_sec; @@ -743,9 +739,9 @@ void HSetEx(CmdArgList args, ConnectionContext* cntx) { } // namespace void HSetFamily::HDel(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); - args.remove_prefix(2); + args.remove_prefix(1); auto cb = [&](Transaction* t, EngineShard* shard) { return OpDel(t->GetOpArgs(shard), key, args); }; @@ -759,7 +755,7 @@ void HSetFamily::HDel(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HLen(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) { return OpLen(t->GetOpArgs(shard), key); }; @@ -772,8 +768,8 @@ void HSetFamily::HLen(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HExists(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view field = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view field = ArgS(args, 1); auto cb = [&](Transaction* t, EngineShard* shard) -> OpResult { return OpExist(t->GetOpArgs(shard), key, field); @@ -788,9 +784,9 @@ void HSetFamily::HExists(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HMGet(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); - args.remove_prefix(2); + args.remove_prefix(1); auto cb = [&](Transaction* t, EngineShard* shard) { return OpMGet(t->GetOpArgs(shard), key, args); }; @@ -817,8 +813,8 @@ void HSetFamily::HMGet(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HGet(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view field = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view field = ArgS(args, 1); auto cb = [&](Transaction* t, EngineShard* shard) { return OpGet(t->GetOpArgs(shard), key, field); @@ -837,9 +833,9 @@ void HSetFamily::HGet(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HIncrBy(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view field = ArgS(args, 2); - string_view incrs = ArgS(args, 3); + string_view key = ArgS(args, 0); + string_view field = ArgS(args, 1); + string_view incrs = ArgS(args, 2); int64_t ival = 0; if (!absl::SimpleAtoi(incrs, &ival)) { @@ -872,9 +868,9 @@ void HSetFamily::HIncrBy(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HIncrByFloat(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view field = ArgS(args, 2); - string_view incrs = ArgS(args, 3); + string_view key = ArgS(args, 0); + string_view field = ArgS(args, 1); + string_view incrs = ArgS(args, 2); double dval = 0; if (!absl::SimpleAtod(incrs, &dval)) { @@ -916,8 +912,8 @@ void HSetFamily::HGetAll(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HScan(CmdArgList args, ConnectionContext* cntx) { - std::string_view key = ArgS(args, 1); - std::string_view token = ArgS(args, 2); + std::string_view key = ArgS(args, 0); + std::string_view token = ArgS(args, 1); uint64_t cursor = 0; @@ -926,12 +922,12 @@ void HSetFamily::HScan(CmdArgList args, ConnectionContext* cntx) { } // HSCAN key cursor [MATCH pattern] [COUNT count] - if (args.size() > 7) { + if (args.size() > 6) { DVLOG(1) << "got " << args.size() << " this is more than it should be"; return (*cntx)->SendError(kSyntaxErr); } - OpResult ops = ScanOpts::TryFrom(args.subspan(3)); + OpResult ops = ScanOpts::TryFrom(args.subspan(2)); if (!ops) { DVLOG(1) << "HScan invalid args - return " << ops << " to the user"; return (*cntx)->SendError(ops.status()); @@ -957,22 +953,22 @@ void HSetFamily::HScan(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HSet(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); ToLower(&args[0]); - string_view cmd = ArgS(args, 0); - - if (args.size() % 2 != 0) { - return (*cntx)->SendError(facade::WrongNumArgsError(cmd), kSyntaxErrType); + if (args.size() % 2 != 1) { + return (*cntx)->SendError(facade::WrongNumArgsError("hset"), kSyntaxErrType); } - args.remove_prefix(2); + args.remove_prefix(1); auto cb = [&](Transaction* t, EngineShard* shard) { return OpSet(t->GetOpArgs(shard), key, args); }; OpResult result = cntx->transaction->ScheduleSingleHopT(std::move(cb)); - if (result && cmd == "hset") { + string_view cmd{cntx->cid->name()}; + + if (result && cmd == "HSET") { (*cntx)->SendLong(*result); } else { (*cntx)->SendError(result.status()); @@ -980,9 +976,9 @@ void HSetFamily::HSet(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HSetNx(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); - args.remove_prefix(2); + args.remove_prefix(1); auto cb = [&](Transaction* t, EngineShard* shard) { return OpSet(t->GetOpArgs(shard), key, args, OpSetParams{.skip_if_exists = true}); }; @@ -996,8 +992,8 @@ void HSetFamily::HSetNx(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HStrLen(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view field = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view field = ArgS(args, 1); auto cb = [&](Transaction* t, EngineShard* shard) { return OpStrLen(t->GetOpArgs(shard), key, field); @@ -1012,7 +1008,7 @@ void HSetFamily::HStrLen(CmdArgList args, ConnectionContext* cntx) { } void HSetFamily::HRandField(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) -> OpResult { auto& db_slice = shard->db_slice(); diff --git a/src/server/journal/serializer.cc b/src/server/journal/serializer.cc index ed5d5c7eb..4f180e517 100644 --- a/src/server/journal/serializer.cc +++ b/src/server/journal/serializer.cc @@ -32,18 +32,7 @@ void JournalWriter::Write(std::string_view sv) { sink_->Write(io::Buffer(sv)); } -void JournalWriter::Write(CmdArgList args) { - Write(args.size()); - size_t cmd_size = 0; - for (auto v : args) { - cmd_size += v.size(); - } - Write(cmd_size); - for (auto v : args) - Write(facade::ToSV(v)); -} - -void JournalWriter::Write(std::pair args) { +template void JournalWriter::Write(std::pair args) { auto [cmd, tail_args] = args; Write(1 + tail_args.size()); @@ -55,10 +44,17 @@ void JournalWriter::Write(std::pair args) { Write(cmd_size); Write(cmd); - for (auto v : tail_args) - Write(v); + for (auto v : tail_args) { + if constexpr (is_same_v) + Write(facade::ToSV(v)); + else + Write(v); + } } +template void JournalWriter::Write(pair); +template void JournalWriter::Write(pair); + void JournalWriter::Write(std::monostate) { } diff --git a/src/server/journal/serializer.h b/src/server/journal/serializer.h index bc001e2eb..3dd24d218 100644 --- a/src/server/journal/serializer.h +++ b/src/server/journal/serializer.h @@ -26,8 +26,9 @@ class JournalWriter { private: void Write(uint64_t v); // Write packed unsigned integer. void Write(std::string_view sv); // Write string. - void Write(CmdArgList args); - void Write(std::pair args); + + template // CmdArgList or ArgSlice. + void Write(std::pair args); void Write(std::monostate); // Overload for empty std::variant diff --git a/src/server/journal/types.h b/src/server/journal/types.h index 6e5f674f8..b4ba42f24 100644 --- a/src/server/journal/types.h +++ b/src/server/journal/types.h @@ -33,9 +33,9 @@ struct EntryBase { struct Entry : public EntryBase { // Payload represents a non-owning view into a command executed on the shard. using Payload = - std::variant // Command and its shard parts. + std::variant, // Parts of a full command. + std::pair // Command and its shard parts. >; Entry(TxId txid, Op opcode, DbIndex dbid, uint32_t shard_cnt, Payload pl) diff --git a/src/server/journal_test.cc b/src/server/journal_test.cc index ba78962a1..1c31c448c 100644 --- a/src/server/journal_test.cc +++ b/src/server/journal_test.cc @@ -18,6 +18,13 @@ struct EntryPayloadVisitor { *out += ' '; } + void operator()(const CmdArgList list) { + for (auto arg : list) { + *out += facade::ToSV(arg); + *out += ' '; + } + } + void operator()(const ArgSlice slice) { for (auto arg : slice) { *out += arg; @@ -25,18 +32,11 @@ struct EntryPayloadVisitor { } } - void operator()(const pair p) { + template void operator()(const pair p) { (*this)(p.first); (*this)(p.second); } - void operator()(const CmdArgList list) { - for (auto arg : list) { - *out += facade::ToSV(arg); - *out += ' '; - } - } - void operator()(monostate) { } @@ -100,12 +100,12 @@ TEST(Journal, WriteRead) { std::vector test_entries = { {0, journal::Op::COMMAND, 0, 2, make_pair("MSET", slice("A", "1", "B", "2"))}, {0, journal::Op::COMMAND, 0, 2, make_pair("MSET", slice("C", "3"))}, - {1, journal::Op::COMMAND, 0, 2, list("DEL", "A", "B")}, - {2, journal::Op::COMMAND, 1, 1, list("LPUSH", "l", "v1", "v2")}, + {1, journal::Op::COMMAND, 0, 2, make_pair("DEL", list("A", "B"))}, + {2, journal::Op::COMMAND, 1, 1, make_pair("LPUSH", list("l", "v1", "v2"))}, {3, journal::Op::COMMAND, 0, 1, make_pair("MSET", slice("D", "4"))}, - {4, journal::Op::COMMAND, 1, 1, list("DEL", "l1")}, - {5, journal::Op::COMMAND, 2, 1, list("SET", "E", "2")}, - {6, journal::Op::MULTI_COMMAND, 2, 1, list("SET", "E", "2")}, + {4, journal::Op::COMMAND, 1, 1, make_pair("DEL", list("l1"))}, + {5, journal::Op::COMMAND, 2, 1, make_pair("DEL", list("E", "2"))}, + {6, journal::Op::MULTI_COMMAND, 2, 1, make_pair("SET", list("E", "2"))}, {6, journal::Op::EXEC, 2, 1}}; // Write all entries to a buffer. diff --git a/src/server/json_family.cc b/src/server/json_family.cc index 8e45fd447..b83d02535 100644 --- a/src/server/json_family.cc +++ b/src/server/json_family.cc @@ -1002,9 +1002,9 @@ OpResult OpSet(const OpArgs& op_args, string_view key, string_view path, } // namespace void JsonFamily::Set(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); - string_view json_str = ArgS(args, 3); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); + string_view json_str = ArgS(args, 2); auto cb = [&](Transaction* t, EngineShard* shard) { return OpSet(t->GetOpArgs(shard), key, path, json_str); @@ -1025,10 +1025,10 @@ void JsonFamily::Set(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::Resp(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); string_view path = DefaultJsonPath; - if (args.size() > 2) { - path = ArgS(args, 2); + if (args.size() > 1) { + path = ArgS(args, 1); } error_code ec; @@ -1059,7 +1059,7 @@ void JsonFamily::Resp(CmdArgList args, ConnectionContext* cntx) { void JsonFamily::Debug(CmdArgList args, ConnectionContext* cntx) { function func; - string_view command = ArgS(args, 1); + string_view command = ArgS(args, 0); // The 'MEMORY' sub-command is not supported yet, calling to operation function should be added // here. if (absl::EqualsIgnoreCase(command, "help")) { @@ -1077,14 +1077,14 @@ void JsonFamily::Debug(CmdArgList args, ConnectionContext* cntx) { return; } - if (args.size() < 4) { - (*cntx)->SendError(facade::WrongNumArgsError(command), facade::kSyntaxErrType); + if (args.size() < 3) { + (*cntx)->SendError(facade::WrongNumArgsError(cntx->cid->name()), facade::kSyntaxErrType); return; } error_code ec; - string_view key = ArgS(args, 2); - string_view path = ArgS(args, 3); + string_view key = ArgS(args, 1); + string_view path = ArgS(args, 2); JsonExpression expression = jsonpath::make_expression(path, ec); if (ec) { @@ -1108,7 +1108,7 @@ void JsonFamily::Debug(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::MGet(CmdArgList args, ConnectionContext* cntx) { - DCHECK_GE(args.size(), 2U); + DCHECK_GE(args.size(), 1U); error_code ec; string_view path = ArgS(args, args.size() - 1); @@ -1133,7 +1133,7 @@ void JsonFamily::MGet(CmdArgList args, ConnectionContext* cntx) { OpStatus result = transaction->ScheduleSingleHop(std::move(cb)); CHECK_EQ(OpStatus::OK, result); - std::vector results(args.size() - 2); + std::vector results(args.size() - 1); for (ShardId sid = 0; sid < shard_count; ++sid) { if (!transaction->IsActive(sid)) continue; @@ -1164,8 +1164,8 @@ void JsonFamily::MGet(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::ArrIndex(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); error_code ec; JsonExpression expression = jsonpath::make_expression(path, ec); @@ -1176,7 +1176,7 @@ void JsonFamily::ArrIndex(CmdArgList args, ConnectionContext* cntx) { return; } - optional search_value = JsonFromString(ArgS(args, 3)); + optional search_value = JsonFromString(ArgS(args, 2)); if (!search_value) { (*cntx)->SendError(kSyntaxErr); return; @@ -1188,18 +1188,18 @@ void JsonFamily::ArrIndex(CmdArgList args, ConnectionContext* cntx) { } int start_index = 0; - if (args.size() >= 5) { - if (!absl::SimpleAtoi(ArgS(args, 4), &start_index)) { - VLOG(1) << "Failed to convert the start index to numeric" << ArgS(args, 4); + if (args.size() >= 4) { + if (!absl::SimpleAtoi(ArgS(args, 3), &start_index)) { + VLOG(1) << "Failed to convert the start index to numeric" << ArgS(args, 3); (*cntx)->SendError(kInvalidIntErr); return; } } int end_index = 0; - if (args.size() >= 6) { - if (!absl::SimpleAtoi(ArgS(args, 5), &end_index)) { - VLOG(1) << "Failed to convert the stop index to numeric" << ArgS(args, 5); + if (args.size() >= 5) { + if (!absl::SimpleAtoi(ArgS(args, 4), &end_index)) { + VLOG(1) << "Failed to convert the stop index to numeric" << ArgS(args, 4); (*cntx)->SendError(kInvalidIntErr); return; } @@ -1221,18 +1221,18 @@ void JsonFamily::ArrIndex(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::ArrInsert(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); int index = -1; - if (!absl::SimpleAtoi(ArgS(args, 3), &index)) { - VLOG(1) << "Failed to convert the following value to numeric: " << ArgS(args, 3); + if (!absl::SimpleAtoi(ArgS(args, 2), &index)) { + VLOG(1) << "Failed to convert the following value to numeric: " << ArgS(args, 2); (*cntx)->SendError(kInvalidIntErr); return; } vector new_values; - for (size_t i = 4; i < args.size(); i++) { + for (size_t i = 3; i < args.size(); i++) { optional val = JsonFromString(ArgS(args, i)); if (!val) { (*cntx)->SendError(kSyntaxErr); @@ -1256,10 +1256,10 @@ void JsonFamily::ArrInsert(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::ArrAppend(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); vector append_values; - for (size_t i = 3; i < args.size(); ++i) { + for (size_t i = 2; i < args.size(); ++i) { optional converted_val = JsonFromString(ArgS(args, i)); if (!converted_val) { (*cntx)->SendError(kSyntaxErr); @@ -1288,18 +1288,18 @@ void JsonFamily::ArrAppend(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::ArrTrim(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); int start_index; int stop_index; - if (!absl::SimpleAtoi(ArgS(args, 3), &start_index)) { + if (!absl::SimpleAtoi(ArgS(args, 2), &start_index)) { VLOG(1) << "Failed to parse array start index"; (*cntx)->SendError(kInvalidIntErr); return; } - if (!absl::SimpleAtoi(ArgS(args, 4), &stop_index)) { + if (!absl::SimpleAtoi(ArgS(args, 3), &stop_index)) { VLOG(1) << "Failed to parse array stop index"; (*cntx)->SendError(kInvalidIntErr); return; @@ -1324,14 +1324,14 @@ void JsonFamily::ArrTrim(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::ArrPop(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); int index = -1; - if (args.size() >= 4) { - if (!absl::SimpleAtoi(ArgS(args, 3), &index)) { + if (args.size() >= 3) { + if (!absl::SimpleAtoi(ArgS(args, 2), &index)) { VLOG(1) << "Failed to convert the following value to numeric, pop out the last item" - << ArgS(args, 3); + << ArgS(args, 2); } } @@ -1365,8 +1365,8 @@ void JsonFamily::ArrPop(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::Clear(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); auto cb = [&](Transaction* t, EngineShard* shard) { return OpClear(t->GetOpArgs(shard), key, path); @@ -1383,11 +1383,11 @@ void JsonFamily::Clear(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::StrAppend(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); vector strs; - for (size_t i = 3; i < args.size(); ++i) { + for (size_t i = 2; i < args.size(); ++i) { strs.emplace_back(ArgS(args, i)); } @@ -1406,8 +1406,8 @@ void JsonFamily::StrAppend(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::ObjKeys(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); error_code ec; JsonExpression expression = jsonpath::make_expression(path, ec); @@ -1440,10 +1440,10 @@ void JsonFamily::ObjKeys(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::Del(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); string_view path; - if (args.size() > 2) { - path = ArgS(args, 2); + if (args.size() > 1) { + path = ArgS(args, 1); } auto cb = [&](Transaction* t, EngineShard* shard) { @@ -1456,9 +1456,9 @@ void JsonFamily::Del(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::NumIncrBy(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); - string_view num = ArgS(args, 3); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); + string_view num = ArgS(args, 2); double dnum; if (!ParseDouble(num, &dnum)) { @@ -1481,9 +1481,9 @@ void JsonFamily::NumIncrBy(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::NumMultBy(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); - string_view num = ArgS(args, 3); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); + string_view num = ArgS(args, 2); double dnum; if (!ParseDouble(num, &dnum)) { @@ -1506,8 +1506,8 @@ void JsonFamily::NumMultBy(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::Toggle(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); auto cb = [&](Transaction* t, EngineShard* shard) { return OpToggle(t->GetOpArgs(shard), key, path); @@ -1524,8 +1524,8 @@ void JsonFamily::Toggle(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::Type(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); error_code ec; JsonExpression expression = jsonpath::make_expression(path, ec); @@ -1560,8 +1560,8 @@ void JsonFamily::Type(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::ArrLen(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); error_code ec; JsonExpression expression = jsonpath::make_expression(path, ec); @@ -1587,8 +1587,8 @@ void JsonFamily::ArrLen(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::ObjLen(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); error_code ec; JsonExpression expression = jsonpath::make_expression(path, ec); @@ -1614,8 +1614,8 @@ void JsonFamily::ObjLen(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::StrLen(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view path = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view path = ArgS(args, 1); error_code ec; JsonExpression expression = jsonpath::make_expression(path, ec); @@ -1641,12 +1641,12 @@ void JsonFamily::StrLen(CmdArgList args, ConnectionContext* cntx) { } void JsonFamily::Get(CmdArgList args, ConnectionContext* cntx) { - DCHECK_GE(args.size(), 2U); - string_view key = ArgS(args, 1); + DCHECK_GE(args.size(), 1U); + string_view key = ArgS(args, 0); vector> expressions; - for (size_t i = 2; i < args.size(); ++i) { + for (size_t i = 1; i < args.size(); ++i) { string_view path = ArgS(args, i); error_code ec; JsonExpression expr = jsonpath::make_expression(path, ec); diff --git a/src/server/json_family_test.cc b/src/server/json_family_test.cc index 8e851c70e..b3e97c7e5 100644 --- a/src/server/json_family_test.cc +++ b/src/server/json_family_test.cc @@ -855,6 +855,15 @@ TEST_F(JsonFamilyTest, ArrIndex) { resp = Run({"JSON.ARRINDEX", "json", "$..a", R"("b")"}); ASSERT_EQ(RespExpr::ARRAY, resp.type); EXPECT_THAT(resp.GetVec(), ElementsAre(IntArg(1), ArgType(RespExpr::NIL))); + + resp = Run( + {"JSON.SET", "json", ".", R"({"key" : ["Alice", "Bob", "Carol", "David", "Eve", "Frank"]})"}); + ASSERT_EQ(resp, "OK"); + resp = Run({"JSON.ARRINDEX", "json", "$.key", R"("Bob")"}); + EXPECT_THAT(resp, IntArg(1)); + + resp = Run({"JSON.ARRINDEX", "json", "$.key", R"("Bob")", "1", "2"}); + EXPECT_THAT(resp, IntArg(1)); } TEST_F(JsonFamilyTest, MGet) { diff --git a/src/server/list_family.cc b/src/server/list_family.cc index b77812a3f..a5a035125 100644 --- a/src/server/list_family.cc +++ b/src/server/list_family.cc @@ -882,16 +882,16 @@ void MoveGeneric(ConnectionContext* cntx, string_view src, string_view dest, Lis } void RPopLPush(CmdArgList args, ConnectionContext* cntx) { - string_view src = ArgS(args, 1); - string_view dest = ArgS(args, 2); + string_view src = ArgS(args, 0); + string_view dest = ArgS(args, 1); MoveGeneric(cntx, src, dest, ListDir::RIGHT, ListDir::LEFT); } void BRPopLPush(CmdArgList args, ConnectionContext* cntx) { - string_view src = ArgS(args, 1); - string_view dest = ArgS(args, 2); - string_view timeout_str = ArgS(args, 3); + string_view src = ArgS(args, 0); + string_view dest = ArgS(args, 1); + string_view timeout_str = ArgS(args, 2); float timeout; if (!absl::SimpleAtof(timeout_str, &timeout)) { @@ -921,9 +921,9 @@ void BRPopLPush(CmdArgList args, ConnectionContext* cntx) { } void BLMove(CmdArgList args, ConnectionContext* cntx) { - string_view src = ArgS(args, 1); - string_view dest = ArgS(args, 2); - string_view timeout_str = ArgS(args, 5); + string_view src = ArgS(args, 0); + string_view dest = ArgS(args, 1); + string_view timeout_str = ArgS(args, 4); float timeout; if (!absl::SimpleAtof(timeout_str, &timeout)) { @@ -934,11 +934,11 @@ void BLMove(CmdArgList args, ConnectionContext* cntx) { return (*cntx)->SendError("timeout is negative"); } + ToUpper(&args[2]); ToUpper(&args[3]); - ToUpper(&args[4]); - optional src_dir = ParseDir(ArgS(args, 3)); - optional dest_dir = ParseDir(ArgS(args, 4)); + optional src_dir = ParseDir(ArgS(args, 2)); + optional dest_dir = ParseDir(ArgS(args, 3)); if (!src_dir || !dest_dir) { return (*cntx)->SendError(kSyntaxErr); } @@ -1075,7 +1075,7 @@ void ListFamily::RPop(CmdArgList args, ConnectionContext* cntx) { } void ListFamily::LLen(CmdArgList args, ConnectionContext* cntx) { - auto key = ArgS(args, 1); + auto key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) { return OpLen(t->GetOpArgs(shard), key); }; OpResult result = cntx->transaction->ScheduleSingleHopT(std::move(cb)); if (result) { @@ -1088,15 +1088,15 @@ void ListFamily::LLen(CmdArgList args, ConnectionContext* cntx) { } void ListFamily::LPos(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view elem = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view elem = ArgS(args, 1); int rank = 1; uint32_t count = 1; uint32_t max_len = 0; bool skip_count = true; - for (size_t i = 3; i < args.size(); i++) { + for (size_t i = 2; i < args.size(); i++) { ToUpper(&args[i]); const auto& arg_v = ArgS(args, i); if (arg_v == "RANK") { @@ -1146,8 +1146,8 @@ void ListFamily::LPos(CmdArgList args, ConnectionContext* cntx) { } void ListFamily::LIndex(CmdArgList args, ConnectionContext* cntx) { - std::string_view key = ArgS(args, 1); - std::string_view index_str = ArgS(args, 2); + std::string_view key = ArgS(args, 0); + std::string_view index_str = ArgS(args, 1); int32_t index; if (!absl::SimpleAtoi(index_str, &index)) { (*cntx)->SendError(kInvalidIntErr); @@ -1170,10 +1170,10 @@ void ListFamily::LIndex(CmdArgList args, ConnectionContext* cntx) { /* LINSERT (BEFORE|AFTER) */ void ListFamily::LInsert(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view param = ArgS(args, 2); - string_view pivot = ArgS(args, 3); - string_view elem = ArgS(args, 4); + string_view key = ArgS(args, 0); + string_view param = ArgS(args, 1); + string_view pivot = ArgS(args, 2); + string_view elem = ArgS(args, 3); int where; ToUpper(&args[2]); @@ -1198,9 +1198,9 @@ void ListFamily::LInsert(CmdArgList args, ConnectionContext* cntx) { } void ListFamily::LTrim(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view s_str = ArgS(args, 2); - string_view e_str = ArgS(args, 3); + string_view key = ArgS(args, 0); + string_view s_str = ArgS(args, 1); + string_view e_str = ArgS(args, 2); int32_t start, end; if (!absl::SimpleAtoi(s_str, &start) || !absl::SimpleAtoi(e_str, &end)) { @@ -1216,9 +1216,9 @@ void ListFamily::LTrim(CmdArgList args, ConnectionContext* cntx) { } void ListFamily::LRange(CmdArgList args, ConnectionContext* cntx) { - std::string_view key = ArgS(args, 1); - std::string_view s_str = ArgS(args, 2); - std::string_view e_str = ArgS(args, 3); + std::string_view key = ArgS(args, 0); + std::string_view s_str = ArgS(args, 1); + std::string_view e_str = ArgS(args, 2); int32_t start, end; if (!absl::SimpleAtoi(s_str, &start) || !absl::SimpleAtoi(e_str, &end)) { @@ -1240,9 +1240,9 @@ void ListFamily::LRange(CmdArgList args, ConnectionContext* cntx) { // lrem key 5 foo, will remove foo elements from the list if exists at most 5 times. void ListFamily::LRem(CmdArgList args, ConnectionContext* cntx) { - std::string_view key = ArgS(args, 1); - std::string_view index_str = ArgS(args, 2); - std::string_view elem = ArgS(args, 3); + std::string_view key = ArgS(args, 0); + std::string_view index_str = ArgS(args, 1); + std::string_view elem = ArgS(args, 2); int32_t count; if (!absl::SimpleAtoi(index_str, &count)) { @@ -1262,9 +1262,9 @@ void ListFamily::LRem(CmdArgList args, ConnectionContext* cntx) { } void ListFamily::LSet(CmdArgList args, ConnectionContext* cntx) { - std::string_view key = ArgS(args, 1); - std::string_view index_str = ArgS(args, 2); - std::string_view elem = ArgS(args, 3); + std::string_view key = ArgS(args, 0); + std::string_view index_str = ArgS(args, 1); + std::string_view elem = ArgS(args, 2); int32_t count; if (!absl::SimpleAtoi(index_str, &count)) { @@ -1292,13 +1292,13 @@ void ListFamily::BRPop(CmdArgList args, ConnectionContext* cntx) { } void ListFamily::LMove(CmdArgList args, ConnectionContext* cntx) { - std::string_view src = ArgS(args, 1); - std::string_view dest = ArgS(args, 2); - std::string_view src_dir_str = ArgS(args, 3); - std::string_view dest_dir_str = ArgS(args, 4); + std::string_view src = ArgS(args, 0); + std::string_view dest = ArgS(args, 1); + std::string_view src_dir_str = ArgS(args, 2); + std::string_view dest_dir_str = ArgS(args, 3); + ToUpper(&args[2]); ToUpper(&args[3]); - ToUpper(&args[4]); optional src_dir = ParseDir(src_dir_str); optional dest_dir = ParseDir(dest_dir_str); @@ -1310,7 +1310,7 @@ void ListFamily::LMove(CmdArgList args, ConnectionContext* cntx) { } void ListFamily::BPopGeneric(ListDir dir, CmdArgList args, ConnectionContext* cntx) { - DCHECK_GE(args.size(), 3u); + DCHECK_GE(args.size(), 2u); float timeout; auto timeout_str = ArgS(args, args.size() - 1); @@ -1351,10 +1351,10 @@ void ListFamily::BPopGeneric(ListDir dir, CmdArgList args, ConnectionContext* cn void ListFamily::PushGeneric(ListDir dir, bool skip_notexists, CmdArgList args, ConnectionContext* cntx) { - std::string_view key = ArgS(args, 1); - vector vals(args.size() - 2); - for (size_t i = 2; i < args.size(); ++i) { - vals[i - 2] = ArgS(args, i); + std::string_view key = ArgS(args, 0); + vector vals(args.size() - 1); + for (size_t i = 1; i < args.size(); ++i) { + vals[i - 1] = ArgS(args, i); } absl::Span span{vals.data(), vals.size()}; auto cb = [&](Transaction* t, EngineShard* shard) { @@ -1370,17 +1370,16 @@ void ListFamily::PushGeneric(ListDir dir, bool skip_notexists, CmdArgList args, } void ListFamily::PopGeneric(ListDir dir, CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); int32_t count = 1; bool return_arr = false; - if (args.size() > 2) { - if (args.size() > 3) { - ToLower(&args[0]); - return (*cntx)->SendError(WrongNumArgsError(ArgS(args, 0))); + if (args.size() > 1) { + if (args.size() > 2) { + return (*cntx)->SendError(WrongNumArgsError(cntx->cid->name())); } - string_view count_s = ArgS(args, 2); + string_view count_s = ArgS(args, 1); if (!absl::SimpleAtoi(count_s, &count)) { return (*cntx)->SendError(kInvalidIntErr); } diff --git a/src/server/main_service.cc b/src/server/main_service.cc index 579fffe42..38f649353 100644 --- a/src/server/main_service.cc +++ b/src/server/main_service.cc @@ -398,7 +398,7 @@ bool IsSHA(string_view str) { } bool EvalValidator(CmdArgList args, ConnectionContext* cntx) { - string_view num_keys_str = ArgS(args, 2); + string_view num_keys_str = ArgS(args, 1); int32_t num_keys; if (!absl::SimpleAtoi(num_keys_str, &num_keys) || num_keys < 0) { @@ -406,7 +406,7 @@ bool EvalValidator(CmdArgList args, ConnectionContext* cntx) { return false; } - if (unsigned(num_keys) > args.size() - 3) { + if (unsigned(num_keys) > args.size() - 2) { (*cntx)->SendError("Number of keys can't be greater than number of args", kSyntaxErrType); return false; } @@ -585,7 +585,7 @@ OpStatus CheckKeysDeclared(const ConnectionState::ScriptInfo& eval_info, const C } } - if (unsigned i = key_index.bonus; i && !eval_info.keys.contains(ArgS(args, i))) + if (key_index.bonus && !eval_info.keys.contains(ArgS(args, *key_index.bonus))) return OpStatus::KEY_NOTFOUND; return OpStatus::OK; @@ -668,7 +668,7 @@ void Service::DispatchCommand(CmdArgList args, facade::ConnectionContext* cntx) } // Validate more complicated cases with custom validators. - if (!cid->Validate(args, dfly_cntx)) { + if (!cid->Validate(args.subspan(1), dfly_cntx)) { return; } @@ -699,7 +699,7 @@ void Service::DispatchCommand(CmdArgList args, facade::ConnectionContext* cntx) return (*cntx)->SendError(error); } // TODO: protect against aggregating huge transactions. - StoredCmd stored_cmd{cid, args}; + StoredCmd stored_cmd{cid, args.subspan(1)}; dfly_cntx->conn_state.exec_info.body.push_back(std::move(stored_cmd)); return (*cntx)->SendSimpleString("QUEUED"); @@ -714,8 +714,8 @@ void Service::DispatchCommand(CmdArgList args, facade::ConnectionContext* cntx) if (under_script) { DCHECK(dfly_cntx->transaction); if (cid->IsTransactional()) { - OpStatus status = - CheckKeysDeclared(*dfly_cntx->conn_state.script_info, cid, args, dfly_cntx->transaction); + OpStatus status = CheckKeysDeclared(*dfly_cntx->conn_state.script_info, cid, args.subspan(1), + dfly_cntx->transaction); if (status == OpStatus::KEY_NOTFOUND) return (*cntx)->SendError("script tried accessing undeclared key"); @@ -724,7 +724,7 @@ void Service::DispatchCommand(CmdArgList args, facade::ConnectionContext* cntx) return (*cntx)->SendError(status); dfly_cntx->transaction->MultiSwitchCmd(cid); - status = dfly_cntx->transaction->InitByArgs(dfly_cntx->conn_state.db_index, args); + status = dfly_cntx->transaction->InitByArgs(dfly_cntx->conn_state.db_index, args.subspan(1)); if (status != OpStatus::OK) return (*cntx)->SendError(status); @@ -736,7 +736,7 @@ void Service::DispatchCommand(CmdArgList args, facade::ConnectionContext* cntx) dist_trans.reset(new Transaction{cid, etl.thread_index()}); if (!dist_trans->IsMulti()) { // Multi command initialize themself based on their mode. - if (auto st = dist_trans->InitByArgs(dfly_cntx->conn_state.db_index, args); + if (auto st = dist_trans->InitByArgs(dfly_cntx->conn_state.db_index, args.subspan(1)); st != OpStatus::OK) return (*cntx)->SendError(st); } @@ -754,7 +754,7 @@ void Service::DispatchCommand(CmdArgList args, facade::ConnectionContext* cntx) // itself. EXEC does not use DispatchCommand for dispatching. bool collect_stats = dfly_cntx->transaction && (!dfly_cntx->transaction->IsMulti() || under_script); - if (!InvokeCmd(args, cid, dfly_cntx, collect_stats)) { + if (!InvokeCmd(args.subspan(1), cid, dfly_cntx, collect_stats)) { dfly_cntx->reply_builder()->SendError("Internal Error"); dfly_cntx->reply_builder()->CloseConnection(); } @@ -977,7 +977,7 @@ void Service::Watch(CmdArgList args, ConnectionContext* cntx) { // Duplicate keys are stored to keep correct count. exec_info.watched_existed += keys_existed.load(memory_order_relaxed); - for (size_t i = 1; i < args.size(); i++) { + for (size_t i = 0; i < args.size(); i++) { exec_info.watched_keys.emplace_back(cntx->db_index(), ArgS(args, i)); } @@ -1004,9 +1004,9 @@ void Service::CallFromScript(CmdArgList args, ObjectExplorer* reply, ConnectionC void Service::Eval(CmdArgList args, ConnectionContext* cntx) { uint32_t num_keys; - CHECK(absl::SimpleAtoi(ArgS(args, 2), &num_keys)); // we already validated this + CHECK(absl::SimpleAtoi(ArgS(args, 1), &num_keys)); // we already validated this - string_view body = ArgS(args, 1); + string_view body = ArgS(args, 0); // body = absl::StripAsciiWhitespace(body); if (body.empty()) { @@ -1025,8 +1025,8 @@ void Service::Eval(CmdArgList args, ConnectionContext* cntx) { EvalArgs eval_args; eval_args.sha = sha; - eval_args.keys = args.subspan(3, num_keys); - eval_args.args = args.subspan(3 + num_keys); + eval_args.keys = args.subspan(2, num_keys); + eval_args.args = args.subspan(2 + num_keys); uint64_t start = absl::GetCurrentTimeNanos(); EvalInternal(eval_args, interpreter, cntx); @@ -1036,22 +1036,22 @@ void Service::Eval(CmdArgList args, ConnectionContext* cntx) { } void Service::EvalSha(CmdArgList args, ConnectionContext* cntx) { - string_view num_keys_str = ArgS(args, 2); + string_view num_keys_str = ArgS(args, 1); uint32_t num_keys; CHECK(absl::SimpleAtoi(num_keys_str, &num_keys)); - ToLower(&args[1]); + ToLower(&args[0]); - string_view sha = ArgS(args, 1); + string_view sha = ArgS(args, 0); ServerState* ss = ServerState::tlocal(); auto interpreter = ss->BorrowInterpreter(); absl::Cleanup clean = [ss, interpreter]() { ss->ReturnInterpreter(interpreter); }; EvalArgs ev_args; ev_args.sha = sha; - ev_args.keys = args.subspan(3, num_keys); - ev_args.args = args.subspan(3 + num_keys); + ev_args.keys = args.subspan(2, num_keys); + ev_args.args = args.subspan(2 + num_keys); uint64_t start = absl::GetCurrentTimeNanos(); EvalInternal(ev_args, interpreter, cntx); @@ -1201,10 +1201,9 @@ bool CheckWatchedKeyExpiry(ConnectionContext* cntx, const CommandRegistry& regis static char EXISTS[] = "EXISTS"; auto& exec_info = cntx->conn_state.exec_info; - CmdArgVec str_list(exec_info.watched_keys.size() + 1); - str_list[0] = MutableSlice{EXISTS, strlen(EXISTS)}; - for (size_t i = 1; i < str_list.size(); i++) { - auto& [db, s] = exec_info.watched_keys[i - 1]; + CmdArgVec str_list(exec_info.watched_keys.size()); + for (size_t i = 0; i < str_list.size(); i++) { + auto& [db, s] = exec_info.watched_keys[i]; str_list[i] = MutableSlice{s.data(), s.size()}; } @@ -1218,8 +1217,7 @@ bool CheckWatchedKeyExpiry(ConnectionContext* cntx, const CommandRegistry& regis }; cntx->transaction->MultiSwitchCmd(registry.Find(EXISTS)); - cntx->transaction->InitByArgs(cntx->conn_state.db_index, - CmdArgList{str_list.data(), str_list.size()}); + cntx->transaction->InitByArgs(cntx->conn_state.db_index, CmdArgList{str_list}); OpStatus status = cntx->transaction->ScheduleSingleHop(std::move(cb)); CHECK_EQ(OpStatus::OK, status); @@ -1259,7 +1257,7 @@ template void IterateAllKeys(ConnectionState::ExecInfo* exec_info, f(args[i]); if (key_index.bonus) - f(args[key_index.bonus]); + f(args[*key_index.bonus]); } } @@ -1390,7 +1388,7 @@ void Service::Exec(CmdArgList args, ConnectionContext* cntx) { } void Service::Publish(CmdArgList args, ConnectionContext* cntx) { - string_view channel = ArgS(args, 1); + string_view channel = ArgS(args, 0); auto* cs = ServerState::tlocal()->channel_store(); vector subscribers = cs->FetchSubscribers(channel); @@ -1398,7 +1396,7 @@ void Service::Publish(CmdArgList args, ConnectionContext* cntx) { if (!subscribers.empty()) { auto subscribers_ptr = make_shared(move(subscribers)); - auto msg_ptr = make_shared(ArgS(args, 2)); + auto msg_ptr = make_shared(ArgS(args, 1)); auto channel_ptr = make_shared(channel); auto cb = [subscribers_ptr, msg_ptr, channel_ptr](unsigned idx, util::ProactorBase*) { @@ -1420,14 +1418,10 @@ void Service::Publish(CmdArgList args, ConnectionContext* cntx) { } void Service::Subscribe(CmdArgList args, ConnectionContext* cntx) { - args.remove_prefix(1); - cntx->ChangeSubscription(true /*add*/, true /* reply*/, std::move(args)); } void Service::Unsubscribe(CmdArgList args, ConnectionContext* cntx) { - args.remove_prefix(1); - if (args.size() == 0) { cntx->UnsubscribeAll(true); } else { @@ -1436,13 +1430,10 @@ void Service::Unsubscribe(CmdArgList args, ConnectionContext* cntx) { } void Service::PSubscribe(CmdArgList args, ConnectionContext* cntx) { - args.remove_prefix(1); cntx->ChangePSubscription(true, true, args); } void Service::PUnsubscribe(CmdArgList args, ConnectionContext* cntx) { - args.remove_prefix(1); - if (args.size() == 0) { cntx->PUnsubscribeAll(true); } else { @@ -1453,8 +1444,8 @@ void Service::PUnsubscribe(CmdArgList args, ConnectionContext* cntx) { // Not a real implementation. Serves as a decorator to accept some function commands // for testing. void Service::Function(CmdArgList args, ConnectionContext* cntx) { - ToUpper(&args[1]); - string_view sub_cmd = ArgS(args, 1); + ToUpper(&args[0]); + string_view sub_cmd = ArgS(args, 0); if (sub_cmd == "FLUSH") { return (*cntx)->SendOk(); @@ -1483,13 +1474,13 @@ void Service::Monitor(CmdArgList args, ConnectionContext* cntx) { } void Service::Pubsub(CmdArgList args, ConnectionContext* cntx) { - if (args.size() < 2) { - (*cntx)->SendError(WrongNumArgsError(ArgS(args, 0))); + if (args.size() < 1) { + (*cntx)->SendError(WrongNumArgsError(cntx->cid->name())); return; } - ToUpper(&args[1]); - string_view subcmd = ArgS(args, 1); + ToUpper(&args[0]); + string_view subcmd = ArgS(args, 0); if (subcmd == "HELP") { string_view help_arr[] = { @@ -1507,8 +1498,8 @@ void Service::Pubsub(CmdArgList args, ConnectionContext* cntx) { if (subcmd == "CHANNELS") { string_view pattern; - if (args.size() > 2) { - pattern = ArgS(args, 2); + if (args.size() > 1) { + pattern = ArgS(args, 1); } PubsubChannels(pattern, cntx); diff --git a/src/server/memory_cmd.cc b/src/server/memory_cmd.cc index 5cf92bca7..da741dc3b 100644 --- a/src/server/memory_cmd.cc +++ b/src/server/memory_cmd.cc @@ -73,7 +73,7 @@ MemoryCmd::MemoryCmd(ServerFamily* owner, ConnectionContext* cntx) : cntx_(cntx) } void MemoryCmd::Run(CmdArgList args) { - string_view sub_cmd = ArgS(args, 1); + string_view sub_cmd = ArgS(args, 0); if (sub_cmd == "HELP") { string_view help_arr[] = { diff --git a/src/server/multi_command_squasher.cc b/src/server/multi_command_squasher.cc index 4972c0980..3166100f2 100644 --- a/src/server/multi_command_squasher.cc +++ b/src/server/multi_command_squasher.cc @@ -17,7 +17,7 @@ template void IterateKeys(CmdArgList args, KeyIndex keys, F&& f) { f(args[i]); if (keys.bonus) - f(args[keys.bonus]); + f(args[*keys.bonus]); } } // namespace diff --git a/src/server/server_family.cc b/src/server/server_family.cc index 9aaf0348b..1ebd36aa0 100644 --- a/src/server/server_family.cc +++ b/src/server/server_family.cc @@ -275,8 +275,8 @@ void ExtendFilenameWithShard(absl::Time now, int shard, fs::path* filename) { } void SlowLog(CmdArgList args, ConnectionContext* cntx) { - ToUpper(&args[1]); - string_view sub_cmd = ArgS(args, 1); + ToUpper(&args[0]); + string_view sub_cmd = ArgS(args, 0); if (sub_cmd == "LEN") { return (*cntx)->SendLong(0); @@ -1106,7 +1106,7 @@ void ServerFamily::FlushAll(CmdArgList args, ConnectionContext* cntx) { } void ServerFamily::Auth(CmdArgList args, ConnectionContext* cntx) { - if (args.size() > 3) { + if (args.size() > 2) { return (*cntx)->SendError(kSyntaxErr); } @@ -1120,7 +1120,7 @@ void ServerFamily::Auth(CmdArgList args, ConnectionContext* cntx) { "default user. Are you sure your configuration is correct?"); } - string_view pass = ArgS(args, 1); + string_view pass = ArgS(args, 0); if (pass == GetPassword()) { cntx->authenticated = true; (*cntx)->SendOk(); @@ -1130,11 +1130,11 @@ void ServerFamily::Auth(CmdArgList args, ConnectionContext* cntx) { } void ServerFamily::Client(CmdArgList args, ConnectionContext* cntx) { - ToUpper(&args[1]); - string_view sub_cmd = ArgS(args, 1); + ToUpper(&args[0]); + string_view sub_cmd = ArgS(args, 0); - if (sub_cmd == "SETNAME" && args.size() == 3) { - cntx->owner()->SetName(ArgS(args, 2)); + if (sub_cmd == "SETNAME" && args.size() == 2) { + cntx->owner()->SetName(ArgS(args, 1)); return (*cntx)->SendOk(); } @@ -1184,8 +1184,8 @@ void ServerFamily::Cluster(CmdArgList args, ConnectionContext* cntx) { constexpr unsigned int kNoReplicaInfoSize = 3; constexpr unsigned int kWithReplicaInfoSize = 4; - ToUpper(&args[1]); - string_view sub_cmd = ArgS(args, 1); + ToUpper(&args[0]); + string_view sub_cmd = ArgS(args, 0); if (!is_emulated_cluster_) { return (*cntx)->SendError("CLUSTER commands requires --cluster_mode=emulated"); @@ -1308,13 +1308,13 @@ void ServerFamily::Cluster(CmdArgList args, ConnectionContext* cntx) { } void ServerFamily::Config(CmdArgList args, ConnectionContext* cntx) { - ToUpper(&args[1]); - string_view sub_cmd = ArgS(args, 1); + ToUpper(&args[0]); + string_view sub_cmd = ArgS(args, 0); if (sub_cmd == "SET") { return (*cntx)->SendOk(); - } else if (sub_cmd == "GET" && args.size() == 3) { - string_view param = ArgS(args, 2); + } else if (sub_cmd == "GET" && args.size() == 2) { + string_view param = ArgS(args, 1); string_view res[2] = {param, "tbd"}; return (*cntx)->SendStringArr(res, RedisReplyBuilder::MAP); @@ -1333,7 +1333,7 @@ void ServerFamily::Config(CmdArgList args, ConnectionContext* cntx) { } void ServerFamily::Debug(CmdArgList args, ConnectionContext* cntx) { - ToUpper(&args[1]); + ToUpper(&args[0]); DebugCmd dbg_cmd{this, cntx}; @@ -1341,7 +1341,7 @@ void ServerFamily::Debug(CmdArgList args, ConnectionContext* cntx) { } void ServerFamily::Memory(CmdArgList args, ConnectionContext* cntx) { - ToUpper(&args[1]); + ToUpper(&args[0]); MemoryCmd mem_cmd{this, cntx}; @@ -1355,9 +1355,9 @@ void ServerFamily::Save(CmdArgList args, ConnectionContext* cntx) { return (*cntx)->SendError(kSyntaxErr); } - if (args.size() == 2) { - ToUpper(&args[1]); - string_view sub_cmd = ArgS(args, 1); + if (args.size() == 1) { + ToUpper(&args[0]); + string_view sub_cmd = ArgS(args, 0); if (sub_cmd == "DF") { new_version = true; } else if (sub_cmd == "RDB") { @@ -1424,15 +1424,15 @@ Metrics ServerFamily::GetMetrics() const { } void ServerFamily::Info(CmdArgList args, ConnectionContext* cntx) { - if (args.size() > 2) { + if (args.size() > 1) { return (*cntx)->SendError(kSyntaxErr); } string_view section; - if (args.size() == 2) { - ToUpper(&args[1]); - section = ArgS(args, 1); + if (args.size() == 1) { + ToUpper(&args[0]); + section = ArgS(args, 0); } string info; @@ -1691,12 +1691,12 @@ void ServerFamily::Hello(CmdArgList args, ConnectionContext* cntx) { // If no arguments are provided default to RESP2. // AUTH and SETNAME options are not supported. bool is_resp3 = false; - if (args.size() > 1) { - string_view proto_version = ArgS(args, 1); + if (args.size() > 0) { + string_view proto_version = ArgS(args, 0); is_resp3 = proto_version == "3"; bool valid_proto_version = proto_version == "2" || is_resp3; - if (!valid_proto_version || args.size() > 2) { - (*cntx)->SendError(UnknownCmd("HELLO", args.subspan(1))); + if (!valid_proto_version || args.size() > 1) { + (*cntx)->SendError(UnknownCmd("HELLO", args)); return; } } @@ -1764,8 +1764,8 @@ std::string ServerFamily::BuildClusterNodeReply(ConnectionContext* cntx) const { } void ServerFamily::ReplicaOf(CmdArgList args, ConnectionContext* cntx) { - std::string_view host = ArgS(args, 1); - std::string_view port_s = ArgS(args, 2); + std::string_view host = ArgS(args, 0); + std::string_view port_s = ArgS(args, 1); auto& pool = service_.proactor_pool(); LOG(INFO) << "Replicating " << host << ":" << port_s; @@ -1838,16 +1838,16 @@ void ServerFamily::ReplicaOf(CmdArgList args, ConnectionContext* cntx) { } void ServerFamily::ReplConf(CmdArgList args, ConnectionContext* cntx) { - if (args.size() % 2 == 0) + if (args.size() % 2 == 1) goto err; - for (unsigned i = 1; i < args.size(); i += 2) { + for (unsigned i = 0; i < args.size(); i += 2) { DCHECK_LT(i + 1, args.size()); ToUpper(&args[i]); std::string_view cmd = ArgS(args, i); std::string_view arg = ArgS(args, i + 1); if (cmd == "CAPA") { - if (arg == "dragonfly" && args.size() == 3 && i == 1) { + if (arg == "dragonfly" && args.size() == 2 && i == 0) { uint32_t sid = dfly_cmd_->CreateSyncSession(cntx); cntx->owner()->SetName(absl::StrCat("repl_ctrl_", sid)); @@ -1922,7 +1922,6 @@ void ServerFamily::Role(CmdArgList args, ConnectionContext* cntx) { } void ServerFamily::Script(CmdArgList args, ConnectionContext* cntx) { - args.remove_prefix(1); ToUpper(&args.front()); script_mgr_->Run(std::move(args), cntx); @@ -1953,8 +1952,8 @@ void ServerFamily::LastSave(CmdArgList args, ConnectionContext* cntx) { } void ServerFamily::Latency(CmdArgList args, ConnectionContext* cntx) { - ToUpper(&args[1]); - string_view sub_cmd = ArgS(args, 1); + ToUpper(&args[0]); + string_view sub_cmd = ArgS(args, 01); if (sub_cmd == "LATEST") { return (*cntx)->SendEmptyArray(); diff --git a/src/server/set_family.cc b/src/server/set_family.cc index 3fd4b1627..e9bc01e20 100644 --- a/src/server/set_family.cc +++ b/src/server/set_family.cc @@ -1042,10 +1042,10 @@ OpResult OpScan(const OpArgs& op_args, string_view key, uint64_t* cur } void SAdd(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - vector vals(args.size() - 2); - for (size_t i = 2; i < args.size(); ++i) { - vals[i - 2] = ArgS(args, i); + string_view key = ArgS(args, 0); + vector vals(args.size() - 1); + for (size_t i = 1; i < args.size(); ++i) { + vals[i - 1] = ArgS(args, i); } ArgSlice arg_slice{vals.data(), vals.size()}; @@ -1062,8 +1062,8 @@ void SAdd(CmdArgList args, ConnectionContext* cntx) { } void SIsMember(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view val = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view val = ArgS(args, 1); auto cb = [&](Transaction* t, EngineShard* shard) { OpResult find_res = shard->db_slice().Find(t->GetDbContext(), key, OBJ_SET); @@ -1086,11 +1086,11 @@ void SIsMember(CmdArgList args, ConnectionContext* cntx) { } void SMIsMember(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); - vector vals(args.size() - 2); - for (size_t i = 2; i < args.size(); ++i) { - vals[i - 2] = ArgS(args, i); + vector vals(args.size() - 1); + for (size_t i = 1; i < args.size(); ++i) { + vals[i - 1] = ArgS(args, i); } StringVec memberships; @@ -1117,9 +1117,9 @@ void SMIsMember(CmdArgList args, ConnectionContext* cntx) { } void SMove(CmdArgList args, ConnectionContext* cntx) { - string_view src = ArgS(args, 1); - string_view dest = ArgS(args, 2); - string_view member = ArgS(args, 3); + string_view src = ArgS(args, 0); + string_view dest = ArgS(args, 1); + string_view member = ArgS(args, 2); Mover mover{src, dest, member, true}; cntx->transaction->Schedule(); @@ -1136,10 +1136,10 @@ void SMove(CmdArgList args, ConnectionContext* cntx) { } void SRem(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - vector vals(args.size() - 2); - for (size_t i = 2; i < args.size(); ++i) { - vals[i - 2] = ArgS(args, i); + string_view key = ArgS(args, 0); + vector vals(args.size() - 1); + for (size_t i = 1; i < args.size(); ++i) { + vals[i - 1] = ArgS(args, i); } ArgSlice span{vals.data(), vals.size()}; @@ -1159,7 +1159,7 @@ void SRem(CmdArgList args, ConnectionContext* cntx) { } void SCard(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) -> OpResult { OpResult find_res = shard->db_slice().Find(t->GetDbContext(), key, OBJ_SET); @@ -1183,10 +1183,10 @@ void SCard(CmdArgList args, ConnectionContext* cntx) { } void SPop(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); unsigned count = 1; - if (args.size() > 2) { - string_view arg = ArgS(args, 2); + if (args.size() > 1) { + string_view arg = ArgS(args, 1); if (!absl::SimpleAtoi(arg, &count)) { (*cntx)->SendError(kInvalidIntErr); return; @@ -1199,7 +1199,7 @@ void SPop(CmdArgList args, ConnectionContext* cntx) { OpResult result = cntx->transaction->ScheduleSingleHopT(std::move(cb)); if (result || result.status() == OpStatus::KEY_NOTFOUND) { - if (args.size() == 2) { // SPOP key + if (args.size() == 1) { // SPOP key if (result.status() == OpStatus::KEY_NOTFOUND) { (*cntx)->SendNull(); } else { @@ -1217,7 +1217,7 @@ void SPop(CmdArgList args, ConnectionContext* cntx) { void SDiff(CmdArgList args, ConnectionContext* cntx) { ResultStringVec result_set(shard_set->size(), OpStatus::SKIPPED); - string_view src_key = ArgS(args, 1); + string_view src_key = ArgS(args, 0); ShardId src_shard = Shard(src_key, result_set.size()); auto cb = [&](Transaction* t, EngineShard* shard) { @@ -1248,9 +1248,9 @@ void SDiff(CmdArgList args, ConnectionContext* cntx) { void SDiffStore(CmdArgList args, ConnectionContext* cntx) { ResultStringVec result_set(shard_set->size(), OpStatus::SKIPPED); - string_view dest_key = ArgS(args, 1); + string_view dest_key = ArgS(args, 0); ShardId dest_shard = Shard(dest_key, result_set.size()); - string_view src_key = ArgS(args, 2); + string_view src_key = ArgS(args, 1); ShardId src_shard = Shard(src_key, result_set.size()); VLOG(1) << "SDiffStore " << src_key << " " << src_shard; @@ -1341,7 +1341,7 @@ void SInter(CmdArgList args, ConnectionContext* cntx) { void SInterStore(CmdArgList args, ConnectionContext* cntx) { ResultStringVec result_set(shard_set->size(), OpStatus::SKIPPED); - string_view dest_key = ArgS(args, 1); + string_view dest_key = ArgS(args, 0); ShardId dest_shard = Shard(dest_key, result_set.size()); atomic_uint32_t inter_shard_cnt{0}; @@ -1404,7 +1404,7 @@ void SUnion(CmdArgList args, ConnectionContext* cntx) { void SUnionStore(CmdArgList args, ConnectionContext* cntx) { ResultStringVec result_set(shard_set->size(), OpStatus::SKIPPED); - string_view dest_key = ArgS(args, 1); + string_view dest_key = ArgS(args, 0); ShardId dest_shard = Shard(dest_key, result_set.size()); auto union_cb = [&](Transaction* t, EngineShard* shard) { @@ -1444,8 +1444,8 @@ void SUnionStore(CmdArgList args, ConnectionContext* cntx) { } void SScan(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view token = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view token = ArgS(args, 1); uint64_t cursor = 0; @@ -1454,12 +1454,12 @@ void SScan(CmdArgList args, ConnectionContext* cntx) { } // SSCAN key cursor [MATCH pattern] [COUNT count] - if (args.size() > 7) { + if (args.size() > 6) { DVLOG(1) << "got " << args.size() << " this is more than it should be"; return (*cntx)->SendError(kSyntaxErr); } - OpResult ops = ScanOpts::TryFrom(args.subspan(3)); + OpResult ops = ScanOpts::TryFrom(args.subspan(2)); if (!ops) { DVLOG(1) << "SScan invalid args - return " << ops << " to the user"; return (*cntx)->SendError(ops.status()); @@ -1486,8 +1486,8 @@ void SScan(CmdArgList args, ConnectionContext* cntx) { // Syntax: saddex key ttl_sec member [member...] void SAddEx(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view ttl_str = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view ttl_str = ArgS(args, 1); uint32_t ttl_sec; constexpr uint32_t kMaxTtl = (1UL << 26); diff --git a/src/server/stream_family.cc b/src/server/stream_family.cc index 1f0bab955..8dc2162fe 100644 --- a/src/server/stream_family.cc +++ b/src/server/stream_family.cc @@ -566,8 +566,8 @@ void SetId(string_view key, string_view gname, CmdArgList args, ConnectionContex } // namespace void StreamFamily::XAdd(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - unsigned id_indx = 2; + string_view key = ArgS(args, 0); + unsigned id_indx = 1; AddOpts add_opts; for (; id_indx < args.size(); ++id_indx) { @@ -592,7 +592,7 @@ void StreamFamily::XAdd(CmdArgList args, ConnectionContext* cntx) { } args.remove_prefix(id_indx); - if (args.size() < 3 || args.size() % 2 == 0) { + if (args.size() < 2 || args.size() % 2 == 0) { return (*cntx)->SendError(WrongNumArgsError("XADD"), kSyntaxErrType); } @@ -622,8 +622,8 @@ void StreamFamily::XAdd(CmdArgList args, ConnectionContext* cntx) { } void StreamFamily::XDel(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - args.remove_prefix(2); + string_view key = ArgS(args, 0); + args.remove_prefix(1); absl::InlinedVector ids(args.size()); @@ -649,8 +649,8 @@ void StreamFamily::XDel(CmdArgList args, ConnectionContext* cntx) { } void StreamFamily::XGroup(CmdArgList args, ConnectionContext* cntx) { - ToUpper(&args[1]); - string_view sub_cmd = ArgS(args, 1); + ToUpper(&args[0]); + string_view sub_cmd = ArgS(args, 0); if (sub_cmd == "HELP") { string_view help_arr[] = { "CREATE [option]", @@ -669,27 +669,27 @@ void StreamFamily::XGroup(CmdArgList args, ConnectionContext* cntx) { return (*cntx)->SendSimpleStrArr(help_arr); } - if (args.size() >= 3) { - string_view key = ArgS(args, 2); + if (args.size() >= 2) { + string_view key = ArgS(args, 1); if (sub_cmd == "CREATE") { - args.remove_prefix(3); + args.remove_prefix(2); return CreateGroup(std::move(args), key, cntx); } - if (sub_cmd == "DESTROY" && args.size() == 4) { - string_view gname = ArgS(args, 3); + if (sub_cmd == "DESTROY" && args.size() == 3) { + string_view gname = ArgS(args, 2); return DestroyGroup(key, gname, cntx); } - if (sub_cmd == "DELCONSUMER" && args.size() == 5) { - string_view gname = ArgS(args, 3); - string_view cname = ArgS(args, 4); + if (sub_cmd == "DELCONSUMER" && args.size() == 4) { + string_view gname = ArgS(args, 2); + string_view cname = ArgS(args, 3); return DelConsumer(key, gname, cname, cntx); } - if (sub_cmd == "SETID" && args.size() >= 5) { - string_view gname = ArgS(args, 3); - args.remove_prefix(4); + if (sub_cmd == "SETID" && args.size() >= 4) { + string_view gname = ArgS(args, 2); + args.remove_prefix(3); return SetId(key, gname, std::move(args), cntx); } } @@ -698,8 +698,8 @@ void StreamFamily::XGroup(CmdArgList args, ConnectionContext* cntx) { } void StreamFamily::XInfo(CmdArgList args, ConnectionContext* cntx) { - ToUpper(&args[1]); - string_view sub_cmd = ArgS(args, 1); + ToUpper(&args[0]); + string_view sub_cmd = ArgS(args, 0); if (sub_cmd == "HELP") { string_view help_arr[] = { "CONSUMERS ", @@ -712,8 +712,8 @@ void StreamFamily::XInfo(CmdArgList args, ConnectionContext* cntx) { return (*cntx)->SendSimpleStrArr(help_arr); } - if (args.size() >= 3) { - string_view key = ArgS(args, 2); + if (args.size() >= 2) { + string_view key = ArgS(args, 1); ShardId sid = Shard(key, shard_set->size()); if (sub_cmd == "GROUPS") { @@ -745,7 +745,7 @@ void StreamFamily::XInfo(CmdArgList args, ConnectionContext* cntx) { } void StreamFamily::XLen(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) { return OpLen(t->GetOpArgs(shard), key); }; OpResult result = cntx->transaction->ScheduleSingleHopT(std::move(cb)); @@ -765,8 +765,8 @@ void StreamFamily::XRevRange(CmdArgList args, ConnectionContext* cntx) { } void StreamFamily::XSetId(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view idstr = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view idstr = ArgS(args, 1); ParsedStreamId parsed_id; if (!ParseID(idstr, true, 0, &parsed_id)) { @@ -792,9 +792,9 @@ void StreamFamily::XSetId(CmdArgList args, ConnectionContext* cntx) { } void StreamFamily::XRangeGeneric(CmdArgList args, bool is_rev, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view start = ArgS(args, 2); - string_view end = ArgS(args, 3); + string_view key = ArgS(args, 0); + string_view start = ArgS(args, 1); + string_view end = ArgS(args, 2); RangeOpts range_opts; RangeId rs, re; if (!ParseRangeId(start, &rs) || !ParseRangeId(end, &re)) { @@ -809,13 +809,13 @@ void StreamFamily::XRangeGeneric(CmdArgList args, bool is_rev, ConnectionContext return (*cntx)->SendError("invalid end ID for the interval", kSyntaxErrType); } - if (args.size() > 4) { - if (args.size() != 6) { + if (args.size() > 3) { + if (args.size() != 5) { return (*cntx)->SendError(WrongNumArgsError("XRANGE"), kSyntaxErrType); } - ToUpper(&args[4]); - string_view opt = ArgS(args, 4); - string_view val = ArgS(args, 5); + ToUpper(&args[3]); + string_view opt = ArgS(args, 3); + string_view val = ArgS(args, 4); if (opt != "COUNT" || !absl::SimpleAtoi(val, &range_opts.count)) { return (*cntx)->SendError(kSyntaxErr); diff --git a/src/server/string_family.cc b/src/server/string_family.cc index 504cab63d..788993cc9 100644 --- a/src/server/string_family.cc +++ b/src/server/string_family.cc @@ -663,8 +663,8 @@ void SetCmd::RecordJournal(const SetParams& params, string_view key, string_view void StringFamily::Set(CmdArgList args, ConnectionContext* cntx) { set_qps.Inc(); - string_view key = ArgS(args, 1); - string_view value = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view value = ArgS(args, 1); SetCmd::SetParams sparams; sparams.memcache_flags = cntx->conn_state.memcache_flag; @@ -672,7 +672,7 @@ void StringFamily::Set(CmdArgList args, ConnectionContext* cntx) { int64_t int_arg; SinkReplyBuilder* builder = cntx->reply_builder(); - for (size_t i = 3; i < args.size(); ++i) { + for (size_t i = 2; i < args.size(); ++i) { ToUpper(&args[i]); string_view cur_arg = ArgS(args, i); @@ -768,8 +768,8 @@ void StringFamily::SetNx(CmdArgList args, ConnectionContext* cntx) { // change the value only if the key does not exist. Otherwise the function // will not modify it. in which case it would return 0 // it would return to the caller 1 in case the key did not exists and was added - string_view key = ArgS(args, 1); - string_view value = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view value = ArgS(args, 1); SetCmd::SetParams sparams; sparams.flags |= SetCmd::SET_IF_NOTEXIST; @@ -789,7 +789,7 @@ void StringFamily::SetNx(CmdArgList args, ConnectionContext* cntx) { void StringFamily::Get(CmdArgList args, ConnectionContext* cntx) { get_qps.Inc(); - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) { return OpGet(t->GetOpArgs(shard), key); }; @@ -815,7 +815,7 @@ void StringFamily::Get(CmdArgList args, ConnectionContext* cntx) { void StringFamily::GetDel(CmdArgList args, ConnectionContext* cntx) { get_qps.Inc(); - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) { bool run_del = true; @@ -843,8 +843,8 @@ void StringFamily::GetDel(CmdArgList args, ConnectionContext* cntx) { } void StringFamily::GetSet(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view value = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view value = ArgS(args, 1); std::optional prev_val; SetCmd::SetParams sparams; @@ -871,12 +871,12 @@ void StringFamily::GetSet(CmdArgList args, ConnectionContext* cntx) { } void StringFamily::GetEx(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); DbSlice::ExpireParams exp_params; int64_t int_arg = 0; - for (size_t i = 2; i < args.size(); i++) { + for (size_t i = 1; i < args.size(); i++) { ToUpper(&args[i]); string_view cur_arg = ArgS(args, i); @@ -949,15 +949,13 @@ void StringFamily::GetEx(CmdArgList args, ConnectionContext* cntx) { } void StringFamily::Incr(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); return IncrByGeneric(key, 1, cntx); } void StringFamily::IncrBy(CmdArgList args, ConnectionContext* cntx) { - DCHECK_EQ(3u, args.size()); - - string_view key = ArgS(args, 1); - string_view sval = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view sval = ArgS(args, 1); int64_t val; if (!absl::SimpleAtoi(sval, &val)) { @@ -967,8 +965,8 @@ void StringFamily::IncrBy(CmdArgList args, ConnectionContext* cntx) { } void StringFamily::IncrByFloat(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view sval = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view sval = ArgS(args, 1); double val; if (!absl::SimpleAtod(sval, &val)) { @@ -991,13 +989,13 @@ void StringFamily::IncrByFloat(CmdArgList args, ConnectionContext* cntx) { } void StringFamily::Decr(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); return IncrByGeneric(key, -1, cntx); } void StringFamily::DecrBy(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view sval = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view sval = ArgS(args, 1); int64_t val; if (!absl::SimpleAtoi(sval, &val)) { @@ -1050,8 +1048,8 @@ void StringFamily::IncrByGeneric(string_view key, int64_t val, ConnectionContext } void StringFamily::ExtendGeneric(CmdArgList args, bool prepend, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view sval = ArgS(args, 2); + string_view key = ArgS(args, 0); + string_view sval = ArgS(args, 1); if (cntx->protocol() == Protocol::REDIS) { auto cb = [&](Transaction* t, EngineShard* shard) { @@ -1081,9 +1079,9 @@ void StringFamily::ExtendGeneric(CmdArgList args, bool prepend, ConnectionContex /// (P)SETEX key seconds value void StringFamily::SetExGeneric(bool seconds, CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view ex = ArgS(args, 2); - string_view value = ArgS(args, 3); + string_view key = ArgS(args, 0); + string_view ex = ArgS(args, 1); + string_view value = ArgS(args, 2); int32_t unit_vals; if (!absl::SimpleAtoi(ex, &unit_vals)) { @@ -1113,7 +1111,7 @@ void StringFamily::SetExGeneric(bool seconds, CmdArgList args, ConnectionContext } void StringFamily::MGet(CmdArgList args, ConnectionContext* cntx) { - DCHECK_GT(args.size(), 1U); + DCHECK_GE(args.size(), 1U); Transaction* transaction = cntx->transaction; unsigned shard_count = shard_set->size(); @@ -1137,7 +1135,7 @@ void StringFamily::MGet(CmdArgList args, ConnectionContext* cntx) { CHECK_EQ(OpStatus::OK, result); // reorder the responses back according to the order of their corresponding keys. - vector res(args.size() - 1); + vector res(args.size()); for (ShardId sid = 0; sid < shard_count; ++sid) { if (!transaction->IsActive(sid)) @@ -1157,7 +1155,7 @@ void StringFamily::MGet(CmdArgList args, ConnectionContext* cntx) { auto& dest = res[indx].emplace(); auto& src = *results[j]; - dest.key = ArgS(args, indx + 1); + dest.key = ArgS(args, indx); dest.value = std::move(src.value); dest.mc_flag = src.mc_flag; dest.mc_ver = src.mc_ver; @@ -1228,7 +1226,7 @@ void StringFamily::MSetNx(CmdArgList args, ConnectionContext* cntx) { } void StringFamily::StrLen(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); + string_view key = ArgS(args, 0); auto cb = [&](Transaction* t, EngineShard* shard) -> OpResult { OpResult it_res = shard->db_slice().Find(t->GetDbContext(), key, OBJ_STRING); @@ -1249,9 +1247,9 @@ void StringFamily::StrLen(CmdArgList args, ConnectionContext* cntx) { } void StringFamily::GetRange(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view from = ArgS(args, 2); - string_view to = ArgS(args, 3); + string_view key = ArgS(args, 0); + string_view from = ArgS(args, 1); + string_view to = ArgS(args, 2); int32_t start, end; if (!absl::SimpleAtoi(from, &start) || !absl::SimpleAtoi(to, &end)) { @@ -1273,9 +1271,9 @@ void StringFamily::GetRange(CmdArgList args, ConnectionContext* cntx) { } void StringFamily::SetRange(CmdArgList args, ConnectionContext* cntx) { - string_view key = ArgS(args, 1); - string_view offset = ArgS(args, 2); - string_view value = ArgS(args, 3); + string_view key = ArgS(args, 0); + string_view offset = ArgS(args, 1); + string_view value = ArgS(args, 2); int32_t start; if (!absl::SimpleAtoi(offset, &start)) { @@ -1347,33 +1345,33 @@ auto StringFamily::OpMGet(bool fetch_mcflag, bool fetch_mcver, const Transaction * X-RateLimit-Reset. */ void StringFamily::ClThrottle(CmdArgList args, ConnectionContext* cntx) { - const string_view key = ArgS(args, 1); + const string_view key = ArgS(args, 0); // Allow max burst in number of tokens uint64_t max_burst; - const string_view max_burst_str = ArgS(args, 2); + const string_view max_burst_str = ArgS(args, 1); if (!absl::SimpleAtoi(max_burst_str, &max_burst)) { return (*cntx)->SendError(kInvalidIntErr); } // Emit count of tokens per period uint64_t count; - const string_view count_str = ArgS(args, 3); + const string_view count_str = ArgS(args, 2); if (!absl::SimpleAtoi(count_str, &count)) { return (*cntx)->SendError(kInvalidIntErr); } // Period of emitting count of tokens uint64_t period; - const string_view period_str = ArgS(args, 4); + const string_view period_str = ArgS(args, 3); if (!absl::SimpleAtoi(period_str, &period)) { return (*cntx)->SendError(kInvalidIntErr); } // Apply quantity of tokens now uint64_t quantity = 1; - if (args.size() > 5) { - const string_view quantity_str = ArgS(args, 5); + if (args.size() > 4) { + const string_view quantity_str = ArgS(args, 4); if (!absl::SimpleAtoi(quantity_str, &quantity)) { return (*cntx)->SendError(kInvalidIntErr); diff --git a/src/server/test_utils.cc b/src/server/test_utils.cc index beee86980..815dc7e70 100644 --- a/src/server/test_utils.cc +++ b/src/server/test_utils.cc @@ -344,12 +344,16 @@ RespVec BaseFamilyTest::TestConnWrapper::ParseResponse(bool fully_consumed) { tmp_str_vec_.emplace_back(new string{sink_.str()}); auto& s = *tmp_str_vec_.back(); auto buf = RespExpr::buffer(&s); - uint32_t consumed = 0; + auto s_copy = s; + + uint32_t consumed = 0; parser_.reset(new RedisParser{false}); // Client mode. RespVec res; RedisParser::Result st = parser_->Parse(buf, &consumed, &res); - CHECK_EQ(RedisParser::OK, st); + + CHECK_EQ(RedisParser::OK, st) << " response: \"" << s_copy << "\" (" << s_copy.size() + << " chars)"; if (fully_consumed) { DCHECK_EQ(consumed, s.size()) << s; } diff --git a/src/server/transaction.cc b/src/server/transaction.cc index 447c350b4..dffffabd8 100644 --- a/src/server/transaction.cc +++ b/src/server/transaction.cc @@ -67,7 +67,7 @@ Transaction::~Transaction() { void Transaction::InitBase(DbIndex dbid, CmdArgList args) { global_ = false; db_index_ = dbid; - cmd_with_full_args_ = args; + full_args_ = args; local_result_ = OpStatus::OK; } @@ -83,21 +83,21 @@ void Transaction::InitGlobal() { void Transaction::BuildShardIndex(KeyIndex key_index, bool rev_mapping, std::vector* out) { - auto args = cmd_with_full_args_; + auto args = full_args_; auto& shard_index = *out; auto add = [this, rev_mapping, &shard_index](uint32_t sid, uint32_t i) { - string_view val = ArgS(cmd_with_full_args_, i); + string_view val = ArgS(full_args_, i); shard_index[sid].args.push_back(val); if (rev_mapping) - shard_index[sid].original_index.push_back(i - 1); + shard_index[sid].original_index.push_back(i); }; if (key_index.bonus) { DCHECK(key_index.step == 1); - uint32_t sid = Shard(ArgS(args, key_index.bonus), shard_data_.size()); - add(sid, key_index.bonus); + uint32_t sid = Shard(ArgS(args, *key_index.bonus), shard_data_.size()); + add(sid, *key_index.bonus); } for (unsigned i = key_index.start; i < key_index.end; ++i) { @@ -157,7 +157,6 @@ void Transaction::InitShardData(absl::Span shard_index, siz void Transaction::InitMultiData(KeyIndex key_index) { DCHECK(multi_); - auto args = cmd_with_full_args_; if (multi_->mode == NON_ATOMIC) return; @@ -184,9 +183,9 @@ void Transaction::InitMultiData(KeyIndex key_index) { // for eval. currently, we lock everything only during the eval call. if (multi_->IsIncrLocks() || !multi_->locks_recorded) { for (size_t i = key_index.start; i < key_index.end; i += key_index.step) - lock_key(ArgS(args, i)); - if (key_index.bonus > 0) - lock_key(ArgS(args, key_index.bonus)); + lock_key(ArgS(full_args_, i)); + if (key_index.bonus) + lock_key(ArgS(full_args_, *key_index.bonus)); } multi_->locks_recorded = true; @@ -195,22 +194,20 @@ void Transaction::InitMultiData(KeyIndex key_index) { } void Transaction::StoreKeysInArgs(KeyIndex key_index, bool rev_mapping) { - DCHECK_EQ(key_index.bonus, 0U); - - auto args = cmd_with_full_args_; + DCHECK(!key_index.bonus); DCHECK(key_index.step == 1u || key_index.step == 2u); // even for a single key we may have multiple arguments per key (MSET). for (unsigned j = key_index.start; j < key_index.end; j++) { - args_.push_back(ArgS(args, j)); + args_.push_back(ArgS(full_args_, j)); if (key_index.step == 2) - args_.push_back(ArgS(args, ++j)); + args_.push_back(ArgS(full_args_, ++j)); } if (rev_mapping) { reverse_index_.resize(args_.size()); for (unsigned j = 0; j < reverse_index_.size(); ++j) { - reverse_index_[j] = j + key_index.start - 1; + reverse_index_[j] = j + key_index.start; } } } @@ -236,7 +233,7 @@ void Transaction::StoreKeysInArgs(KeyIndex key_index, bool rev_mapping) { **/ void Transaction::InitByKeys(KeyIndex key_index) { - auto args = cmd_with_full_args_; + auto args = full_args_; if (key_index.start == args.size()) { // eval with 0 keys. CHECK(absl::StartsWith(cid_->name(), "EVAL")); @@ -266,7 +263,7 @@ void Transaction::InitByKeys(KeyIndex key_index) { shard_data_.resize(shard_set->size()); // shard_data isn't sparse, so we must allocate for all :( CHECK(key_index.step == 1 || key_index.step == 2); - DCHECK(key_index.step == 1 || (args.size() % 2) == 1); + DCHECK(key_index.step == 1 || (args.size() % 2) == 0); // Safe, because flow below is not preemptive. auto& shard_index = tmp_space.GetShardIndex(shard_data_.size()); @@ -299,7 +296,7 @@ void Transaction::InitByKeys(KeyIndex key_index) { // Validation. Check reverse mapping was built correctly. if (needs_reverse_mapping) { for (size_t i = 0; i < args_.size(); ++i) { - DCHECK_EQ(args_[i], ArgS(args, 1 + reverse_index_[i])); // 1 for the commandname. + DCHECK_EQ(args_[i], ArgS(args, reverse_index_[i])); } } @@ -322,7 +319,6 @@ OpStatus Transaction::InitByArgs(DbIndex index, CmdArgList args) { return OpStatus::OK; } - CHECK_GT(args.size(), 1U); // first entry is the command name. DCHECK_EQ(unique_shard_cnt_, 0u); DCHECK(args_.empty()); @@ -1360,11 +1356,11 @@ void Transaction::LogAutoJournalOnShard(EngineShard* shard) { // TODO: Handle complex commands like LMPOP correctly once they are implemented. journal::Entry::Payload entry_payload; + + string_view cmd{cid_->name()}; if (unique_shard_cnt_ == 1 || args_.empty()) { - CHECK(!cmd_with_full_args_.empty()); - entry_payload = cmd_with_full_args_; + entry_payload = make_pair(cmd, full_args_); } else { - auto cmd = facade::ToSV(cmd_with_full_args_.front()); entry_payload = make_pair(cmd, GetShardArgs(shard->shard_id())); } LogJournalOnShard(shard, std::move(entry_payload), unique_shard_cnt_, false, true); @@ -1411,17 +1407,20 @@ OpResult DetermineKeys(const CommandId* cid, CmdArgList args) { if (cid->opt_mask() & CO::VARIADIC_KEYS) { // ZUNION/INTER [ ...] // EVAL