fix: limit parsing in zrange commands (#3626)

1. The offset value can be negative, in that case we should return an empty array.
2. Fix edge cases of inf*0 and -inf + inf, so they will result in 0 and non NaN (similarily to Valkey).

Signed-off-by: Roman Gershman <roman@dragonflydb.io>
This commit is contained in:
Roman Gershman 2024-09-03 10:08:45 +03:00 committed by GitHub
parent d40e9088ae
commit 1a0d44354f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 34 additions and 5 deletions

View File

@ -67,14 +67,14 @@ jobs:
echo "-----------------------------"
# The order of redirect is important
./dragonfly --proactor_threads=4 --dbfilename= --noversion_check --port=6380 \
./dragonfly --proactor_threads=4 --noversion_check --port=6380 \
--lua_resp2_legacy_float 1> /tmp/dragonfly.log 2>&1 &
- name: Run tests
working-directory: tests/fakeredis
run: |
poetry run pytest test/ --ignore test/test_hypothesis.py --ignore test/test_json/ \
--ignore test_bitmap_commands.py --ignore test/test_mixins/ \
--ignore test_bitmap_commands.py \
--junit-xml=results-tests.xml --html=report-tests.html -v
continue-on-error: true # For now to mark the flow as successful

View File

@ -81,6 +81,9 @@ void RespMatcher::DescribeTo(std::ostream* os) const {
case RespExpr::ARRAY:
*os << "array of length " << exp_int_;
break;
case RespExpr::DOUBLE:
*os << exp_double_;
break;
default:
*os << "TBD";
break;

View File

@ -731,6 +731,8 @@ ScoredMap FromObject(const CompactObj& co, double weight) {
for (auto& elem : arr) {
elem.second *= weight;
if (isnan(elem.second))
elem.second = 0;
res.emplace(std::move(elem));
}
@ -749,7 +751,8 @@ ScoredMap ScoreMapFromSet(const PrimeValue& pv, double weight) {
double Aggregate(double v1, double v2, AggType atype) {
switch (atype) {
case AggType::SUM:
return v1 + v2;
v1 += v2;
return isnan(v1) ? 0 : v1;
case AggType::MAX:
return max(v1, v2);
case AggType::MIN:
@ -2161,15 +2164,21 @@ void ZSetFamily::ZRangeGeneric(CmdArgList args, ConnectionContext* cntx, RangePa
}
if (parser.Check("LIMIT")) {
int64_t limit;
tie(range_params.offset, limit) = parser.Next<uint32_t, int64_t>();
auto [offset, limit] = parser.Next<int32_t, int32_t>();
range_params.limit = limit < 0 ? UINT32_MAX : static_cast<uint32_t>(limit);
range_params.offset = offset < 0 ? UINT32_MAX : static_cast<uint32_t>(offset);
continue;
}
return cntx->SendError(absl::StrCat("unsupported option ", parser.Peek()));
}
if (range_params.offset == UINT32_MAX) {
auto* rb = static_cast<RedisReplyBuilder*>(cntx->reply_builder());
return rb->SendEmptyArray();
}
ZRangeInternal(args.subspan(0, 3), range_params, cntx);
}

View File

@ -637,6 +637,19 @@ TEST_F(ZSetFamilyTest, ZUnionStoreOpts) {
ASSERT_THAT(resp, IntArg(3));
resp = Run({"zrange", "max", "0", "-1", "withscores"});
EXPECT_THAT(resp.GetVec(), ElementsAre("c", "0", "a", "2", "b", "4"));
// Check that infinity is handled correctly.
Run({"ZADD", "src1", "inf", "x"});
Run({"ZADD", "src2", "inf", "x"});
Run({"ZUNIONSTORE", "dest", "2", "src1", "src2", "WEIGHTS", "1.0", "0.0"});
resp = Run({"ZSCORE", "dest", "x"});
EXPECT_THAT(resp, DoubleArg(numeric_limits<double>::infinity()));
Run({"ZADD", "foo", "inf", "e1"});
Run({"ZADD", "bar", "-inf", "e1", "0.0", "e2"});
Run({"ZUNIONSTORE", "dest", "3", "foo", "bar", "foo"});
resp = Run({"ZSCORE", "dest", "e1"});
EXPECT_THAT(resp, DoubleArg(0));
}
TEST_F(ZSetFamilyTest, ZInterStore) {
@ -1143,6 +1156,9 @@ TEST_F(ZSetFamilyTest, RangeLimit) {
resp = Run({"ZRANGEBYSCORE", "", "0.0", "0.0", "foo"});
EXPECT_THAT(resp, ErrArg("unsupported option"));
resp = Run({"ZRANGEBYLEX", "foo", "-", "+", "LIMIT", "-1", "3"});
EXPECT_THAT(resp, ArrLen(0));
}
} // namespace dfly

View File

@ -591,6 +591,7 @@ def test_lua_log_different_types(r, caplog):
]
@pytest.mark.unsupported_server_types("dragonfly")
def test_lua_log_wrong_level(r: redis.Redis):
script = "redis.log(10, 'string')"
script = r.register_script(script)