Clojurians Log v2
sql
is there a way to determine, whether the :multi-rs option is supported for a specific jdbc implementation?
the tips&tricks next-jdbc docs don't say it explicitly and it seems to only return a single result, since sqlite's begin/end doesn't seem to document multiple return values:
(jdbc/execute!
(jdbc/get-datasource "jdbc:sqlite::memory:")
["begin; pragma table_list; pragma table_info(sqlite_schema); end"]
{:multi-rs true})
=> [[{:next.jdbc/update-count 0}]]maybe i should try it with batching?
since it doesn't seem to be a valid use to provide multiple statements within a jdbc/execute! call anyway...
That's definitely all very DB-specific, unfortunately. Happy to add more notes to Tips & Tricks if you want to create an issue with details, or submit a PR.
good to know, but i don't think im qualified enough yet to make such calls. in the meantime, i leave some of my experiments here, in case it's useful for others too. This is supposed to return the id values (or the implicit ROWID column, if we https://www.sqlite.org/c3ref/last_insert_rowid.html), but it returns an empty map instead:
(require '[next.jdbc :as jdbc])
(require '[next.jdbc.result-set :as rs])
(jdbc/with-transaction [tx "jdbc:sqlite::memory:"]
(jdbc/execute-one! tx ["create table t (id integer primary key autoincrement, c)"])
(with-open
[insert (jdbc/prepare tx ["insert into t (c) values (?)"] {:return-keys [:id]})]
[(jdbc/execute-batch! insert [[10] [20] [30]]
{:batch-size 2
:return-generated-keys true})
(jdbc/execute! tx ["select c from t"] {:builder-fn rs/as-arrays})]))
=> [[] [[:t/c] [10] [20] [30]]]=> [[1 1 1] [[:t/c] [10] [20] [30]]](jdbc/with-transaction [tx "jdbc:sqlite::memory:"]
(jdbc/execute-one! tx ["create table t (id integer primary key autoincrement, c)"])
[(jdbc/execute! tx ["insert into t (c) values (?), (?)" 10 20] {:return-keys false})
(jdbc/execute! tx ["insert into t (c) values (?), (?)" 30 40] {:return-keys [:id]})
(jdbc/execute! tx ["insert into t (c) values (?), (?) returning id,c" 50 60])])
=> [[{:next.jdbc/update-count 2}]
[{:last_insert_rowid() 4}]
[{:t/id 5, :t/c 50} {:t/id 6, :t/c 60}]]RETURNING (https://sqlite.org/lang_returning.html) clause works, but that can't be used in a batch, because we get an error about the returned values:
(jdbc/with-transaction [tx "jdbc:sqlite::memory:"]
(jdbc/execute-one! tx ["create table t (c)"])
(with-open
[insert (jdbc/prepare tx ["insert into t (c) values (?) returning *"]
{:return-keys true})]
(jdbc/execute-batch! insert [[10] [20] [30]] {:return-generated-keys true})))
Execution error (BatchUpdateException) at org.sqlite.core.DB/executeBatch (DB.java:944).
batch entry 0: query returns resultssomeone already "complained" about it over in the sqlite jdbc driver: https://github.com/xerial/sqlite-jdbc/issues/1186 while i agree that referenced the docs are misleading, i also agree, it would be a little too much magic trying to fiddle with sql statement itself and inject the RETURNING clause, if the generated keys are requested... 😕
I'm using integrant, and not component, and have need to allow a service to be configured optionally with a connection pool and pool specific configuration options. I can do that via connection/->pool obviously, but the issue then is in closing the pool afterwards. As this is itself a library, then I'd rather give the client the control over pool configuration as they know best. There is an internal private function attempt-close that is used by the component implementation. Would there be interest in a small PR to make attempt-close a) do the check to see if the handle implements http://java.io.Closeable, and then falls back to that reflective approach looking for a 'close' method? and b) remove the :private metadata? I can obviously implement this myself manually if there is not interest in widening the API at this point?
I just checked and it looks as if c3p0 added Autocloseable to its datasource as of Jan 24. java.io.Closeable implements Autocloseable, so arguably all of this could be replaced by a check for Autocloseable. See https://github.com/swaldman/c3p0/commit/a3906341a86c610327aa95c9429e82529ebca9c4.
That's good to know. Can you create an issue on gh so I remember to look into this and make changes? I'm on the road to Conj right now.
Of course. Enjoy!
Crux of the matter is that it would be ill-advised to make the fallback close mechanics public, given that any user nowadays can just call .close without reflection themselves. Sorry for all of the noise.
Just to be clear, that whole attempt close machinery right now is specific to Component and allowing "any" connection pooling library. But realistically there are only Hikari and c3p0
That (private) function is currently used in only one place, like so:
([clazz db-spec]
(component clazz db-spec #(if (isa? clazz java.io.Closeable)
(.close ^java.io.Closeable %)
(attempt-close %))))instance? check inside attempt-close, but it needs a better name if it becomes public, and then it's part of the documented API of next.jdbc.connection...So, yeah, feel free to submit a PR and come up with a better name...
we have a https://github.com/xtdb/xtdb/blob/main/core/src/main/clojure/xtdb/util.clj#L65-L144 of these in XT's utils file, feel free to pinch whatever you find useful 🙂
Maybe I'm not getting something, but when you control the creation of something, why is there any need for introspection? Just use the way to dispose of that something.
As an example:
(defmethod ig/init-key ::src
[_ options]
(next.jdbc.connection/->pool HikariDataSource options))
(defmethod ig/halt-key! ::src
[_ ^Closeable db-src]
(try
(.close db-src)
(catch Exception _
;; Just swallow the exception - there's nothing reasonable that we can do here.
nil)))HikariDataSource is closable, so you know for sure that your ::src is closable, and you can just treat it as such, with no checks.it's more if it's not created in the first place, IIUC
Yeah, but if a client of the library controls the creation of something, it's their responsibility to dispose of it. At least that's how I'd do it.
Example: service that provides access to a SQLite database. I don't want to have to have another service to manage the connection pool; I simply want my config to look something like {:f "wibble.sql" :pool {:class 'HikariDataSource :pool-size 5 ...} and the 'service' manages that datasource. Hoisting the responsibility for the pooling to the caller in this case doesn't work. next.jdbc.connection/->pool provides that configuration, but the 'close' part is harder without reflection. One option is to hardcode two options e.g. :hikari :c3po and manage the differences internally, but as next.jdbc exposes this hook for the creation of the pool, it feels fairly appropriate to also provide a hook for safe closing?
I could hardcode to use Hikari and then things are easier of course.
Mm yeah, to me that definitely smells. Like probably 90% of introspection out there.
With things being general, I try to think in general terms. What if the :class that a user provides doesn't have anything that can be detected as closable? What if it is detected as such, but a proper disposal relies on something else, or on some particular procedure?
That could be not a concern at all in reality. But I myself have simply seen too many cases of "not a concern right now" turning into snafus down the line.
Yes. You might be right. Hardcoding would be fine in the integrant init, and then the library just uses whatever it is given, rather than taking responsibility for open/close. In fact maybe that is the better option here anyway.
My library has an 'open' function that just takes a file and options, so hardcoding pooling there would be wrong. But hardcoding a pool at the integrant system level ie. in init method, would be absolutely fine.
Thanks all. Perhaps can avoid after all.
Could be something like :class + :closing-fn :closable (maybe by default) for most cases.
:class + :closing-fn some.fqn for the rest.
The reason next.jdbc does this is because, while Hikari's pooled datasource is Closeable, c3p0's is not and casting it to Closeable to call .close on it doesn't work (as I recall). Calling .close on it without the type hint does work, but omitting the type hint produces a reflection warning (again, as I recall) and I didn't want to be that annoying library maintainer that has reflection warnings in his project 🙂
So, yeah, this machinery is kind of ugly and it's weird to have explicit Java reflection stuff going on but...
(I do recall that this was all a PITA to get working without reflection warnings)