microsoft
A set STORE command doesn't properly overwrite an existing key
Describe the bug
SINTERSTORE doesn't overwrite an existing string. SUNIONSTORE and SDIFFSTORE are also affected.
Steps to reproduce the bug
Please see the following Python snippet. Commands executed in the CLI have the same results. The "expected" values come from Redis 7.4.1.
import redis
r = redis.Redis()
r.set('foo', 'bar')
r.sadd('s1', *range(10))
r.sadd('s2', *range(5, 15))
r.sinterstore('foo', ['s1', 's2'])
print(r.keys('*')) # [b'foo', b's1', b's2', b'foo']; expected: [b's2', b's1', b'foo']
print(r.type('foo')) # 'string'; expected: 'set'
Expected behavior
No response
Screenshots
No response
Release version
v1.0.35
IDE
No response
OS version
Windows 11
Additional context
No response
It's not about SINTERSTORE as such, see https://github.com/microsoft/garnet/issues/358#issuecomment-2093671190
Garnet uses two stores internally: a main store to hold raw strings and an object store to hold objects. Thus it's not unexpected to allow the same key in both stores. Changing this will require us to check both stores every time, which is too expensive. We are working on unifying the two indexes but that's not ready yet.
Some fun with this in PowerShell using FarNet.Redis module with Garnet server
$db = Open-Redis
# set same key as String and Set
Set-RedisString test:dupe-key v1
Set-RedisSet test:dupe-key v2, v3
# 2 same keys exist in 2 stores
Search-RedisKey test:dupe-key
<# 2 keys
test:dupe-key
test:dupe-key
#>
# one is String
Get-RedisString test:dupe-key
<# String value
v1
#>
# another is Set
Get-RedisSet test:dupe-key
<# Set values
v2
v3
#>
How important is it that the same key be prevented by the system from being used for the main and object store? Seems users can easily work around this. cc @TedHartMS
Stating the obvious perhaps, for a key-value system uniqueness of keys is very important. Users may work around the current feature (not sure about "easily", depends). But users will be tripped over this anyway. Workarounds will come with some price. If the server does not want to pay it, then clients have to pay, arguably higher overall price.
NB If the actual plan is to move to the single index and hence fix this issue then importance is probably low. It would be useful to mention the current feature somewhere in the docs, to minimize surprises.
