Skip to content

fix(command): avoid duplicate GEOSEARCH args - #3955

Open
mehmettokgoz wants to merge 1 commit into
redis:masterfrom
mehmettokgoz:fix/geosearch
Open

fix(command): avoid duplicate GEOSEARCH args#3955
mehmettokgoz wants to merge 1 commit into
redis:masterfrom
mehmettokgoz:fix/geosearch

Conversation

@mehmettokgoz

@mehmettokgoz mehmettokgoz commented Aug 5, 2026

Copy link
Copy Markdown

GeoSearchLocation builds the complete command argument list in geo_commands.go and then passes it to NewGeoSearchLocationCmd.

The problem was in NewGeoSearchLocationCmd in command.go. Instead of storing the complete argument list it received, the constructor called geoSearchLocationArgs again. This appended the same query options a second time:

GEOSEARCH Sicily 
    FROMLONLAT 15 37 BYRADIUS 200 km ASC WITHCOORD WITHDIST WITHHASH 
    FROMLONLAT 15 37 BYRADIUS 200 km ASC WITHCOORD WITHDIST WITHHASH

Update NewGeoSearchLocationCmd to preserve the supplied arguments so that each option is included only once.


Note

Low Risk
Small constructor fix for geo commands with a regression test on argument encoding; no auth or data-path changes.

Overview
Fixes GEOSEARCH commands built by GeoSearchLocation so Redis receives each query option once instead of twice.

NewGeoSearchLocationCmd no longer re-runs geoSearchLocationArgs on the args already assembled in geo_commands.go; it stores the passed-in slice as-is. A test now asserts cmd.Args() for a full GeoSearchLocation query (fromlonlat, byradius, sort, withcoord/withdist/withhash).

Reviewed by Cursor Bugbot for commit f5fcd35. Bugbot is set up for automated code reviews on this repo. Configure here.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f5fcd35960

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread command.go
baseCmd: baseCmd{
ctx: ctx,
args: geoSearchLocationArgs(opt, args),
args: args,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restore query expansion in the public constructor

This leaves the exported constructor no longer expanding GeoSearchLocationQuery into the GEOSEARCH arguments. Code that constructs this command directly, such as NewGeoSearchLocationCmd(ctx, q, "geosearch", key) for Client.Process, now sends only the supplied prefix and omits the FROM*/BY*/WITH* options, so Redis returns a syntax or arity error even though the same call worked before; the duplicate-args fix can be applied in the cmdable wrapper without changing the constructor's public behavior.

Useful? React with 👍 / 👎.

@ndyakov

ndyakov commented Aug 8, 2026

Copy link
Copy Markdown
Member

Hello @mehmettokgoz, thank you for the contribution. I will check this next week, I do see you think the bot is wrong, let me verify that and I will get back to you.

@vlady-kotsev

Copy link
Copy Markdown

Hey @mehmettokgoz , thanks again for the contribution. I reviewed your PR, and you're correct that the arguments are being sent twice. To preserve the expected behavior of the public constructor NewGeoSearchLocationCmd, could we remove the redundant geoSearchLocationArgs call from the GeoSearchLocation function instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants