search

aviggiano / redis-roaring

Roaring Bitmaps for Redis
MIT License
345 stars 55 forks source link

Require to use in redis cluster #65

Closed jiangtao244 closed 3 years ago

jiangtao244 commented 3 years ago

As https://redis.io/topics/modules-api-ref say:

we need to implement two func below:

  1. RedisModule_IsKeysPositionRequest(ctx);

  2. RedisModule_KeyAtPos(ctx,pos);

aviggiano commented 3 years ago

@jiangtao244 thank you for the heads up, I wasn't aware of these requirements. Would you be interested in submitting a pull request?

jiangtao244 commented 3 years ago

@aviggiano , sorry to say, it's difficult for me to do this job, all is new to me.

aviggiano commented 3 years ago

No problem @jiangtao244 I'll look into this soon to fix this issue. Thanks

jiangtao244 commented 3 years ago

I just read the redis source code. https://github.com/redis/redis/blob/4e2e5be201439cae4c0a03cfc8b6a60be4bff625/src/module.c#L705-L707

Find that we have no need to use these two api. RedisModule_IsKeysPositionRequest(ctx); RedisModule_KeyAtPos(ctx,pos);

Sorry, not fully investigated.

jiangtao244 commented 3 years ago

R.BITOP [ ...]

Bug here: https://github.com/aviggiano/redis-roaring/blob/ef9b5752e79e0e0d3889e9e7b404abb9393b43e4/src/redis-roaring.c#L719 shoud be if (RedisModule_CreateCommand(ctx, "R.BITOP", RBitOpCommand, "write", 2, -1, 1) == REDISMODULE_ERR) { return REDISMODULE_ERR; } Refered: https://redis.io/commands/command https://github.com/redis/redis/blob/58f79e2ff48bb20e48d5eb60ff6a87c9afae5fe9/src/server.c#L287