[kvrocks] branch unstable updated: Fix GEOHASH / GEOPOS should return nil array instead of error for non-existing key (#1573)

[hulk]

This is an automated email from the ASF dual-hosted git repository.

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 03ceb6e8 Fix GEOHASH / GEOPOS should return nil array instead of error 
for non-existing key (#1573)
03ceb6e8 is described below

commit 03ceb6e8105656c2fc08ae47a820e3e37d3c56f3
Author: Binbin <binloveplay1...@qq.com>
AuthorDate: Wed Jul 12 12:12:50 2023 +0800

    Fix GEOHASH / GEOPOS should return nil array instead of error for 
non-existing key (#1573)
    
    The current code GETHASH returns an error for a key
    that doesn't exist:
    ```
    127.0.0.1:6666> geohash mykey a b c
    (error) ERR NotFound:
    ```
    
    In Redis, this would return:
    ```
    127.0.0.1:6379> geohash mykey a b c
    1) (nil)
    2) (nil)
    3) (nil)
    ```
    
    GEOPOS also has the same issue, this PR fixes these inconsistencies in this 
case.
---
 src/commands/cmd_geo.cc           | 15 +++++++++++++--
 tests/gocase/unit/geo/geo_test.go | 10 ++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/commands/cmd_geo.cc b/src/commands/cmd_geo.cc
index 49cc99ad..62dab23c 100644
--- a/src/commands/cmd_geo.cc
+++ b/src/commands/cmd_geo.cc
@@ -163,10 +163,14 @@ class CommandGeoHash : public Commander {
     std::vector<std::string> hashes;
     redis::Geo geo_db(svr->storage, conn->GetNamespace());
     auto s = geo_db.Hash(args_[1], members_, &hashes);
-    if (!s.ok()) {
+    if (!s.ok() && !s.IsNotFound()) {
       return {Status::RedisExecErr, s.ToString()};
     }
 
+    if (s.IsNotFound()) {
+      hashes.resize(members_.size(), "");
+    }
+
     *output = redis::MultiBulkString(hashes);
     return Status::OK();
   }
@@ -188,11 +192,18 @@ class CommandGeoPos : public Commander {
     std::map<std::string, GeoPoint> geo_points;
     redis::Geo geo_db(svr->storage, conn->GetNamespace());
     auto s = geo_db.Pos(args_[1], members_, &geo_points);
-    if (!s.ok()) {
+    if (!s.ok() && !s.IsNotFound()) {
       return {Status::RedisExecErr, s.ToString()};
     }
 
     std::vector<std::string> list;
+
+    if (s.IsNotFound()) {
+      list.resize(members_.size(), "");
+      *output = redis::MultiBulkString(list);
+      return Status::OK();
+    }
+
     for (const auto &member : members_) {
       auto iter = geo_points.find(member.ToString());
       if (iter == geo_points.end()) {
diff --git a/tests/gocase/unit/geo/geo_test.go 
b/tests/gocase/unit/geo/geo_test.go
index 173ae67c..c34dee86 100644
--- a/tests/gocase/unit/geo/geo_test.go
+++ b/tests/gocase/unit/geo/geo_test.go
@@ -134,12 +134,22 @@ func TestGeo(t *testing.T) {
                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("GEOHASH against non existing key", func(t *testing.T) {
+               require.NoError(t, rdb.Del(ctx, "points").Err())
+               require.EqualValues(t, []interface{}{nil, nil, nil}, 
rdb.Do(ctx, "GEOHASH", "points", "a", "b", "c").Val())
+       })
+
        t.Run("GEOHASH is able to return geohash strings", func(t *testing.T) {
                require.NoError(t, rdb.Del(ctx, "points").Err())
                require.NoError(t, rdb.GeoAdd(ctx, "points", 
&redis.GeoLocation{Name: "test", Longitude: -5.6, Latitude: 42.6}).Err())
                require.EqualValues(t, []string([]string{"ezs42e44yx0"}), 
rdb.GeoHash(ctx, "points", "test").Val())
        })
 
+       t.Run("GEOPOS against non existing key", func(t *testing.T) {
+               require.NoError(t, rdb.Del(ctx, "points").Err())
+               require.EqualValues(t, []interface{}{nil, nil, nil}, 
rdb.Do(ctx, "GEOPOS", "points", "a", "b", "c").Val())
+       })
+
        t.Run("GEOPOS simple", func(t *testing.T) {
                require.NoError(t, rdb.Del(ctx, "points").Err())
                require.NoError(t, rdb.GeoAdd(ctx, "points", 
&redis.GeoLocation{Name: "a", Longitude: 10, Latitude: 20}, 
&redis.GeoLocation{Name: "b", Longitude: 30, Latitude: 40}).Err())