seancorfield / next-jdbc

A modern low-level Clojure wrapper for JDBC-based access to databases.
https://cljdoc.org/d/com.github.seancorfield/next.jdbc/
Eclipse Public License 1.0
767 stars 90 forks source link

Optionally don't add metadata to rows in ResultSetBuilder #240

Closed sirmspencer closed 1 year ago

sirmspencer commented 1 year ago

Is your feature request related to a problem? Please describe. We never use the metadata added to the result set row. It can cause hard to find bugs elsewhere. For example when using nippy/freeze to serialize. Metadata on the rows doesn't show in the output of most logs, so its hard to even see its there.

Unfreezable type: class next.jdbc.result_set$navize_row$fn__1985 {:type next.jdbc.result_set$navize_row$fn__1985, :as-str "#object[next.jdbc.result_set$navize_row$fn__1985 0x5f766de4 \"next.jdbc.result_set$navize_row$fn__1985@5f766de4\"]"}

Describe the solution you'd like Add a binding that can be set to just not add metadata. My preference would be to have that off by default in order to prevent all these hidden bugs, but I suspect backwards compatibility would win.

seancorfield commented 1 year ago

File a bug or enhancement request against Nippy to handle metadata appropriately.

datafy/nav support requires the metadata in next.jdbc and people rely on that. Adding a conditional to omit protocol-related metadata in the three places where it is currently added imposes a performance overhead on all users of the library, as well as requiring that I document, test, and support such a feature for all time forward.

If your serialization library can't handle the metadata, you should strip it off at the point you call freeze.

sirmspencer commented 1 year ago

Fair enough, thanks.