search

metabase / toucan

A classy high-level Clojure library for defining application models and retrieving them from a DB
Eclipse Public License 1.0
570 stars 49 forks source link

Use HoneySQL to generate INSERT SQL, instead of clojure.java.jdbc #13

Closed plexus closed 7 years ago

plexus commented 7 years ago

Inserts were being done with jdbc/insert-multi!, which internally generates the necessary SQL statements. This means HoneySQL is bypassed for inserts, and so HoneySQL features like calling SQL functions with (sql/call ,,,) are unavailable.

This patch instead makes HoneySQL generate the INSERT statements, and executes them with the lower level jdbc/db-do-prepared-return-keys. When inserting multiple rows at once it executes separate INSERT statements, rather than a single statement with multiple VALUES. This behavior is the same with jdbc/insert-multi!. It's otherwise not possible to retrieve the primary keys of the inserted rows. (This is a limitation of jdbc/db-do-prepared-return-keys, not of JDBC itself).

I've added test-cases for db/insert-many! and db/simple-insert-many!, since those were still missing, as well as an extra test case for db/insert! which demonstrates the use of an SQL function call.

mazameli commented 7 years ago

@camsaul: ping in case you missed this

camsaul commented 7 years ago

Oh yeah. Thanks @mazameli i didn't get an email about this. I'll take a look at this @plexus

plexus commented 7 years ago

Thanks @camsaul! There are two other open PRs as well, a small one (#12) by @bandresen and a bigger one (#14) by me.

plexus commented 7 years ago

Hi @camsaul, thanks for reviewing this. Sorry for the delay, I had some other stuff I had to wrap up first.

I think I addressed your comments. Let me know if there's anything else.

camsaul commented 7 years ago

Thanks @plexus, Sorry I haven't got a chance to go over this the last few days but I should have time to take a look at everything tomorrow. 👍