Skip to content

Commit 8f2ad94

Browse files
nicolasffnicolasff
committed
Fixed MULTI/EXEC bug with ZRANGE.
1 parent b9a1c75 commit 8f2ad94

File tree

4 files changed

+110
-36
lines changed

4 files changed

+110
-36
lines changed

‎library.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,12 @@ PHPAPI void redis_long_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_s
292292
}
293293

294294

295-
PHPAPI void redis_sock_read_multibulk_reply_zipped(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab TSRMLS_DC) {
295+
PHPAPI int redis_sock_read_multibulk_reply_zipped(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab TSRMLS_DC) {
296296

297-
redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL TSRMLS_CC);
297+
int ret = redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL TSRMLS_CC);
298298
array_zip_values_and_scores(INTERNAL_FUNCTION_PARAM_PASSTHRU, 1);
299+
300+
return ret;
299301
}
300302

301303
PHPAPI void redis_1_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab TSRMLS_DC) {

‎library.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ PHPAPI int redis_sock_disconnect(RedisSock *redis_sock TSRMLS_DC);
1717
PHPAPI char *redis_sock_read_bulk_reply(RedisSock *redis_sock, int bytes);
1818
PHPAPI int redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *_z_tab TSRMLS_DC);
1919
PHPAPI int redis_sock_read_multibulk_reply_loop(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, int numElems TSRMLS_DC);
20-
PHPAPI void redis_sock_read_multibulk_reply_zipped(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab TSRMLS_DC);
20+
PHPAPI int redis_sock_read_multibulk_reply_zipped(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab TSRMLS_DC);
2121
PHPAPI int redis_sock_write(RedisSock *redis_sock, char *cmd, size_t sz);
2222
PHPAPI void redis_check_eof(RedisSock *redis_sock TSRMLS_DC);
2323
//PHPAPI int redis_sock_get(zval *id, RedisSock **redis_sock TSRMLS_DC);

‎redis.c

Lines changed: 92 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -855,7 +855,6 @@ PHP_METHOD(Redis, delete)
855855
1, &redis_sock TSRMLS_CC);
856856
zval * object = getThis();
857857

858-
859858
IF_ATOMIC() {
860859
redis_long_response(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL TSRMLS_CC);
861860
}
@@ -2670,7 +2669,10 @@ PHP_METHOD(Redis, zRange)
26702669
REDIS_PROCESS_RESPONSE(redis_sock_read_multibulk_reply_zipped);
26712670
} else {
26722671
IF_ATOMIC() {
2673-
redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL TSRMLS_CC);
2672+
if (redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU,
2673+
redis_sock, NULL TSRMLS_CC) < 0) {
2674+
RETURN_FALSE;
2675+
}
26742676
}
26752677
REDIS_PROCESS_RESPONSE(redis_sock_read_multibulk_reply);
26762678
}
@@ -2790,9 +2792,9 @@ PHP_METHOD(Redis, zReverseRange)
27902792
"$%d" _NL\
27912793
"%s" _NL\
27922794
"$%d" _NL\
2793-
"%f" _NL\
2795+
"%d" _NL\
27942796
"$%d" _NL\
2795-
"%f" _NL
2797+
"%d" _NL
27962798

27972799
if(withscores) {
27982800
cmd_len = redis_cmd_format(&cmd,
@@ -2801,15 +2803,15 @@ PHP_METHOD(Redis, zReverseRange)
28012803
"$10" _NL
28022804
"WITHSCORES" _NL
28032805
, key_len, key, key_len
2804-
, double_length(start), start
2805-
, double_length(end), end);
2806+
, integer_length(start), start
2807+
, integer_length(end), end);
28062808
} else {
28072809
cmd_len = redis_cmd_format(&cmd,
28082810
"*4" _NL
28092811
ZREVRANGE_FORMAT
28102812
, key_len, key, key_len
2811-
, double_length(start), start
2812-
, double_length(end), end);
2813+
, integer_length(start), start
2814+
, integer_length(end), end);
28132815

28142816

