Clojurians Log v2

There’s some additional discussion on https://github.com/seancorfield/honeysql/issues/315 that I’d like more input on. I’m inclined to roll that change back out at this point…?

Assuming a function such as

(defn find-foos [ids]
  (-> (select :*)
      (from :foo)
      (where [:in :id ids])))

Thanks, @sandqvist ! Good feedback. “avoid magic, make programmer errors visible”

So I guess then a question would be: should HoneySQL throw an exception if the IN collection is empty and/or contains nil? The former seems reasonable since WHERE col IN () is going to blow up in JDBC-land anyway, but the latter is probably a grey area since it “works” in JDBC-land, it just doesn’t necessarily do what the developer might want/expect…?

This is a tricky one. For the [] and nil cases an exception would be useful if it makes complex statements easier to debug and fix than looking at the database logs and reading the SQL statement which is what I currently do as I'm still on v1. An exception without a good HoneySQL data structure context might be tricky to figure out. What I mean is unless the IN clause has a helper fn, you can only throw in format, right?

Correct. But the error message could include the SQL fragment that would fail, e.g., col IN () so you would at least see the col (or expression).

I compose HoneySQL queries quite a bit. What I'm afraid of is an error like [:in :id nil] is not allowed when I've composed three or four subqueries for tables that all have a surrogate primary key column called id. In that case, just looking at the database log is simpler.

But just a debugging option to ignore the check and let the DB complain would also be fine.

Or perhaps leave the default behavior “as-is” and add a :checking true opt-in feature that performs this check (and potential other checks in the future)?

I would prefer that, with the possibility of later adding helper fns for IN and other clauses. The stacktrace from those would be very helpful. Of course, anyone can make those.