Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Elknar opened this issue Feb 23, 2018 · 4 comments

Comments

@Elknar
Copy link

Elknar commented Feb 23, 2018

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
Copy link
Member

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?

ptaoussanis added a commit that referenced this issue Mar 21, 2018
This is technically in violation of the current docs (Ref. https://goo.gl/iy74on),
but seems to match actual Redis behaviour. The docs do seem to be buggy, so
choosing to match actual behaviour here- though I'm not sure how useful this is
in practice.
@Elknar
Copy link
Author

Elknar commented Mar 23, 2018

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
Copy link
Author

Elknar commented Mar 23, 2018

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
Copy link
Member

[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 👍

Hope that helps, cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants