This is an automated email from the ASF dual-hosted git repository.
git-hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 7c4972814 fix(geo): make geo store commands return the resulting set
size with COUNT (#3457)
7c4972814 is described below
commit 7c4972814fcbeee94011fc17cafc542798379a51
Author: 纪华裕 <[email protected]>
AuthorDate: Tue Apr 21 22:22:54 2026 +0800
fix(geo): make geo store commands return the resulting set size with COUNT
(#3457)
Fix the return value of geo store commands when `COUNT` is specified.
Previously, these commands stored only the limited number of members in
the destination zset, but returned the full matched result count instead
of the actual stored count.
Assisted-by: Codex
---
src/commands/cmd_geo.cc | 25 +++++++-----
tests/gocase/unit/geo/geo_test.go | 85 +++++++++++++++++++++++++++++++++++++++
2 files changed, 101 insertions(+), 9 deletions(-)
diff --git a/src/commands/cmd_geo.cc b/src/commands/cmd_geo.cc
index d6f7921d1..be5cf32a1 100644
--- a/src/commands/cmd_geo.cc
+++ b/src/commands/cmd_geo.cc
@@ -88,6 +88,15 @@ class CommandGeoBase : public Commander {
return conversion;
}
+ static size_t GetReturnedItemsCount(size_t result_length, int count) {
+ if (count == 0) {
+ return result_length;
+ }
+
+ const auto requested_count = static_cast<size_t>(count);
+ return result_length < requested_count ? result_length : requested_count;
+ }
+
protected:
DistanceUnit distance_unit_ = kDistanceMeter;
};
@@ -316,7 +325,7 @@ class CommandGeoRadius : public CommandGeoBase {
}
if (store_key_.size() != 0) {
- *output = redis::Integer(geo_points.size());
+ *output = redis::Integer(GetReturnedItemsCount(geo_points.size(),
count_));
} else {
*output = GenerateOutput(conn, geo_points);
}
@@ -324,10 +333,9 @@ class CommandGeoRadius : public CommandGeoBase {
}
std::string GenerateOutput(const Connection *conn, const
std::vector<GeoPoint> &geo_points) {
- int result_length = static_cast<int>(geo_points.size());
- int returned_items_count = (count_ == 0 || result_length < count_) ?
result_length : count_;
+ size_t returned_items_count = GetReturnedItemsCount(geo_points.size(),
count_);
std::vector<std::string> list;
- for (int i = 0; i < returned_items_count; i++) {
+ for (size_t i = 0; i < returned_items_count; i++) {
const auto &geo_point = geo_points[i];
if (!with_coord_ && !with_hash_ && !with_dist_) {
list.emplace_back(redis::BulkString(geo_point.member));
@@ -520,11 +528,10 @@ class CommandGeoSearch : public CommandGeoBase {
}
std::string generateOutput(const Connection *conn, const
std::vector<GeoPoint> &geo_points) {
- int result_length = static_cast<int>(geo_points.size());
- int returned_items_count = (count_ == 0 || result_length < count_) ?
result_length : count_;
+ size_t returned_items_count = GetReturnedItemsCount(geo_points.size(),
count_);
std::vector<std::string> output;
output.reserve(returned_items_count);
- for (int i = 0; i < returned_items_count; i++) {
+ for (size_t i = 0; i < returned_items_count; i++) {
const auto &geo_point = geo_points[i];
if (!with_coord_ && !with_hash_ && !with_dist_) {
output.emplace_back(redis::BulkString(geo_point.member));
@@ -625,7 +632,7 @@ class CommandGeoSearchStore : public CommandGeoSearch {
if (!s.ok()) {
return {Status::RedisExecErr, s.ToString()};
}
- *output = redis::Integer(geo_points.size());
+ *output = redis::Integer(GetReturnedItemsCount(geo_points.size(), count_));
return Status::OK();
}
@@ -669,7 +676,7 @@ class CommandGeoRadiusByMember : public CommandGeoRadius {
}
if (store_key_.size() != 0) {
- *output = redis::Integer(geo_points.size());
+ *output = redis::Integer(GetReturnedItemsCount(geo_points.size(),
count_));
} else {
*output = GenerateOutput(conn, geo_points);
}
diff --git a/tests/gocase/unit/geo/geo_test.go
b/tests/gocase/unit/geo/geo_test.go
index fc465d629..b863bd67f 100644
--- a/tests/gocase/unit/geo/geo_test.go
+++ b/tests/gocase/unit/geo/geo_test.go
@@ -287,6 +287,16 @@ var testGeo = func(t *testing.T, configs
util.KvrocksServerConfigs) {
require.EqualValues(t,
[]redis.GeoLocation([]redis.GeoLocation{{Name: "central park n/q/r", Longitude:
0, Latitude: 0, Dist: 0, GeoHash: 0}, {Name: "4545", Longitude: 0, Latitude: 0,
Dist: 0, GeoHash: 0}, {Name: "union square", Longitude: 0, Latitude: 0, Dist:
0, GeoHash: 0}}), rdb.GeoRadius(ctx, "nyc", -73.9798091, 40.7598464,
&redis.GeoRadiusQuery{Radius: 10, Unit: "km", Sort: "asc", Count: 3}).Val())
})
+ t.Run("GEORADIUS with COUNT greater than matches", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "count_points").Err())
+ require.EqualValues(t, 3, rdb.GeoAdd(ctx, "count_points",
+ &redis.GeoLocation{Name: "central park n/q/r",
Longitude: -73.9733487, Latitude: 40.7648057},
+ &redis.GeoLocation{Name: "union square", Longitude:
-73.9903085, Latitude: 40.7362513},
+ &redis.GeoLocation{Name: "4545", Longitude:
-73.9564142, Latitude: 40.7480973}).Val())
+
+ require.EqualValues(t,
[]redis.GeoLocation([]redis.GeoLocation{{Name: "central park n/q/r", Longitude:
0, Latitude: 0, Dist: 0, GeoHash: 0}, {Name: "4545", Longitude: 0, Latitude: 0,
Dist: 0, GeoHash: 0}, {Name: "union square", Longitude: 0, Latitude: 0, Dist:
0, GeoHash: 0}}), rdb.GeoRadius(ctx, "count_points", -73.9798091, 40.7598464,
&redis.GeoRadiusQuery{Radius: 3, Unit: "km", Sort: "asc", Count: 10}).Val())
+ })
+
t.Run("GEORADIUS HUGE, (redis issue #2767)", func(t *testing.T) {
require.NoError(t, rdb.GeoAdd(ctx, "users",
&redis.GeoLocation{Name: "user_000000", Longitude: -47.271613776683807,
Latitude: -54.534504198047678}).Err())
require.EqualValues(t, 1, len(rdb.GeoRadius(ctx, "users", 0, 0,
&redis.GeoRadiusQuery{Radius: 50000, Unit: "km", WithCoord: true}).Val()))
@@ -329,6 +339,18 @@ var testGeo = func(t *testing.T, configs
util.KvrocksServerConfigs) {
require.EqualValues(t,
[]redis.GeoLocation([]redis.GeoLocation{{Name: "wtc one", Longitude: 0,
Latitude: 0, Dist: 0, GeoHash: 0}, {Name: "union square", Longitude: 0,
Latitude: 0, Dist: 0, GeoHash: 0}, {Name: "central park n/q/r", Longitude: 0,
Latitude: 0, Dist: 0, GeoHash: 0}, {Name: "4545", Longitude: 0, Latitude: 0,
Dist: 0, GeoHash: 0}, {Name: "lic market", Longitude: 0, Latitude: 0, Dist: 0,
GeoHash: 0}}), rdb.GeoRadiusByMember(ctx, "nyc", "wtc one",
&redis.GeoRadiusQuery{Radius: [...]
})
+ t.Run("GEORADIUSBYMEMBER with COUNT greater than matches", func(t
*testing.T) {
+ require.NoError(t, rdb.Del(ctx, "count_member_points").Err())
+ require.EqualValues(t, 5, rdb.GeoAdd(ctx, "count_member_points",
+ &redis.GeoLocation{Name: "wtc one", Longitude:
-74.0131604, Latitude: 40.7126674},
+ &redis.GeoLocation{Name: "union square", Longitude:
-73.9903085, Latitude: 40.7362513},
+ &redis.GeoLocation{Name: "central park n/q/r",
Longitude: -73.9733487, Latitude: 40.7648057},
+ &redis.GeoLocation{Name: "4545", Longitude:
-73.9564142, Latitude: 40.7480973},
+ &redis.GeoLocation{Name: "lic market", Longitude:
-73.9454966, Latitude: 40.747533}).Val())
+
+ require.EqualValues(t,
[]redis.GeoLocation([]redis.GeoLocation{{Name: "wtc one", Longitude: 0,
Latitude: 0, Dist: 0, GeoHash: 0}, {Name: "union square", Longitude: 0,
Latitude: 0, Dist: 0, GeoHash: 0}, {Name: "4545", Longitude: 0, Latitude: 0,
Dist: 0, GeoHash: 0}, {Name: "central park n/q/r", Longitude: 0, Latitude: 0,
Dist: 0, GeoHash: 0}, {Name: "lic market", Longitude: 0, Latitude: 0, Dist: 0,
GeoHash: 0}}), rdb.GeoRadiusByMember(ctx, "count_member_points", "wtc one",
&redis.GeoRad [...]
+ })
+
t.Run("GEORADIUSBYMEMBER store option", func(t *testing.T) {
require.NoError(t, rdb.Do(ctx, "DEL", "src", "dst").Err())
@@ -338,6 +360,25 @@ var testGeo = func(t *testing.T, configs
util.KvrocksServerConfigs) {
require.EqualValues(t, []interface{}([]interface{}{"Shenzhen",
"Guangzhou"}), rdb.Do(ctx, "ZRANGE", "dst", 0, -1).Val())
})
+ t.Run("GEORADIUSBYMEMBER STORE with COUNT should return stored item
count", func(t *testing.T) {
+ require.NoError(t, rdb.Do(ctx, "DEL", "points",
"points2").Err())
+ require.NoError(t, rdb.GeoAdd(ctx, "points",
+ &redis.GeoLocation{Name: "Washington", Longitude:
-77.0369, Latitude: 38.9072},
+ &redis.GeoLocation{Name: "Baltimore", Longitude:
-76.6121893, Latitude: 39.2903848},
+ &redis.GeoLocation{Name: "New York", Longitude:
-74.0059413, Latitude: 40.7127837},
+ &redis.GeoLocation{Name: "Philadelphia", Longitude:
-75.16521960, Latitude: 39.95258288}).Err())
+
+ require.EqualValues(t, 1, rdb.GeoRadiusByMemberStore(ctx,
"points", "Washington", &redis.GeoRadiusQuery{
+ Radius: 200,
+ Unit: "km",
+ Sort: "ASC",
+ Count: 1,
+ Store: "points2",
+ }).Val())
+ require.EqualValues(t, 1, rdb.ZCard(ctx, "points2").Val())
+ require.EqualValues(t, []string{"Washington"}, rdb.ZRange(ctx,
"points2", 0, -1).Val())
+ })
+
t.Run("GEOHASH errors", func(t *testing.T) {
require.NoError(t, rdb.Del(ctx, "points").Err())
@@ -461,6 +502,31 @@ var testGeo = func(t *testing.T, configs
util.KvrocksServerConfigs) {
rdb.GeoSearchStore(ctx, "points", "points2",
&redis.GeoSearchStoreQuery{GeoSearchQuery: redis.GeoSearchQuery{BoxWidth: 200,
BoxHeight: 200, BoxUnit: "km", Longitude: -77.0368707, Latitude: 38.9071923,
Sort: "DESC"}, StoreDist: false}).Val())
})
+ t.Run("GEOSEARCHSTORE with COUNT should return stored item count",
func(t *testing.T) {
+ require.NoError(t, rdb.Do(ctx, "DEL", "points",
"points2").Err())
+ require.NoError(t, rdb.GeoAdd(ctx, "points",
+ &redis.GeoLocation{Name: "Washington", Longitude:
-77.0369, Latitude: 38.9072},
+ &redis.GeoLocation{Name: "Baltimore", Longitude:
-76.6121893, Latitude: 39.2903848},
+ &redis.GeoLocation{Name: "New York", Longitude:
-74.0059413, Latitude: 40.7127837},
+ &redis.GeoLocation{Name: "Philadelphia", Longitude:
-75.16521960, Latitude: 39.95258288}).Err())
+
+ require.EqualValues(t, 1,
+ rdb.GeoSearchStore(ctx, "points", "points2",
&redis.GeoSearchStoreQuery{
+ GeoSearchQuery: redis.GeoSearchQuery{
+ BoxWidth: 200,
+ BoxHeight: 200,
+ BoxUnit: "km",
+ Longitude: -77.0368707,
+ Latitude: 38.9071923,
+ Sort: "DESC",
+ Count: 1,
+ },
+ StoreDist: false,
+ }).Val())
+ require.EqualValues(t, 1, rdb.ZCard(ctx, "points2").Val())
+ require.EqualValues(t, []string{"Baltimore"}, rdb.ZRange(ctx,
"points2", 0, -1).Val())
+ })
+
t.Run("GEOSEARCHSTORE will overwrite the dst key", func(t *testing.T) {
// dst key wrong type
require.NoError(t, rdb.Do(ctx, "del", "src", "dst").Err())
@@ -573,6 +639,25 @@ var testGeo = func(t *testing.T, configs
util.KvrocksServerConfigs) {
require.EqualValues(t, rdb.ZRange(ctx, "points", 0, -1).Val(),
rdb.ZRange(ctx, "points2", 0, -1).Val())
})
+ t.Run("GEORADIUS STORE with COUNT should return stored item count",
func(t *testing.T) {
+ require.NoError(t, rdb.Do(ctx, "DEL", "points",
"points2").Err())
+ require.NoError(t, rdb.GeoAdd(ctx, "points",
+ &redis.GeoLocation{Name: "Washington", Longitude:
-77.0369, Latitude: 38.9072},
+ &redis.GeoLocation{Name: "Baltimore", Longitude:
-76.6121893, Latitude: 39.2903848},
+ &redis.GeoLocation{Name: "New York", Longitude:
-74.0059413, Latitude: 40.7127837},
+ &redis.GeoLocation{Name: "Philadelphia", Longitude:
-75.16521960, Latitude: 39.95258288}).Err())
+
+ require.EqualValues(t, 1, rdb.GeoRadiusStore(ctx, "points",
-77.0368707, 38.9071923, &redis.GeoRadiusQuery{
+ Radius: 200,
+ Unit: "km",
+ Sort: "ASC",
+ Count: 1,
+ Store: "points2",
+ }).Val())
+ require.EqualValues(t, 1, rdb.ZCard(ctx, "points2").Val())
+ require.EqualValues(t, []string{"Washington"}, rdb.ZRange(ctx,
"points2", 0, -1).Val())
+ })
+
t.Run("GEORADIUS DESC with equal distances should not crash", func(t
*testing.T) {
require.NoError(t, rdb.Del(ctx, "geokey").Err())
require.NoError(t, rdb.GeoAdd(ctx, "geokey",
