taoensso / carmine

Redis client + message queue for Clojure
https://www.taoensso.com/carmine
Eclipse Public License 1.0
1.15k stars 130 forks source link

:as-pipeline does not wrap results of a single hgetall #215

Closed brianfay closed 5 years ago

brianfay commented 5 years ago

Super minor issue, but I figured I'd bring it up:

hgetall returns a vector:

;;create two Redis Hashes
(car/wcar {:host "localhost" :port 6379}
  (car/hset "example1" "foo" "bar")
  (car/hset "example2" "baz" "qux"))

;;get a vector of [key1, value1, key2, value2... keyN, valueN]
(car/wcar {:host "localhost" :port 6379}
  (car/hgetall "example1"))
;; => ["foo" "bar"]

When using multiple hgetall commands in a pipeline, you get a vector of vectors:

(car/wcar {:host "localhost" :port 6379}
  (car/hgetall "example1")
  (car/hgetall "example2"))
;; => [["foo" "bar"] ["baz" "qux"]]

I have a use case where I'm doing hgetalls on a dynamic number of Hashes (one to many). From my understanding the :as-pipeline flag to wcar exists for this use case; this way the response is in the same format no matter how many requests you're making.

I would expect that using :as-pipeline, the response would be a vector with a vector in it, but instead I'm just getting the usual vector of keys and values.

(car/wcar :as-pipeline {:host "localhost" :port 6379}
  (car/hgetall "example1"))
;;EXPECTED => [["foo" "bar"]]
;;ACTUAL => ["foo" "bar"]
brianfay commented 5 years ago

my hacky local workaround for this is to thread the response through:

(defn- wrap-pipeline-resp
  [car-resp]
  (cond-> car-resp
    (some (complement vector?) car-resp)
    vector))
ptaoussanis commented 5 years ago

Hi Brian,

Thanks for the report. That would be a bug, but I don't appear to be able to reproduce:

(wcar {} (hset "example3" "foo" "bar"))     ; => 1
(wcar {} :as-pipeline (hgetall "example3")) ; => [["foo" "bar"]]

Could you mention what version of Carmine you're running? Thanks!

ptaoussanis commented 5 years ago

Ah, I see the problem- you've got your argument order wrong:

Try (car/wcar {:host "localhost" :port 6379} :as-pipeline ...) instead of (car/wcar :as-pipeline {:host "localhost" :port 6379} ...).

Closing :-)

brianfay commented 5 years ago

Whoops! Sorry for the false report, and thanks for looking into it.

ptaoussanis commented 5 years ago

No worries Brian, thanks for checking! 👍