Skip to content

feat: Implement GEORADIUS_RO command #5678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 25, 2025
Merged

feat: Implement GEORADIUS_RO command #5678

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e33c40
feat: Implement GEORADIUS_RO command
EricHayter Aug 16, 2025
601c719
Implement test cases for GEORADIUS_RO
EricHayter Aug 16, 2025
bb51978
Update logic when parsing arguments
EricHayter Aug 19, 2025
1feb731
Add happy path test case for GEORADIUS_RO
EricHayter Aug 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 33 additions & 16 deletions src/server/geo_family.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ void GeoFamily::GeoRadiusByMember(CmdArgList args, const CommandContext& cmd_cnt
GeoSearchStoreGeneric(cmd_cntx.tx, builder, shape, key, member, geo_ops);
}

void GeoFamily::GeoRadius(CmdArgList args, const CommandContext& cmd_cntx) {
void GeoFamily::GeoRadiusGeneric(CmdArgList args, const CommandContext& cmd_cntx, bool read_only) {
GeoShape shape = {};
GeoSearchOpts geo_ops;

Expand All @@ -787,12 +787,21 @@ void GeoFamily::GeoRadius(CmdArgList args, const CommandContext& cmd_cntx) {
shape.type = CIRCULAR_TYPE;

while (parser.HasNext()) {
// try and parse for only RO options first
auto type =
parser.MapNext("STORE", Type::STORE, "STOREDIST", Type::STOREDIST, "ASC", Type::ASC, "DESC",
Type::DESC, "COUNT", Type::COUNT, "WITHCOORD", Type::WITHCOORD, "WITHDIST",
Type::WITHDIST, "WITHHASH", Type::WITHHASH);
parser.TryMapNext("ASC", Type::ASC, "DESC", Type::DESC, "COUNT", Type::COUNT, "WITHCOORD",
Type::WITHCOORD, "WITHDIST", Type::WITHDIST, "WITHHASH", Type::WITHHASH);
// if writing variant and there there was a mapping failure test for write variant arguments
if (!type && !read_only) {
type = parser.MapNext("STORE", Type::STORE, "STOREDIST", Type::STOREDIST);
}

switch (type) {
// could not map the argument to an argument for RO or write GEORADIUS
if (!type) {
return builder->SendError("syntax error", kSyntaxErrType);
}

switch (*type) {
case Type::STORE:
geo_ops.store_key = parser.Next();
geo_ops.store = geo_ops.store == GeoStoreType::kNoStore ? GeoStoreType::kStoreHash
Expand Down Expand Up @@ -857,6 +866,14 @@ void GeoFamily::GeoRadius(CmdArgList args, const CommandContext& cmd_cntx) {
GeoSearchStoreGeneric(cmd_cntx.tx, builder, shape, key, "", geo_ops);
}

void GeoFamily::GeoRadius(CmdArgList args, const CommandContext& cmd_cntx) {
GeoRadiusGeneric(args, cmd_cntx, false);
}

void GeoFamily::GeoRadiusRO(CmdArgList args, const CommandContext& cmd_cntx) {
GeoRadiusGeneric(args, cmd_cntx, true);
}

#define HFUNC(x) SetHandler(&GeoFamily::x)

namespace acl {
Expand All @@ -867,21 +884,21 @@ constexpr uint32_t kGeoDist = READ | GEO | SLOW;
constexpr uint32_t kGeoSearch = READ | GEO | SLOW;
constexpr uint32_t kGeoRadiusByMember = WRITE | GEO | SLOW;
constexpr uint32_t kGeoRadius = WRITE | GEO | SLOW;
constexpr uint32_t kGeoRadiusRO = READ | GEO | SLOW;
} // namespace acl

void GeoFamily::Register(CommandRegistry* registry) {
registry->StartFamily();
*registry << CI{"GEOADD", CO::FAST | CO::WRITE | CO::DENYOOM, -5, 1, 1, acl::kGeoAdd}.HFUNC(
GeoAdd)
<< CI{"GEOHASH", CO::FAST | CO::READONLY, -2, 1, 1, acl::kGeoHash}.HFUNC(GeoHash)
<< CI{"GEOPOS", CO::FAST | CO::READONLY, -2, 1, 1, acl::kGeoPos}.HFUNC(GeoPos)
<< CI{"GEODIST", CO::READONLY, -4, 1, 1, acl::kGeoDist}.HFUNC(GeoDist)
<< CI{"GEOSEARCH", CO::READONLY, -7, 1, 1, acl::kGeoSearch}.HFUNC(GeoSearch)
<< CI{"GEORADIUSBYMEMBER", CO::WRITE | CO::STORE_LAST_KEY, -5, 1, 1,
acl::kGeoRadiusByMember}
.HFUNC(GeoRadiusByMember)
<< CI{"GEORADIUS", CO::WRITE | CO::STORE_LAST_KEY, -6, 1, 1, acl::kGeoRadius}.HFUNC(
GeoRadius);
*registry
<< CI{"GEOADD", CO::FAST | CO::WRITE | CO::DENYOOM, -5, 1, 1, acl::kGeoAdd}.HFUNC(GeoAdd)
<< CI{"GEOHASH", CO::FAST | CO::READONLY, -2, 1, 1, acl::kGeoHash}.HFUNC(GeoHash)
<< CI{"GEOPOS", CO::FAST | CO::READONLY, -2, 1, 1, acl::kGeoPos}.HFUNC(GeoPos)
<< CI{"GEODIST", CO::READONLY, -4, 1, 1, acl::kGeoDist}.HFUNC(GeoDist)
<< CI{"GEOSEARCH", CO::READONLY, -7, 1, 1, acl::kGeoSearch}.HFUNC(GeoSearch)
<< CI{"GEORADIUSBYMEMBER", CO::WRITE | CO::STORE_LAST_KEY, -5, 1, 1, acl::kGeoRadiusByMember}
.HFUNC(GeoRadiusByMember)
<< CI{"GEORADIUS", CO::WRITE | CO::STORE_LAST_KEY, -6, 1, 1, acl::kGeoRadius}.HFUNC(GeoRadius)
<< CI{"GEORADIUS_RO", CO::READONLY, -6, 1, 1, acl::kGeoRadiusRO}.HFUNC(GeoRadiusRO);
}

} // namespace dfly
2 changes: 2 additions & 0 deletions src/server/geo_family.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ class GeoFamily {
static void GeoDist(CmdArgList args, const CommandContext& cmd_cntx);
static void GeoSearch(CmdArgList args, const CommandContext& cmd_cntx);
static void GeoRadiusByMember(CmdArgList args, const CommandContext& cmd_cntx);
static void GeoRadiusGeneric(CmdArgList args, const CommandContext& cmd_cntx, bool read_only);
static void GeoRadius(CmdArgList args, const CommandContext& cmd_cntx);
static void GeoRadiusRO(CmdArgList args, const CommandContext& cmd_cntx);
};

} // namespace dfly
27 changes: 27 additions & 0 deletions src/server/geo_family_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,33 @@ TEST_F(GeoFamilyTest, GeoRadius) {
EXPECT_THAT(resp, ErrArg("syntax error"));
}

TEST_F(GeoFamilyTest, GeoRadiusRO) {
EXPECT_EQ(10, CheckedInt({"geoadd", "Europe", "13.4050", "52.5200", "Berlin", "3.7038",
"40.4168", "Madrid", "9.1427", "38.7369", "Lisbon", "2.3522",
"48.8566", "Paris", "16.3738", "48.2082", "Vienna", "4.8952",
"52.3702", "Amsterdam", "10.7522", "59.9139", "Oslo", "23.7275",
"37.9838", "Athens", "19.0402", "47.4979", "Budapest", "6.2603",
"53.3498", "Dublin"}));

// GEORADIUS_RO should not accept arguments for storing (writing data)
auto resp =
Run({"GEORADIUS_RO", "Europe", "13.4050", "52.5200", "900", "KM", "STORE_DIST", "store_key"});
EXPECT_THAT(resp, ErrArg("syntax error"));

resp = Run({"GEORADIUS_RO", "Europe", "13.4050", "52.5200", "900", "KM", "STORE", "store_key"});
EXPECT_THAT(resp, ErrArg("syntax error"));

resp = Run({"GEORADIUS_RO", "Europe", "13.4050", "52.5200", "500", "KM", "COUNT", "3",
"WITHCOORD", "WITHDIST"});
EXPECT_THAT(
resp,
RespArray(ElementsAre(
RespArray(ElementsAre("Berlin", DoubleArg(0.00017343178521311378),
RespArray(ElementsAre(DoubleArg(13.4050), DoubleArg(52.5200))))),
RespArray(ElementsAre("Dublin", DoubleArg(487.5619030644293),
RespArray(ElementsAre(DoubleArg(6.2603), DoubleArg(53.3498))))))));
}

TEST_F(GeoFamilyTest, GeoRadiusByMemberUb) {
Run({"GEOADD", "geo", "-118.2437", "34.0522", "972"});
Run({"GEOADD", "geo", "-73.935242", "40.730610", "973"});
Expand Down
Loading