redis-rb / redis-client

Simple low level client for Redis 6+
MIT License
124 stars 60 forks source link

Can not pass in nil for HMSET #87

Closed arianf closed 1 year ago

arianf commented 1 year ago

It raises an error:

[4] pry(main)> RedisCache.current.hmset('foo123', 'time', Time.current.to_s, 'foo', nil)
TypeError: Unsupported command argument type: NilClass
from /usr/local/bundle/ruby/3.2.0/gems/redis-client-0.12.0/lib/redis_client/command_builder.rb:37:in `block in generate'

Previously:

Redis.current.hmset('foo123', 'time', Time.current.to_s, 'foo', nil)
=> "OK"

Redis.current.hget('foo123', 'foo')
=> ""
byroot commented 1 year ago

This is by design. Redis hash values can only be strings, you either have not to set that key, or to cast that nil to an empty string.

arianf commented 1 year ago

@byroot it seems interesting that we auto convert integers to strings

https://github.com/redis-rb/redis-client/blob/7897ffefb4e8d1f4926bc4ad8d718fac2a4ad02f/lib/redis_client/command_builder.rb#L34-L35

but don't do something like

when nil then ''
byroot commented 1 year ago

Yes, because integers have an unambiguous string representation, so it's easy to know that's what the caller expects.

Casting nil to "" however is super opiniated, other languages would cast it to "null" or something.

Also nil is much more likely to be a mistake.

arianf commented 1 year ago

Ahh I see thanks for the explanation