java.jdbc

Add execute-return-keys!

Details

  • Type: Enhancement Enhancement
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Declined
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    Developed on OSX
  • Patch:
    Code and Test

Description

Added a function execute-return-keys! which can execute a prepared statement and return the key inserted.

Though I only support a single insert, I'm returning a list of values, similar to the insert! function

Activity

Hide
Sean Corfield added a comment -

Given the availability of insert! which returns keys when it can, what use case does this satisfy?

Show
Sean Corfield added a comment - Given the availability of insert! which returns keys when it can, what use case does this satisfy?
Hide
Tejas Dinkar added a comment -

insert! creates a sql query every time it's called (from string).

I would like to reuse the prepared statement across multiple insert! calls.

I originally started making insert! accept a prepared statement, patching db-do-prepared-return-keys, but I thought the execute-return-keys! would be a better place for it.

Show
Tejas Dinkar added a comment - insert! creates a sql query every time it's called (from string). I would like to reuse the prepared statement across multiple insert! calls. I originally started making insert! accept a prepared statement, patching db-do-prepared-return-keys, but I thought the execute-return-keys! would be a better place for it.
Hide
Sean Corfield added a comment -

Why not just use db-do-prepared-return-keys for this?

Adding an arbitrary variant of execute! is not a good approach, in my opinion. If you need something lower level than insert! or execute! as they stand, all the pieces should be there already.

Show
Sean Corfield added a comment - Why not just use db-do-prepared-return-keys for this? Adding an arbitrary variant of execute! is not a good approach, in my opinion. If you need something lower level than insert! or execute! as they stand, all the pieces should be there already.
Hide
Tejas Dinkar added a comment -

I can resubmit the patch to only change db-do-prepared-return-keys.

In the current patch, I did patch db-do-prepared-return-keys, but I just created execute-return-keys! as an alternative front end for it.

Show
Tejas Dinkar added a comment - I can resubmit the patch to only change db-do-prepared-return-keys. In the current patch, I did patch db-do-prepared-return-keys, but I just created execute-return-keys! as an alternative front end for it.
Hide
Sean Corfield added a comment -

Ah, db-do-prepared-return-keys only supports one parameter group, so I can see why it won't work for you here.

So this is to address the one specific case where you want to create a single prepared statement and run it for multiple parameter groups... If that really isn't possible with the current API, a generic way to do so should be added. You can easily create a prepared statement that will return generated keys, using prepare-statement and query will accept a prepared statement instead of SQL, so this should just be a matter of ensuring execute! and/or db-do-commands can accept a prepared statement instead of SQL. That would be a more generic solution.

Show
Sean Corfield added a comment - Ah, db-do-prepared-return-keys only supports one parameter group, so I can see why it won't work for you here. So this is to address the one specific case where you want to create a single prepared statement and run it for multiple parameter groups... If that really isn't possible with the current API, a generic way to do so should be added. You can easily create a prepared statement that will return generated keys, using prepare-statement and query will accept a prepared statement instead of SQL, so this should just be a matter of ensuring execute! and/or db-do-commands can accept a prepared statement instead of SQL. That would be a more generic solution.
Hide
Tejas Dinkar added a comment -

yes, but sadly execute! only returns the number of rows updated, not the actual rows (ids), though it accepts a prepared statement.

So there is a need for some generic API that is similar to execute!, but returns the rows.

Show
Tejas Dinkar added a comment - yes, but sadly execute! only returns the number of rows updated, not the actual rows (ids), though it accepts a prepared statement. So there is a need for some generic API that is similar to execute!, but returns the rows.
Hide
Sean Corfield added a comment - - edited

Looking more closely at db-do-prepared - which execute! uses - it should accept a prepared statement instead of a SQL string and thus so should execute!.

Therefore, you should be able to do something like:

(execute! db-spec [(prepare-statement (get-connection db-spec) "insert ..." :return-keys true) params params])
Show
Sean Corfield added a comment - - edited Looking more closely at db-do-prepared - which execute! uses - it should accept a prepared statement instead of a SQL string and thus so should execute!. Therefore, you should be able to do something like:
(execute! db-spec [(prepare-statement (get-connection db-spec) "insert ..." :return-keys true) params params])
Hide
Sean Corfield added a comment -

And I've just noticed that this functionality is new in 0.3.4 which has not yet been released! OK, so if I make a new release, you will have the functionality you need.

Show
Sean Corfield added a comment - And I've just noticed that this functionality is new in 0.3.4 which has not yet been released! OK, so if I make a new release, you will have the functionality you need.
Hide
Tejas Dinkar added a comment - - edited

(yes, I know about that patch to 0.3.4, I submitted that patch).

I believe the only way to get the generated keys, is via the .getGeneratedKeys function, which is used in [1]. It is only in the execution path for insert!, not execute!.

That said, it is possible to do the following (just tried it), so maybe this patch isn't as useful as I'd thought.

(let [prepared-statement (j/prepare-statement ... :return-keys true)]
  (j/execute! db prepared-statement)
  (j/result-set-seq (.getGeneratedKeys prepared-statement)))

[1] https://github.com/clojure/java.jdbc/blob/master/src/main/clojure/clojure/java/jdbc.clj#L684

Show
Tejas Dinkar added a comment - - edited (yes, I know about that patch to 0.3.4, I submitted that patch). I believe the only way to get the generated keys, is via the .getGeneratedKeys function, which is used in [1]. It is only in the execution path for insert!, not execute!. That said, it is possible to do the following (just tried it), so maybe this patch isn't as useful as I'd thought.
(let [prepared-statement (j/prepare-statement ... :return-keys true)]
  (j/execute! db prepared-statement)
  (j/result-set-seq (.getGeneratedKeys prepared-statement)))
[1] https://github.com/clojure/java.jdbc/blob/master/src/main/clojure/clojure/java/jdbc.clj#L684
Hide
Sean Corfield added a comment -

I've made the 0.3.4 release. The code you show looks like a reasonable compromise given my reluctance to extend the API in somewhat arbitrary ways

I'm going to close this.

Show
Sean Corfield added a comment - I've made the 0.3.4 release. The code you show looks like a reasonable compromise given my reluctance to extend the API in somewhat arbitrary ways I'm going to close this.
Hide
Sean Corfield added a comment -

With the 0.3.4 release allowing execute! to accept a prepared statement and the reasonably clean way to execute a prepared statement and then get the generated keys, I don't think a change to the API is necessary.

Show
Sean Corfield added a comment - With the 0.3.4 release allowing execute! to accept a prepared statement and the reasonably clean way to execute a prepared statement and then get the generated keys, I don't think a change to the API is necessary.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: