#30 implementation consideration of command arity

[LukasMusebrink]

commands generated with redis 4.0.11

[jbt]

Hi @LukasMusebrink sorry for the delay getting round to reviewing this - this looks good, but I'm a little concerned that this seems to change more commands than it should. For example the echo command has keyless: false and readOnly: false but obviously echo is definitely keyless and readonly.

It looks like this was a change a few years ago in Redis, for commands that don't even touch the keyspace: redis/redis@5500c51 - I'm wondering the best solution for this, whether we should keep a separate list of actually-readonly commands or whether there's a better way of computing what we want the readOnly value.

I'll need to look into this a bit more but it looks like maybe something like checking whether the key indices are all 0, and there isn't the write or movablekeys flag set, then the command is readonly-safe.

[jbt]

Actually on second thought, I'm not sure what the best tactic should be here - the first argument to KEYS is not a key, it's a pattern for matching keys. I'm not altogether sure the KEYS command is something that you want to be running against a cluster, because at best it'll only return the keys from one node, and at worst it'll try and match your pattern as a key and go to the wrong node entirely (e.g. foo* maps to slot 5933 but foobar would be in slot 12325)

I think if the KEYS command is going to be supported at all, it should probably have a special-case implementation that runs the command on all nodes in the cluster then combines the results. If this is something people would like to see then we're happy to accept a PR for that.

Meanwhile though, I'll take a look at what's going on with the change to readonly behaviour because regenerating against 4.0.12 changes more things than it should.

[NatasG]

@jbt Please create a PR for the KEYS command.