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.

Sign up for GitHub

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

Jump to bottom

ReactiveRedisConnection ZAddCommand nx and xx does not work as expected #2731

Closed
yeunghl-shoalter opened this issue Oct 12, 2023 · 2 comments
Closed

ReactiveRedisConnection ZAddCommand nx and xx does not work as expected #2731

yeunghl-shoalter opened this issue Oct 12, 2023 · 2 comments
Assignees
@mp911de
Labels
type: bug A general bug
Milestone
2.7.17 (2021.2.17)

Comments

@yeunghl-shoalter
Copy link 

this.reactiveRedisTemplate.getConnectionFactory()
            .getReactiveConnection()
            .zSetCommands()
            .zAdd(Mono.just(
                  ReactiveZSetCommands.ZAddCommand
                        .tuple(new DefaultTuple(
                              ByteUtils.getBytes(ByteBuffer.wrap(storeSkuId.getBytes())),
                              (double)(LocalDateTime.now().toEpochSecond(ZoneOffset.UTC))))
                        .to(ByteBuffer.wrap(storeId.getBytes()))
                        .xx()));

In the Redis official document, it claims that XX only update elements that already exist. Don't add new elements. However, the above code will still insert a non-exist element.

Also, why it will use flag called upsert to represent whether it is nx or xx? The meaning of upsert does not match with the actual usage of them.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 12, 2023
@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 12, 2023
@mp911de mp911de self-assigned this Oct 12, 2023
@mp911de mp911de added this to the 2.7.17 (2021.2.17) milestone Oct 12, 2023
@mp911de
Copy link
Member

mp911de commented Oct 12, 2023

This is a bug on our side. Thanks for bringing it to our attention.

mp911de added a commit that referenced this issue Oct 12, 2023
@mp911de
Correctly apply Reactive zadd NX/XX command flags.
bb1d129 
We now correctly apply if exists/if not exists constraints on the reactive zadd command.

Previously, we only considered upsert which wasn't sufficient to apply xx/nx.

Closes #2731
mp911de added a commit that referenced this issue Oct 12, 2023
@mp911de
Correctly apply Reactive zadd NX/XX command flags.
3684484 
We now correctly apply if exists/if not exists constraints on the reactive zadd command.

Previously, we only considered upsert which wasn't sufficient to apply xx/nx.

Closes #2731
mp911de added a commit that referenced this issue Oct 12, 2023
@mp911de
Correctly apply Reactive zadd NX/XX command flags.
2bee798 
We now correctly apply if exists/if not exists constraints on the reactive zadd command.

Previously, we only considered upsert which wasn't sufficient to apply xx/nx.

Closes #2731
@mp911de
Copy link
Member

mp911de commented Oct 12, 2023

That's fixed and backported now.

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

@mp911de mp911de

Labels
type: bug A general bug
Projects
None yet
Milestone
2.7.17 (2021.2.17)
Development

No branches or pull requests
3 participants
@mp911de @spring-projects-issues @yeunghl-shoalter