Skip to content

fix(geo): make geo store commands return the resulting set size with COUNT#3457

Merged
git-hulk merged 6 commits into
apache:unstablefrom
jihuayu:codex/fix-geo-store-count-return
Apr 21, 2026
Merged

fix(geo): make geo store commands return the resulting set size with COUNT#3457
git-hulk merged 6 commits into
apache:unstablefrom
jihuayu:codex/fix-geo-store-count-return

Conversation

@jihuayu

@jihuayu jihuayu commented Apr 17, 2026

Copy link
Copy Markdown
Member

Fix the return value of geo store commands when COUNT is specified.

Previously, these commands stored only the limited number of members in the destination zset, but returned the full matched result count instead of the actual stored count.

Assisted-by: Codex

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the integer reply of GEO store commands when COUNT is specified so the reply matches the number of members actually stored in the destination zset (rather than the total matched results).

Changes:

  • Adjust GEORADIUS{,BYMEMBER} ... STORE and GEOSEARCHSTORE to return min(matches, COUNT) when COUNT is provided.
  • Refactor result truncation logic into a shared helper (GetReturnedItemsCount) and update output loops to use size_t.
  • Add Go unit tests covering COUNT behavior for GEORADIUS STORE, GEORADIUSBYMEMBER STORE, and GEOSEARCHSTORE.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/commands/cmd_geo.cc Updates GEO store command integer replies to reflect stored count when COUNT is set; centralizes truncation logic.
tests/gocase/unit/geo/geo_test.go Adds regression tests verifying store commands return the stored item count with COUNT.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/commands/cmd_geo.cc
@git-hulk

Copy link
Copy Markdown
Member

@jihuayu Good catch! Can we add a test for COUNT > matches to confirm it returns the smaller value?

@jihuayu jihuayu force-pushed the codex/fix-geo-store-count-return branch from 81e2cc9 to 9b7b16a Compare April 21, 2026 12:08
@git-hulk git-hulk enabled auto-merge (squash) April 21, 2026 13:42
@git-hulk git-hulk merged commit 7c49728 into apache:unstable Apr 21, 2026
71 of 73 checks passed
@sonarqubecloud

Copy link
Copy Markdown

nagisa-kunhah pushed a commit to nagisa-kunhah/kvrocks that referenced this pull request Apr 29, 2026
…COUNT (apache#3457)

Fix the return value of geo store commands when `COUNT` is specified.

Previously, these commands stored only the limited number of members in
the destination zset, but returned the full matched result count instead
of the actual stored count.

Assisted-by: Codex
nagisa-kunhah pushed a commit to nagisa-kunhah/kvrocks that referenced this pull request Apr 29, 2026
…COUNT (apache#3457)

Fix the return value of geo store commands when `COUNT` is specified.

Previously, these commands stored only the limited number of members in
the destination zset, but returned the full matched result count instead
of the actual stored count.

Assisted-by: Codex
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