java.jdbc

metadata-result leaks ResultSets

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

When working with Oracle DatabaseMetaData, I noticed out of cursors exceptions (ORA-01000) when iterating over tables and requesting their .getIndexInfo and .getImportedKeys information. These errors tend to point to leaked resultsets. Investigating the metadata-result code I noticed this does happen, and the result set are just leaked silently on the other databases I tend to use.

A fix would include a default result-set-fn doall in the :or keys clause, and binding the true clause of the (if (instance? java.sql.ResultSet rs-or-value) to a new var, .close the rs-or-value, and return the new var.

Activity

Hide
Sean Corfield added a comment -

Thanks Niels. I'll take a look at that shortly. I may have some time next week.

Show
Sean Corfield added a comment - Thanks Niels. I'll take a look at that shortly. I may have some time next week.
Hide
Sean Corfield added a comment - - edited

I finally got around to looking at this – apologies for the delay – and I'm not convinced metadata-result should close the ResultSet since that is passed in by the user. java.jdbc itself only ever calls metadata-result in a test so I updated that with the way I think it should be handled, by the user:

(sql/with-db-metadata [metadata db-spec]
  (with-open [result (.getTables metadata ...)]
    (let [table-info (sql/metadata-result result)]
      ... do stuff with table-info ...)))

The point here is that the user creates the ResultSet, not java.jdbc, so the user should close it correctly.

Show
Sean Corfield added a comment - - edited I finally got around to looking at this – apologies for the delay – and I'm not convinced metadata-result should close the ResultSet since that is passed in by the user. java.jdbc itself only ever calls metadata-result in a test so I updated that with the way I think it should be handled, by the user:
(sql/with-db-metadata [metadata db-spec]
  (with-open [result (.getTables metadata ...)]
    (let [table-info (sql/metadata-result result)]
      ... do stuff with table-info ...)))
The point here is that the user creates the ResultSet, not java.jdbc, so the user should close it correctly.
Hide
Niels van Klaveren added a comment - - edited

The similarities between metadata-result and query (both having row-fn and result-set-fn) made me suppose abstracting away the connection handling of the result-set was the intention. It seems that assumption was a bit too hasty.

However, currently metadata-result IMO straddles uneasily between an implementation that just returns a result-set-seq or value, and one that can do all processing intended for abstracting away connection opening and closing, without actually doing so.

This does opens the way for a quite simple workaround that does though, a simple wrapping macro

(defmacro metadata-query
  [query & params]
  `(with-open [result# ~query]
     (j/metadata-result result# ~@params)))

which takes a java metadata call and the result set processing parameters for metadata-result, and processes the results in a with-open block according to the parameters.

(j/with-db-metadata [meta db-spec]
                    (metadata-query (.getTables meta nil nil nil (into-array String ["TABLE"]))
                                    :result-set-fn (fn [rs] (->> rs
                                                                 (filter #(= (:user db-spec) (:table_schem %)))
                                                                 (map :table_name)
                                                                 doall))))
Show
Niels van Klaveren added a comment - - edited The similarities between metadata-result and query (both having row-fn and result-set-fn) made me suppose abstracting away the connection handling of the result-set was the intention. It seems that assumption was a bit too hasty. However, currently metadata-result IMO straddles uneasily between an implementation that just returns a result-set-seq or value, and one that can do all processing intended for abstracting away connection opening and closing, without actually doing so. This does opens the way for a quite simple workaround that does though, a simple wrapping macro
(defmacro metadata-query
  [query & params]
  `(with-open [result# ~query]
     (j/metadata-result result# ~@params)))
which takes a java metadata call and the result set processing parameters for metadata-result, and processes the results in a with-open block according to the parameters.
(j/with-db-metadata [meta db-spec]
                    (metadata-query (.getTables meta nil nil nil (into-array String ["TABLE"]))
                                    :result-set-fn (fn [rs] (->> rs
                                                                 (filter #(= (:user db-spec) (:table_schem %)))
                                                                 (map :table_name)
                                                                 doall))))
Hide
Sean Corfield added a comment -

A new actionable suggestion has been made.

Show
Sean Corfield added a comment - A new actionable suggestion has been made.
Hide
Sean Corfield added a comment -

That seems like a reasonable addition which would clarify the usage. Thank you.

Show
Sean Corfield added a comment - That seems like a reasonable addition which would clarify the usage. Thank you.
Hide
Sean Corfield added a comment -

Will be in 0.4.2.

Show
Sean Corfield added a comment - Will be in 0.4.2.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: