vjzning / redis

Automatically exported from code.google.com/p/redis
https://code.google.com/p/redis/
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

SINTERSTORE does not return a status reply #121

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

Here are the raw commands and responses I used with a Redis server:

FLUSHDB
+OK
SINTERSTORE c a b
+OK
SADD a 3
foo
:1
SINTERSTORE c a b
+OK
SADD b 3
bar
:1
SINTERSTORE c a b
:0
SADD b 3
foo
:1
SINTERSTORE c a b
:1
SADD a 3
bar
:1
SINTERSTORE c a b
:1

What is the expected output? What do you see instead?

From reading the docs, I expect a status reply, but, depending on the 
situation, I get an integer reply representing the amount of 
common keys.

What version of the product are you using? On what operating system?

Redis version 1.000, on Mac OSX 10.5 on an Intel x86

Please provide any additional information below.

Original issue reported on code.google.com by r.geoghe...@gmail.com on 10 Dec 2009 at 11:58

GoogleCodeExporter commented 9 years ago
Same behaviour with Redis freshly cloned from github. Here is the info output:

redis_version:1.1.91
arch_bits:64
multiplexing_api:kqueue
uptime_in_seconds:42
uptime_in_days:0
connected_clients:1
connected_slaves:0
used_memory:868528
changes_since_last_save:1
bgsave_in_progress:0
last_save_time:1260814764
total_connections_received:545
total_commands_processed:549
role:master

Are there any update/planned-patch for that inconsistency?

Original comment by jcout...@gmail.com on 14 Dec 2009 at 6:20

GoogleCodeExporter commented 9 years ago
Hello, this is a feature, what should be patched is the documentation I guess, 
or I'm 
missing something? :)

Original comment by anti...@gmail.com on 14 Dec 2009 at 6:58

GoogleCodeExporter commented 9 years ago
What I meant was this command should return either a status code reply or an 
integer 
code reply, because as it is right now, it sort of depends...

If we refer back to the original bug report by r.geoghegan, it clearly shows 2 
type of 
reply returned by this command. Or maybe I am missing something too? :)

Original comment by jcout...@gmail.com on 14 Dec 2009 at 7:10

GoogleCodeExporter commented 9 years ago
I had a look at the code, and the issue is a bit more complicated. The current 
behavior is that if any of the keys 
given to intersect does not exist, deletes the destination key and returns +OK. 
This is very logical in a sort of 
SQL sense where Null + Not Null = Null, so None intersection Some Set = None.

I propose that SINTERSTORE, if given a non-existing key, deletes the 
destination key and returns :0, which 
should be in line with most replies from SINTERSTORE. I have attached a patch 
to that effect.

Original comment by r.geoghe...@gmail.com on 15 Dec 2009 at 4:41

Attachments:

GoogleCodeExporter commented 9 years ago
You are right, fixed: I just swapped the shared.ok with shared.czero that 
should be like 
what r.geoghegan proposed, this error is the result of a behavior change of 
SINTERSTORE at some time, it used to return just +OK, but now it returns the 
size of the 
intersection... but I clearly forgot to change this line of code. Now I'm 
adding a 
regression test for this.

Thanks!

Original comment by anti...@gmail.com on 15 Dec 2009 at 9:07

GoogleCodeExporter commented 9 years ago
Great! Thank you very much, both of you!

Original comment by jcout...@gmail.com on 15 Dec 2009 at 11:08