From 2248590a163411d8dfdd9731d569d67c30692c19 Mon Sep 17 00:00:00 2001 From: Binbin Date: Fri, 4 Aug 2023 14:29:06 +0800 Subject: [PATCH] GEOSEARCH* from is not optional, FROMLONLAT or FROMMEMBER must require Fix that FROMLONLAT or FROMMEMBER is required, otherwise we would have the following inconsistency: ``` 127.0.0.1:6379> GEOSEARCH src BYBOX 88 88 m asc (error) ERR exactly one of FROMMEMBER or FROMLONLAT can be specified for GEOSEARCH 127.0.0.1:6379> GEOSEARCHSTORE dst src BYBOX 88 88 m asc (error) ERR exactly one of FROMMEMBER or FROMLONLAT can be specified for GEOSEARCHSTORE 127.0.0.1:6666> GEOSEARCH src BYBOX 88 88 m asc (empty array) 127.0.0.1:6666> GEOSEARCHSTORE dst src BYBOX 88 88 m asc (integer) 0 ``` Minor fix to minor bug. --- src/commands/cmd_geo.cc | 8 ++++++++ tests/gocase/unit/geo/geo_test.go | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/commands/cmd_geo.cc b/src/commands/cmd_geo.cc index 6795e8d4dab..5f3cfa67016 100644 --- a/src/commands/cmd_geo.cc +++ b/src/commands/cmd_geo.cc @@ -415,6 +415,10 @@ class CommandGeoSearch : public CommandGeoBase { } } + if (origin_point_type_ == kNone) { + return {Status::RedisParseErr, "exactly one of FROMMEMBER or FROMLONLAT can be specified for GEOSEARCH"}; + } + if (member_ != "" && longitude_ != 0 && latitude_ != 0) { return {Status::RedisParseErr, "please use only one of FROMMEMBER or FROMLONLAT"}; } @@ -574,6 +578,10 @@ class CommandGeoSearchStore : public CommandGeoSearch { } } + if (origin_point_type_ == kNone) { + return {Status::RedisParseErr, "exactly one of FROMMEMBER or FROMLONLAT can be specified for GEOSEARCHSTORE"}; + } + if (member_ != "" && longitude_ != 0 && latitude_ != 0) { return {Status::RedisParseErr, "please use only one of FROMMEMBER or FROMLONLAT"}; } diff --git a/tests/gocase/unit/geo/geo_test.go b/tests/gocase/unit/geo/geo_test.go index 12501d00b82..527c6b6a83a 100644 --- a/tests/gocase/unit/geo/geo_test.go +++ b/tests/gocase/unit/geo/geo_test.go @@ -134,6 +134,12 @@ 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: 7, Unit: "km"}).Val()) }) + t.Run("GEOHASH errors", func(t *testing.T) { + require.NoError(t, rdb.Del(ctx, "points").Err()) + + util.ErrorRegexp(t, rdb.Do(ctx, "geosearch", "points", "BYBOX", 88, 88, "m", "asc").Err(), ".*exactly one of FROMMEMBER or FROMLONLAT can be specified for GEOSEARCH*.") + }) + 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()) @@ -196,6 +202,13 @@ func TestGeo(t *testing.T) { rdb.GeoSearch(ctx, "points", &redis.GeoSearchQuery{BoxWidth: 200, BoxHeight: 200, BoxUnit: "km", Member: "Washington", Sort: "DESC"}).Val()) }) + t.Run("GEOSEARCHSTORE errors", func(t *testing.T) { + require.NoError(t, rdb.Del(ctx, "points").Err()) + require.NoError(t, rdb.Del(ctx, "points2").Err()) + + util.ErrorRegexp(t, rdb.Do(ctx, "geosearchstore", "points2", "points", "BYBOX", 88, 88, "m", "asc").Err(), ".*exactly one of FROMMEMBER or FROMLONLAT can be specified for GEOSEARCHSTORE*.") + }) + t.Run("GEOSEARCHSTORE against non existing src key", func(t *testing.T) { require.NoError(t, rdb.Del(ctx, "points").Err()) require.EqualValues(t, 0, rdb.Do(ctx, "GEOSEARCHSTORE", "dst", "src", "FROMMEMBER", "Shenzhen", "BYBOX", 88, 88, "m").Val())