28152817
}
@@ -2823,7 +2825,10 @@ PHP_METHOD(Redis, zReverseRange)
28232825
REDIS_PROCESS_RESPONSE(redis_sock_read_multibulk_reply_zipped);
28242826
} else {
28252827
IF_ATOMIC() {
2826-
redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL TSRMLS_CC);
2828+
if (redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU,
2829+
redis_sock, NULL TSRMLS_CC) < 0) {
2830+
RETURN_FALSE;
2831+
}
28272832
}
28282833
REDIS_PROCESS_RESPONSE(redis_sock_read_multibulk_reply);
28292834
}
@@ -2840,6 +2845,8 @@ PHP_METHOD(Redis, zRangeByScore)
28402845
int key_len, cmd_len, response_len;
28412846
zend_bool withscores = 0;
28422847
double start, end;
2848+
int has_limit = 0;
2849+
long limit_low, limit_high;
28432850

28442851
if (zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), "Osdd|a",
28452852
&object, redis_ce,
@@ -2863,26 +2870,87 @@ PHP_METHOD(Redis, zRangeByScore)
28632870
if(zend_hash_find(Z_ARRVAL_P(z_options), "limit", sizeof("limit"), (void**)&z_limit_val_pp)== SUCCESS) {;
28642871
if(zend_hash_num_elements(Z_ARRVAL_PP(z_limit_val_pp)) == 2) {
28652872
zval **z_offset_pp, **z_count_pp;
2866-
/* get the two values from the table, check that they are indeed of LONG type */
2873+
// get the two values from the table, check that they are indeed of LONG type
28672874
if(SUCCESS == zend_hash_index_find(Z_ARRVAL_PP(z_limit_val_pp), 0, (void**)&z_offset_pp) &&
28682875
SUCCESS == zend_hash_index_find(Z_ARRVAL_PP(z_limit_val_pp), 1, (void**)&z_count_pp) &&
28692876
Z_TYPE_PP(z_offset_pp) == IS_LONG &&
28702877
Z_TYPE_PP(z_count_pp) == IS_LONG) {
28712878

2872-
spprintf(&limit, 0, " LIMIT %ld %ld", Z_LVAL_PP(z_offset_pp), Z_LVAL_PP(z_count_pp));
2879+
has_limit = 1;
2880+
limit_low = Z_LVAL_PP(z_offset_pp);
2881+
limit_high = Z_LVAL_PP(z_count_pp);
28732882
}
28742883
}
28752884
}
28762885
}
28772886

2887+
#define BASIC_FORMAT\
2888+
"$13" _NL\
2889+
"ZRANGEBYSCORE" _NL\
2890+
\
2891+
"$%d" _NL /* key_len */\
2892+
"%s" _NL /* key */\
2893+
\
2894+
"$%d" _NL /* start_len */\
2895+
"%f" _NL /* start */\
2896+
\
2897+
"$%d" _NL /* end_len */\
2898+
"%f" _NL /* end */
2899+
#define BASIC_FORMAT_WITH_LIMIT BASIC_FORMAT\
2900+
"$5" _NL\
2901+
"LIMIT" _NL\
2902+
\
2903+
"$%d" _NL /* limit_low_len */\
2904+
"%d" _NL /* limit_low */\
2905+
\
2906+
"$%d" _NL /* limit_high_len */\
2907+
"%d" _NL /* limit_high */
2908+
28782909
if(withscores) {
2879-
cmd_len = spprintf(&cmd, 0, "ZRANGEBYSCORE %s %f %f%s WITHSCORES\r\n", key, start, end, limit?limit:"");
2910+
if(has_limit) {
2911+
cmd_len = redis_cmd_format(&cmd,
2912+
"*8" _NL
2913+
BASIC_FORMAT_WITH_LIMIT
2914+
"$10" _NL
2915+
"WITHSCORES" _NL
2916+
, key_len, key, key_len
2917+
, double_length(start), start
2918+
, double_length(end), end
2919+
, integer_length(limit_low), limit_low
2920+
, integer_length(limit_high), limit_high);
2921+
} else {
2922+
cmd_len = redis_cmd_format(&cmd,
2923+
"*5" _NL
2924+
BASIC_FORMAT
2925+
"$10" _NL
2926+
"WITHSCORES" _NL
2927+
, key_len, key, key_len
2928+
, double_length(start), start
2929+
, double_length(end), end);
2930+
}
28802931
} else {
2881-
cmd_len = spprintf(&cmd, 0, "ZRANGEBYSCORE %s %f %f%s\r\n", key, start, end, limit?limit:"");
2882-
}
2883-
if(limit) {
2884-
efree(limit);
2932+
2933+
if(has_limit) {
2934+
2935+
cmd_len = redis_cmd_format(&cmd,
2936+
"*7" _NL
2937+
BASIC_FORMAT_WITH_LIMIT
2938+
, key_len, key, key_len
2939+
, double_length(start), start
2940+
, double_length(end), end
2941+
, integer_length(limit_low), limit_low
2942+
, integer_length(limit_high), limit_high);
2943+
} else {
2944+
cmd_len = redis_cmd_format(&cmd,
2945+
"*4" _NL
2946+
BASIC_FORMAT
2947+
, key_len, key, key_len
2948+
, double_length(start), start
2949+
, double_length(end), end);
2950+
}
28852951
}
2952+
#undef BASIC_FORMAT
2953+
#undef BASIC_FORMAT_WITH_LIMIT
28862954

