Skip to content
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.

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

Go: Add Zcount command #2930

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Conversation

prateek-kumar-improving
Copy link
Collaborator

@prateek-kumar-improving prateek-kumar-improving commented Jan 8, 2025

Issue link

This Pull Request is linked to issue (URL): #2833

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Create merge commit if merging release branch into main, squash otherwise.

Signed-off-by: Prateek Kumar <[email protected]>
Signed-off-by: Prateek Kumar <[email protected]>
Signed-off-by: Prateek Kumar <[email protected]>
Copy link
Collaborator

@yipin-chen yipin-chen left a comment

Choose a reason for hiding this comment

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

LGTM.

Signed-off-by: Prateek Kumar <[email protected]>
go/api/options/zcount_options.go Outdated Show resolved Hide resolved
go/api/options/zcount_options.go Outdated Show resolved Hide resolved
go/api/options/zcount_options.go Outdated Show resolved Hide resolved
go/api/sorted_set_commands.go Outdated Show resolved Hide resolved
Signed-off-by: Prateek Kumar <[email protected]>
Signed-off-by: Prateek Kumar <[email protected]>
Signed-off-by: Prateek Kumar <[email protected]>
Signed-off-by: prateek-kumar-improving <[email protected]>
// specific score and inclusivity.
//
// Return value:
// Result[int64] - The number of members in the specified score range.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Result[int64] - The number of members in the specified score range.
// The number of members in the specified score range.

// fmt.Println(zCountResult.Value()) // Output: 3
//
// [valkey.io]: https://valkey.io/commands/zcount/
ZCount(key string, rangeOptions *options.ZCountRange) (Result[int64], error)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ZCount(key string, rangeOptions *options.ZCountRange) (Result[int64], error)
ZCount(key string, rangeOptions *options.ZCountRange) (int64, error)

// if err!= nil {
// // Print err
// }
// fmt.Println(zCountResult.Value()) // Output: 3
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// fmt.Println(zCountResult.Value()) // Output: 3
// fmt.Println(zCountResult) // Output: 3

@@ -376,4 +376,34 @@ type SortedSetCommands interface {
//
// [valkey.io]: https://valkey.io/commands/zrevrank/
ZRevRankWithScore(key string, member string) (Result[int64], Result[float64], error)

// Returns the number of members in the sorted set stored at `key` with scores between `min` and `max` score.
Copy link
Collaborator

Choose a reason for hiding this comment

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

As discussed, move the entire doc to the implementation

return CreateNilInt64Result(), err
}
result, err := client.executeCommand(C.ZCount, append([]string{key}, zCountRangeArgs...))
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

you missed handleLongResponse

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

might have deleted a line when resolving the merge conflict.

@@ -1579,6 +1579,14 @@ func (client *baseClient) Persist(key string) (Result[bool], error) {
return handleBooleanResponse(result)
}

func (client *baseClient) ZCount(key string, rangeOptions *options.ZCountRange) (Result[int64], error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func (client *baseClient) ZCount(key string, rangeOptions *options.ZCountRange) (Result[int64], error) {
func (client *baseClient) ZCount(key string, rangeOptions *options.ZCountRange) (int64, error) {

return value is not nullable

Copy link
Collaborator

Choose a reason for hiding this comment

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

Update tests accordingly

Comment on lines +12 to +68
type (
InfBoundary string
)

const (
// The highest bound in the sorted set
PositiveInfinity InfBoundary = "+inf"
// The lowest bound in the sorted set
NegativeInfinity InfBoundary = "-inf"
)

// This struct represents the infinity boundary for a score range.
type InfScoreBound struct {
value InfBoundary
}

// Create a new infinite score boundary
func NewInfScoreBoundBuilder(value InfBoundary) *InfScoreBound {
return &InfScoreBound{value: value}
}

func (infScoreBound *InfScoreBound) ToArgs() ([]string, error) {
return []string{string(infScoreBound.value)}, nil
}

// This struct represents score boundary for a bound.
type ScoreBoundary struct {
bound float64
isInclusive bool
}

// Create a new score boundary.
func NewScoreBoundaryBuilder() *ScoreBoundary {
return &ScoreBoundary{isInclusive: true}
}

// Set the bound for a score boundary.
func (scoreBoundary *ScoreBoundary) SetBound(bound float64) *ScoreBoundary {
scoreBoundary.bound = bound
return scoreBoundary
}

// Set if the bound for a score boundary is inclusive or not inclusive in the boundary.
func (scoreBoundary *ScoreBoundary) SetIsInclusive(isInclusive bool) *ScoreBoundary {
scoreBoundary.isInclusive = isInclusive
return scoreBoundary
}

func (scoreBoundary *ScoreBoundary) ToArgs() ([]string, error) {
args := []string{}
if !scoreBoundary.isInclusive {
args = append(args, "("+utils.FloatToString(scoreBoundary.bound))
} else {
args = append(args, utils.FloatToString(scoreBoundary.bound))
}
return args, nil
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

all this already implemented in another file

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.

4 participants