Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syntax error with SET NX/XX #72

Closed
mistoo opened this issue Oct 26, 2018 · 4 comments
Closed

Syntax error with SET NX/XX #72

mistoo opened this issue Oct 26, 2018 · 4 comments

Comments

@mistoo
Copy link

mistoo commented Oct 26, 2018

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
Copy link
Owner

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
Copy link

m-o-e commented Nov 21, 2019

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
Copy link
Owner

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

@m-o-e
Copy link

m-o-e commented Nov 30, 2019

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants