taoensso / carmine

Redis client + message queue for Clojure
https://www.taoensso.com/carmine
Eclipse Public License 1.0
1.15k stars 130 forks source link

eval functions (eval, eval* ...) require some argument, even if you specify 0 keys. #204

Closed Elknar closed 6 years ago

Elknar commented 6 years ago

Example:

Carmine (fails):

some-ns=>(wcar* (car/eval* "return 10;" 0))

ArityException Wrong number of args (2) passed to: carmine/eval*  clojure.lang.AFn.throwArity (AFn.java:429)

Carmine (works):

some-ns=>(wcar* (car/eval* "return 10;" 0 "completely irrelevant thing"))

10

Redis-cli:

127.0.0.1:6379> eval "return 10;" 0
(integer) 10
ptaoussanis commented 6 years ago

Hi there,

So this is an interesting case. According to the Redis command spec, eval actually requires at least one key.

Obviously, as you showed though, (wcar {} (redis-call ["eval" "return 10;" 0])) does indeed work.

I could mod the spec->edn generator to handle eval as a special case - but I'm wondering: what is the utility of calling eval without any key?

Do you have any practical application in mind?

Elknar commented 6 years ago

Yup, for example using a lua script to insert multiple key-value pairs in a single call by passing the stuff as JSON.

More concise than using multi with lots of SET calls. And the JSON argument isn't a key... (wcar* (car/eval* <big lua insertion script with whatever additional logic i want for the JSON, like processing nested objects> 0 "{\"some\": \"stuff\"}"))

Elknar commented 6 years ago

According to the Redis command spec, eval actually requires at least one key.

Not exactly, keys are required if the script operates on keys. The page you linked even has the above "return 10" example with no keys. They explain:

All Redis commands must be analyzed before execution to determine which keys the command will operate on. In order for this to be true for EVAL, keys must be passed explicitly. This is useful in many ways, but especially to make sure Redis Cluster can forward your request to the appropriate cluster node.

Note this rule is not enforced in order to provide the user with opportunities to abuse the Redis single instance configuration, at the cost of writing scripts not compatible with Redis Cluster.

(My JSON scenario above is an example of the described single instance abuse)

ptaoussanis commented 6 years ago

[According to the Redis command spec, eval actually requires at least one key.] Not exactly, keys are required if the script operates on keys.

So specifically, I meant the Redis command spec.

Arguably this should mark the :key argument as optional since Redis does clearly accept that as input (as in the eval "return 10;" 0 example) :-)

Yup, for example using a lua script to insert multiple key-value pairs in a single call by passing the stuff as JSON.

Would just note that this could cause problems if there's a chance you might want to use Redis Cluster in future. (Since you're still effectively using keys, just not providing info on them to Redis).

More concise than using multi with lots of SET calls.

Just noting that another option would be MSET, which would be quite concise- while retaining compatibility with Cluster.

In any case, added support in [com.taoensso/carmine "2.18.0"] for eval and evalsha without any keys :+1:

Hope that helps, cheers!