search

LauJensen / clojureql

ClojureQL is superior SQL integration for Clojure
https://clojureql.sabrecms.com
Eclipse Public License 1.0
285 stars 39 forks source link

Let `in` predicate accept any seq #99

Closed lynaghk closed 12 years ago

lynaghk commented 12 years ago

I was surprised that when I passed a list into in:

(-> tbl
    (select (where (in :column a_list))))

the compiled SQL only contained the first element of the list. I dug into the source and found that it was because prefix operators check their arguments with vector?. I changed this to the more general seq?, and it seems to work fine for me (both in the REPL, and all of the tests pass when running cake test).

bendlas commented 12 years ago

Thanks for the PR.

Unfortunately, seq? is not a generalization of vector? because it returns false on vectors. The function you might have been looking for is sequential?. Also, there is no testcase for IN, that would have caught that.

Would you mind adding test cases and correcting the code?

kind regards

lynaghk commented 12 years ago

Thanks for the catch! I just started with Clojure, and I probably shouldn't be submitting pull requests at 10 p.m. anyway = )

I've amended the commit to use coll? and updated the test cases to try passing a vector, list, and set to in.

bendlas commented 12 years ago

That seems to work fine, thanks!