Skip to content

fix: bugs in zpopmin/zpopmax;#136

Open
iwecon wants to merge 2 commits into
swift-server:mainfrom
iwecon:fix-zpop
Open

fix: bugs in zpopmin/zpopmax;#136
iwecon wants to merge 2 commits into
swift-server:mainfrom
iwecon:fix-zpop

Conversation

@iwecon

@iwecon iwecon commented Dec 8, 2025

Copy link
Copy Markdown

Details: #135

Because zadd defaults to (RESPValue, Double), scoreIsFirst is set to false by default.

zadd function:

@inlinable
public func zadd<Value: RESPValueConvertible>(
    _ elements: [(element: Value, score: Double)],
    to key: RedisKey,
    inserting insertBehavior: RedisZaddInsertBehavior = .allElements,
    returning returnBehavior: RedisZaddReturnBehavior = .insertedElementsCount
) -> EventLoopFuture<Int> {
    var args: [RESPValue] = [.init(from: key)]

    args.append(convertingContentsOf: [insertBehavior.string, returnBehavior.string].compactMap({ $0 }))
    args.add(contentsOf: elements, overestimatedCountBeingAdded: elements.count * 2) { (array, next) in
        array.append(.init(bulk: next.score.description))
        array.append(next.element.convertedToRESPValue())
    }

    return self.send(command: "ZADD", with: args)
        .tryConverting()
}

@iwecon

iwecon commented Dec 8, 2025

Copy link
Copy Markdown
Author

@fabianfett @0xTim pls review! :)

@iwecon

iwecon commented Jan 16, 2026

Copy link
Copy Markdown
Author

Has this project been forgotten?
@Joannis @fabianfett @Mordil

@Joannis

Joannis commented Jan 16, 2026

Copy link
Copy Markdown
Member

@iwecon it's been superceded by the official Valkey client: https://github.com/valkey-io/valkey-swift
Valkey is the OpenSource fork of Redis powered by the Linux Foundation, highly recommend you use thhat library going forward.

@iwecon

iwecon commented Jan 16, 2026

Copy link
Copy Markdown
Author

@iwecon it's been superceded by the official Valkey client: https://github.com/valkey-io/valkey-swift Valkey is the OpenSource fork of Redis powered by the Linux Foundation, highly recommend you use thhat library going forward.

Ah, okay. Is this repository no longer being maintained?

@0xTim

0xTim commented Jan 16, 2026

Copy link
Copy Markdown
Member

Ah, okay. Is this repository no longer being maintained?

@iwecon practically yes, however I can merge the changes and do a release for this given it's a small patch

Comment thread Sources/RediStack/Commands/SortedSetCommands.swift

@0xTim 0xTim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@0xTim

0xTim commented Feb 10, 2026

Copy link
Copy Markdown
Member

@iwecon I don't permissions to bypass the checks so you'll need to get CI passing

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