aconchillo / guile-redis

Redis module for Guile
GNU General Public License v3.0
29 stars 4 forks source link

LRANGE doesn't seem to work #8

Closed rekado closed 2 years ago

rekado commented 2 years ago

I'm trying to use LRANGE to fetch elements from a list, e.g. with (redis-send conn (lrange (list "whatever" 0 10))). This results in an error:

In ice-9/boot-9.scm:
   222:17  4 (map1 (#<<redis-command> name: "LRANGE" args: ("log:u1…>))
In redis/utils.scm:
    65:18  3 (read-reply _)
    48:34  2 (build-list _ #<procedure 7f340165d3a0 at redis/utils.s…>)
    70:12  1 (read-reply #<<redis-connection> host: "127.0.0.1" port…>)
In ice-9/boot-9.scm:
  1685:16  0 (raise-exception _ #:continuable? _)

This appears to be right where the second invocation of READ-REPLY happens at https://github.com/aconchillo/guile-redis/blob/master/redis/utils.scm#L65

rekado commented 2 years ago

It seems to me that the read-reply there should actually be redis-read-delimited.

aconchillo commented 2 years ago

hi @rekado . i'm trying to reproduce it without success:

scheme@(guile-user)> (use-modules (redis))
scheme@(guile-user)> (define conn (redis-connect))
scheme@(guile-user)> (redis-send conn (ping))
$1 = "PONG"
scheme@(guile-user)> (redis-send conn (rpush '("guilelist" "one" "two" "three" "four")))
$2 = 4
scheme@(guile-user)> (redis-send conn (lrange (list "guilelist" 0 1)))
$3 = ("one" "two")
scheme@(guile-user)> 

what's the content of your list?

aconchillo commented 2 years ago

It seems to me that the read-reply there should actually be redis-read-delimited.

No, because RESP Arrays can contain mixed types and redis-read-delimited doesn't handle that.

https://redis.io/docs/reference/protocol-spec/#resp-arrays

aconchillo commented 2 years ago

There's however a bug if we use mixed types:

scheme@(guile-user)> (redis-send conn (rpush '("mixedlist" "one" 2 "three" 4)))
$4 = 4
scheme@(guile-user)> (redis-send conn (lrange (list "mixedlist" 0 3)))
$5 = ("one" "2" "three" "4")

2 and 4 should be numbers.

aconchillo commented 2 years ago

There's however a bug if we use mixed types:

scheme@(guile-user)> (redis-send conn (rpush '("mixedlist" "one" 2 "three" 4)))
$4 = 4
scheme@(guile-user)> (redis-send conn (lrange (list "mixedlist" 0 3)))
$5 = ("one" "2" "three" "4")

2 and 4 should be numbers.

Apologies, that is actually not true, it should be strings as it is. It can be tried here: https://redis.io/commands/rpush/

rekado commented 2 years ago

Turns out that the problem only happens when the list contains an empty string.

scheme@(guile-user)> ,use (redis)
scheme@(guile-user)> ,use (redis commands)
scheme@(guile-user)> (define conn (redis-connect))
scheme@(guile-user)> (redis-send conn (lpush (list "newtest" ".")))
$1 = 2
scheme@(guile-user)> (redis-send conn (lpush (list "newtest" "")))
$2 = 3
scheme@(guile-user)> (redis-send conn (lpush (list "newtest" "")))
$3 = 4
scheme@(guile-user)> (redis-send conn (lrange (list "newtest" 0 100)))
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
Throw to key `redis-invalid' with args `(#<<redis-connection> host: "127.0.0.1" port: 6379 sock: #<input-output: socket 13>>)'.

Entering a new prompt.  Type `,bt' for a backtrace or `,q' to continue.
scheme@(guile-user) [1]> ,bt
In redis/utils.scm:
    65:18  3 (read-reply _)
    48:34  2 (build-list _ #<procedure 7f14f7a34680 at redis/utils.scm:65:34 ()>)
    70:12  1 (read-reply #<<redis-connection> host: "127.0.0.1" port: 6379 sock: #<input-output: socket 13>>)
In ice-9/boot-9.scm:
  1685:16  0 (raise-exception _ #:continuable? _)
scheme@(guile-user) [1]> 
aconchillo commented 2 years ago

This worked for me as well:

scheme@(redis)> (redis-send conn (lpush (list "newtest" "")))
$7 = 2
scheme@(redis)> (redis-send conn (lrange (list "newtest" 0 100)))
$8 = ("" ".")
scheme@(redis)> (redis-send conn (lpush (list "newtest" "")))
$9 = 3
scheme@(redis)> (redis-send conn (lrange (list "newtest" 0 100)))
$10 = ("" "" ".")
scheme@(redis)> 

Are you running 2.2.0 or master? Empty strings where fixed in version 2.1.2. https://github.com/aconchillo/guile-redis/commit/f4fea1fae8fb4a3afed9cb5553c35e65bb57d923

rekado commented 2 years ago

Ah crap, I didn't realize we're still on 2.1.1 in Guix... My apologies for taking up your time! I'll upgrade the package in Guix right away.

aconchillo commented 2 years ago

Ah crap, I didn't realize we're still on 2.1.1 in Guix... My apologies for taking up your time! I'll upgrade the package in Guix right away.

No worries! Glad it was this. Thanks!