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

Support Go 1.23 iterators for the ZADD command #652

Merged
merged 3 commits into from
Oct 25, 2024

Conversation

mingdaoy
Copy link
Contributor

@mingdaoy mingdaoy commented Oct 25, 2024

@mingdaoy mingdaoy changed the title Issues/644 Support Go 1.23 iterators for the ZADD command Support Go 1.23 iterators for the ZADD command Oct 25, 2024
@@ -24,3 +27,10 @@ func (c XaddFieldValue) FieldValueIter(seq iter.Seq2[string, string]) XaddFieldV
}
return c
}

func (c ZaddScoreMember) ScoreMemberIter(seq iter.Seq2[float64, string]) ZaddScoreMember {
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 (c ZaddScoreMember) ScoreMemberIter(seq iter.Seq2[float64, string]) ZaddScoreMember {
func (c ZaddScoreMember) ScoreMemberIter(seq iter.Seq2[string, float64]) ZaddScoreMember {

Using [string, float64] will be much more convenient for users because the first value usually refers to a unique key.

And Redis sorted set allows elements with the same score, so we better put float64 at the second value.


func (c ZaddScoreMember) ScoreMemberIter(seq iter.Seq2[float64, string]) ZaddScoreMember {
for score, member := range seq {
c.cs.s = append(c.cs.s, fmt.Sprintf("%f", score), member)
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
c.cs.s = append(c.cs.s, fmt.Sprintf("%f", score), member)
c.cs.s = append(c.cs.s, strconv.FormatFloat(score, 'f', -1, 64), member)

strconv.FormatFloat is much more performant than fmt.Sprintf

@mingdaoy
Copy link
Contributor Author

Just updated the PR.
TestIter result:
image

@rueian rueian merged commit 25821d0 into redis:main Oct 25, 2024
27 checks passed
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.

2 participants