stefanwille / crystal-redis

Full featured Redis client for Crystal
MIT License
380 stars 61 forks source link

Syntax error with SET NX/XX #72

Closed mistoo closed 4 years ago

mistoo commented 5 years ago

redis.set("foo", "bar", nx: true) raises exception:

Unhandled exception: ERR syntax error (Redis::Error)
  from lib/redis/src/redis/connection.cr:0:7 in 'receive'
  from lib/redis/src/redis/strategy/single_statement.cr:12:5 in 'command'
  from lib/redis/src/redis.cr:309:7 in 'command'

Quick fix (not sure if it's correct):

--- commands.cr.orig    2018-10-26 10:11:40.898628867 +0200
+++ commands.cr 2018-10-26 10:12:03.481962675 +0200
@@ -44,12 +44,12 @@
     # redis.set("foo", "test")
     # redis.set("bar", "test", ex: 7)
     # ```
-    def set(key, value, ex = nil, px = nil, nx = nil, xx = nil)
+   def set(key, value, ex = nil, px = nil, nx = false, xx = false)
       q = ["SET", key.to_s, value.to_s]
       q << "EX" << ex.to_s if ex
       q << "PX" << px.to_s if px
-      q << "NX" << nx.to_s if nx
-      q << "XX" << xx.to_s if xx
+     q << "NX" if nx
+     q << "XX" if xx
       string_or_nil_command(q)
     end
stefanwille commented 5 years ago

Hi @mistoo, thanks for the issue report. I am quite busy at the moment, as I am traveling. I may have time only in 1 or 2 weeks to look at this and publish a fix.

m-o-e commented 4 years ago

Any chance to commit this fix?

Having the set-command work correctly seems pretty important for a redis client. ;) (that's not to diminish your work, thanks a lot for this shard that otherwise works perfectly for me! 🙇)

The problem is that there doesn't seem to be a workaround (short of monkey-patching) for users who need these parameters.

Fwiw, here's the monkey-patch for anyone else who needs this:

# Include this anywhere in your code to make
# the NX and XX parameters work.
class Redis
  module Commands
    def set(key, value, ex = nil, px = nil, nx = nil, xx = nil)
      q = ["SET", key.to_s, value.to_s]
      q << "EX" << ex.to_s if ex
      q << "PX" << px.to_s if px
      q << "NX" if nx
      q << "XX" if xx
      string_or_nil_command(q)
    end
  end
end
stefanwille commented 4 years ago

I forgot about this issue completely. A fix is in master now.

m-o-e commented 4 years ago

Awesome, thanks!