Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

bug: the return value of Pika ZPOPMAX is not the same as Redis #2182

Closed
Mixficsol opened this issue Dec 6, 2023 · 2 comments
Closed

bug: the return value of Pika ZPOPMAX is not the same as Redis #2182

Mixficsol opened this issue Dec 6, 2023 · 2 comments
Labels

Comments

@Mixficsol
Copy link
Collaborator

Mixficsol commented Dec 6, 2023

Is this a regression?

Yes

Description

ZPOPMAX这种参数个数可以变化的命令,当参数个数超过大小时,缺少了一些判断逻辑导致和Redis那边的返回值不一致

Pika
截屏2023-12-06 18 26 31

Redis
截屏2023-12-06 18 27 27

pika/src/pika_zset 文件下的 1422 行,需要对这个 DoInitial函数进行修改

void ZPopmaxCmd::DoInitial() {
  if (!CheckArg(argv_.size())) {
    res_.SetRes(CmdRes::kWrongNum, kCmdNameZPopmax);
    return;
  }
  key_ = argv_[1];
  if (argv_.size() == 2) {
    count_ = 1;
    return;
  }
  if (pstd::string2int(argv_[2].data(), argv_[2].size(), static_cast<int64_t *>(&count_)) == 0) {
    res_.SetRes(CmdRes::kInvalidInt);
    return;
  }
}

希望修改一下对参数大小的逻辑判断,补齐相应的测试

@Mixficsol Mixficsol added the ☢️ Bug Something isn't working label Dec 6, 2023
@AlexStocks AlexStocks changed the title ZPOPMAX命令返回与Redis不一致 bug: the return value of Pika ZPOPMAX is not the same as Redis Dec 6, 2023
@hero-heng
Copy link
Contributor

我负责修改

@Mixficsol
Copy link
Collaborator Author

Mixficsol commented Dec 6, 2023

我负责修改

bool Cmd::CheckArg(uint64_t num) const { return !((arity_ > 0 && num != arity_) || (arity_ < 0 && num < -arity_)); }

Pika 的这个 CheckArg 方法对于那种参数个数不固定的命令只做了最低参数个数要求的判断,对于参数过多的情况有的没有做判断,除了ZPOPMAX命令应该还有其他的,辛苦排查一下~

这里我给了一个已经做了判断的命令样本,这里对argv_.size() > 3大小做了判断

void SRandmemberCmd::DoInitial() {
  if (!CheckArg(argv_.size())) {
    res_.SetRes(CmdRes::kWrongNum, kCmdNameSRandmember);
    return;
  }
  key_ = argv_[1];
  if (argv_.size() > 3) {
    res_.SetRes(CmdRes::kWrongNum, kCmdNameSRandmember);
    return;
  } else if (argv_.size() == 3) {
    if (pstd::string2int(argv_[2].data(), argv_[2].size(), &count_) == 0) {
      res_.SetRes(CmdRes::kInvalidInt);
    } else {
      reply_arr = true;
    }
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants