feat: add unit field to bitcount #538
Conversation
|
@rueian |
Yes, we should support the Unit there too. |
rueidiscompat/adapter.go
Outdated
| resp = c.client.Do(ctx, c.client.B().Bitcount().Key(key).Build()) | ||
| } else { | ||
| resp = c.client.Do(ctx, c.client.B().Bitcount().Key(key).Start(bitCount.Start).End(bitCount.End).Build()) | ||
| resp := c.client.Do(ctx, c.client.B().Bitcount().Key(key).Bit().Build()) // Default to BIT |
There was a problem hiding this comment.
Hi @SoulPancake,
We can't use .Bit() as default because it is only supported on Redis >= 7. We should keep the original behavior.
rueidiscompat/adapter.go
Outdated
| switch bitCount.Unit { | ||
| case BitCountIndexByte: | ||
| resp = c.client.Do(ctx, builder.Byte().Build()) // Handling for BYTE | ||
| case BitCountIndexBit, "": // Default to BIT if not specified |
There was a problem hiding this comment.
Same here. we can't use .Bit() as default in the case of "".
|
Hi @SoulPancake, would you like to continue on this? |
|
yes give me some time I will address it tonight @rueian |
|
@rueian I was thinking whether I should continue here or on valkey-go |
|
Hi @SoulPancake, Here is ok. I will sync commits between them. |
|
Will that make me a contributor there too? As I would like to be so @rueian |
Yes of course |
SoulPancake
force-pushed
the
ab/feature/add-unit-field-bitcount
branch
from
6c7e253
to
7b1c162
Compare
|
I have some doubts But still I am getting an |
|
Further, In the container that I am using, I am able to get the response too |
…om/SoulPancake/rueidis into ab/feature/add-unit-field-bitcount
rueidiscompat/command.go
Outdated
| @@ -1959,6 +1959,7 @@ type SetArgs struct { | |||
|
|
|||
| type BitCount struct { | |||
| Start, End int64 | |||
| Unit *string // Stores BIT or BYTE | |||
There was a problem hiding this comment.
Hi @SoulPancake,
Could we keep using string instead of *string? We'd like to make the interface as close to go-redis'.
Do you mean here? The RESP2 test suite is tested against Redis 5. You need to wrap your new tests in a if block: if resp3 {
bitCount = adapter.BitCount(ctx, "key", &BitCount{
Start: 1,
End: 1,
Unit: "BYTE",
})
Expect(bitCount.Err()).NotTo(HaveOccurred())
Expect(bitCount.Val()).To(Equal(int64(6)))
} |
|
Please review @rueian :) |
|
LGTM. Thanks @SoulPancake! |
fixes