Skip to content

Commit

Permalink
GEOSEARCH* from is not optional, FROMLONLAT or FROMMEMBER must require (
Browse files Browse the repository at this point in the history
#1656)

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.
  • Loading branch information
enjoy-binbin authored Aug 9, 2023
1 parent c85daac commit 4fe7fb0
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/commands/cmd_geo.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"};
}
Expand Down Expand Up @@ -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"};
}
Expand Down
13 changes: 13 additions & 0 deletions tests/gocase/unit/geo/geo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down

0 comments on commit 4fe7fb0

Please sign in to comment.