bwlewis / rredis

R client for Redis
http://illposed.net/
93 stars 25 forks source link

redisIncrBy broken #26

Closed stefanfritsch closed 9 years ago

stefanfritsch commented 9 years ago

Hi,

using rredis 1.6.9 on R 3.1.2 I get the following with the example of redisIncrBy

> redisSet('x','5')
[1] "OK"
> redisIncrBy('x','3')
Error in doTryCatch(return(expr), name, parentenv, handler) : 
  ERR value is not an integer or out of range

The problem might be an encoding issue (?). Because that is what I get in redis:

redis 127.0.0.1:6379> get x
"X\n\x00\x00\x00\x02\x00\x03\x01\x02\x00\x02\x03\x00\x00\x00\x00\x10\x00\x00\x00
\x01\x00\x04\x00\t\x00\x00\x00\x015"
redis 127.0.0.1:6379> set x "5"
OK
redis 127.0.0.1:6379> get x
"5"
redis 127.0.0.1:6379> incrby x 1
(integer) 6

Ciao, Stefan

bwlewis commented 9 years ago

Hi Stefan

The default behavior of redisSet stores values as serialized R values. In order to put unserialized strings into Redis from R, use:

redisSet('x',charToRaw('5'))

then things work for you (this is documented behavior).

Best,

Bryan

stefanfritsch commented 9 years ago

Thanks, I'll do that. :)

But shouldn't you change the example for redisIncrBy in that case?

bwlewis commented 9 years ago

I'm sorry, I see! The examples are broken. Sorry about that. Will fix those examples... I'm reopening this to remind me.

bwlewis commented 9 years ago

OK, wow this turned out to lead to some interesting new issues. I found that redis ":" messages were automatically converted to numeric values, which is problematic for a few reasons.

The upshot is that I ended up having to change the API somewhat and therefore made a major increase in version number to 1.7.0. See the details in the README page on github:

https://github.com/bwlewis/rredis

You can try things out by installing the new package directly from github with: devtools::install_github("bwlewis/rredis", quick=TRUE)

(requires the 'devtools' package and the ability to compile source packages).

I will review this over the next week or so to make sure it seems OK and I don't find any other problems, and then I'll submit the new version to CRAN.

Thanks for alerting me to these problems...

stefanfritsch commented 9 years ago

Thanks a lot, I'll try to try it tomorrow. :) :+1: