search

yiisoft / yii2-redis

Yii 2 Redis extension.
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
452 stars 183 forks source link

Null values not allowed anymore #190

Closed tehmaestro closed 4 years ago

tehmaestro commented 4 years ago

Hi, regarding this commit https://github.com/yiisoft/yii2-redis/commit/c441761484d98af0065420d37b47e157e82a1235

I find it to be a major change. I have many HMSET key a 1 b 2 c 3 commands that, previously would accept a null value, like HMSET key a null b 2 c 3 But now, it completely changes the number of arguments, removing null arguments, which leads to many issues. Is this intended? Thanks.

Q A
Yii vesion 2.0.28
PHP version 7.2
Operating system Linux
samdark commented 4 years ago

@razonyang any comments on the topic?

rob006 commented 4 years ago

IMO fix from #166 should be reverted. nulls could be skipped at command call level (it probably needs some documentation):

$redis->zrangebyscore('application:delay', 0, 1555544444, 'LIMIT', 0, 10);

instead of 

$redis->zrangebyscore('application:delay', 0, 1555544444, null, 'LIMIT', 0, 10);
razonyang commented 4 years ago

@samdark I am agree with @rob006, fix #167 breaks compatibility, I apologize for not thinking enough.

samdark commented 4 years ago

It was done in 2.0.9 September 23, 2018. Do we need to break current code by reverting it?

rob006 commented 4 years ago

@samdark Current behavior is incorrect (and I doubt that it is used on large scale). #166 should be fixed by adjusting documentation (I have no idea how - you can't simply map redis command line interface to PHP methods).

samdark commented 4 years ago
  1. Need to revert it.
  2. Need to mention it in UPGRADE.

@razonyang do you want to handle it?