[JDBC-111] db-do-prepared does not correctly check if transaction? was passed Created: 26/Jul/15 Updated: 27/Jul/15 Resolved: 27/Jul/15
|Reporter:||Stefan Kamphausen||Assignee:||Sean Corfield|
The function db-do-prepared allows an optional second parameter "transaction?". Whether or not this was passed is done by checking it for being a string:
The assumption behind this seems to be, that a string indicates an SQL query parameter.
But, as the code a little further down shows, users can also pass in a prepared statement:
So, to find out if transaction? was passed as a parameter one could either check for string and prepared statement or turn the logic around an check for boolean:
The latter does not deal with nil which would probably be a natural choice for passing falsehood while the former would have to be extended when another thing may be passed as SQL, which seems unlikely.
|Comment by Sean Corfield [ 26/Jul/15 2:11 PM ]|
The assumption was that if you wanted to pass a PreparedStatement, you had to provide transaction? first (i.e., it could only be omitted when passing a sql string).
I can see value in allowing it to be omitted when a PreparedStatement is passed by expanding the condition to: (if (or (string? transaction?) (instance? PreparedStatement transaction?)) ...
|Comment by Stefan Kamphausen [ 26/Jul/15 3:56 PM ]|
Assuming that transaction? be passed anyway, if a prepared statement is passed, is fair enough, too. In that case, I humbly suggest adjusting the docstring a bit.
BTW, just replacing
Breaks the tests. Seems like it is not as easy as it seemed at first glance.
|Comment by Stefan Kamphausen [ 26/Jul/15 4:01 PM ]|
works of course. Sorry for the noise.
|Comment by Stefan Kamphausen [ 26/Jul/15 4:11 PM ]|
1. Adjust check for transaction? to test for PreparedStatement, too
2. First patch plus addition to docstring.
|Comment by Sean Corfield [ 27/Jul/15 12:31 AM ]|
Fixed. Thank you!