search

korma / Korma

Tasty SQL for Clojure.
http://sqlkorma.com
1.48k stars 222 forks source link

(insert TABLE (values [] should return list of generated keys? #216

Open The-Alchemist opened 10 years ago

The-Alchemist commented 10 years ago

Hi there! Firstly, awesome project. :)

Secondly, I'm using Korma to talk to a MySQL on AWS's RDS, and I'm using the (insert TABLE (values [] notation (forgive me, I can't think of a better name) to insert multiple rows.

However, the return value only includes the first generated key, seemingly.

=> (count universities) 108 => (insert Institution (values universities)) {:GENERATED_KEY 110} => (select Institution (aggregate (count :*) :cnt)) [{:cnt 108}]

I might be misunderstanding something (I'm new to Clojure), so I apologize if I'm doing something wrong.

bitemyapp commented 10 years ago

Can't follow up fully right now, but to leave a breadcrumb for others:

Probably :results :keys vs. :results :results related. Changed the defaults so that bulks inserts weren't slow.

Compare the beginning and final line:

https://github.com/korma/Korma/blob/master/src/korma/core.clj#L54-L79

Less of a bug, more of an API/documentation wart that this isn't more explicit if my guess is right.

@immoh thoughts? This is something I changed awhile back.

immoh commented 10 years ago

You're doing it correctly. The documentation is a bit off, insert always returns generated keys of the first inserted record.

@bitemyapp Could you point me to a commit/issue/PR that changed the behavior? I would like to understand the history.

bitemyapp commented 10 years ago

@immoh actually, no. I scanned every commit I made in the github repo's history and couldn't find it.

Basically we flipped around the defaults as far as :results vs. :keys goes. The user can override it.

The-Alchemist commented 10 years ago

@bitemyapp: Thanks, Chris.

Is this an artifact of the MySQL JDBC driver?

If not, I'd like to make a request as someone new to korma/Clojure: can insert return all the generated keys, not just the first one? It looks like this is supported.

Or, perhaps like JDBC's PreparedStatement.executeUpdate(), just return the number of updated columns?

If someone points me in the right direction, I'm willing to learn and help out in this effort.