28872955
REDIS_PROCESS_REQUEST(redis_sock, cmd, cmd_len);
28882956
if(withscores) {
@@ -2891,12 +2959,17 @@ PHP_METHOD(Redis, zRangeByScore)
28912959
* we want [elt0 => val0, elt1 => val1], etc.
28922960
*/
28932961
IF_ATOMIC() {
2894-
redis_sock_read_multibulk_reply_zipped(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL TSRMLS_CC);
2962+
if(redis_sock_read_multibulk_reply_zipped(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL TSRMLS_CC) < 0) {
2963+
RETURN_FALSE;
2964+
}
28952965
}
28962966
REDIS_PROCESS_RESPONSE(redis_sock_read_multibulk_reply_zipped);
28972967
} else {
28982968
IF_ATOMIC() {
2899-
redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL TSRMLS_CC);
2969+
if(redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU,
2970+
redis_sock, NULL TSRMLS_CC) < 0) {
2971+
RETURN_FALSE;
2972+
}
29002973
}
29012974
REDIS_PROCESS_RESPONSE(redis_sock_read_multibulk_reply);
29022975
}
@@ -3560,7 +3633,7 @@ PHP_METHOD(Redis, multi)
35603633
current = NULL;
35613634

35623635
IF_MULTI() {
3563-
cmd_len = redis_cmd_format(&cmd, "MULTI \r\n");
3636+
cmd_len = redis_cmd_format(&cmd, "*1" _NL "$5" _NL "MULTI" _NL);
35643637

35653638
if (redis_sock_write(redis_sock, cmd, cmd_len) < 0) {
35663639
efree(cmd);

‎tests/testPipeline.php

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@ function test1($r, $type) {
169169
}
170170

171171
function test2($r, $type) {
172-
173172
// general command
174173
$ret = $r->multi($type)
175174
->select(3)
@@ -426,18 +425,18 @@ function test2($r, $type) {
426425
->zRangeByScore('zkey1', 1, 6)
427426
->zCard('zkey1')
428427
->zScore('zkey1', 'zValue15')
429-
// ->zadd('zkey2', 5, 'zValue5')
430-
// ->zadd('zkey2', 2, 'zValue2')
431-
// //->zRange('zkey1', 'zkey2')
432-
// ->zInter('zInter', array('zkey1', 'zkey2'))
433-
// ->zRange('zkey1', 0, -1)
434-
// ->zRange('zkey2', 0, -1)
435-
// ->zRange('zInter', 0, -1)
436-
// ->zInter('zUnion', array('zkey1', 'zkey2'))
437-
// ->zRange('zUnion', 0, -1)
438-
// ->zadd('zkey5', 5, 'zValue5')
439-
// ->zIncrBy('zkey5', 3, 'zValue5') /* fix this */
440-
// ->zScore('zkey5', 'zValue5')
428+
->zadd('zkey2', 5, 'zValue5')
429+
->zadd('zkey2', 2, 'zValue2')
430+
//->zRange('zkey1', 'zkey2')
431+
->zInter('zInter', array('zkey1', 'zkey2'))
432+
->zRange('zkey1', 0, -1)
433+
->zRange('zkey2', 0, -1)
434+
->zRange('zInter', 0, -1)
435+
->zInter('zUnion', array('zkey1', 'zkey2'))
436+
->zRange('zUnion', 0, -1)
437+
->zadd('zkey5', 5, 'zValue5')
438+
->zIncrBy('zkey5', 3, 'zValue5') /* fix this */
439+
->zScore('zkey5', 'zValue5')
441440
->exec();
442441
// var_dump($ret);
443442

@@ -484,7 +483,7 @@ function test2($r, $type) {
484483

485484

486485
$count = 10000;
487-
$count = 100;
486+
$count = 10000;
488487

489488
for($i = 1; $i <= $count; $i++) {
490489
test1($r, Redis::MULTI);

0 commit comments

Comments
 (0)