handle java.sql.Date explicitly in nippy; preventing values to be thawed as java.util.Date

[philomates]

Since java.sql.Date is a child class of java.util.Date and there are currently no serialization rules for java.sql.Date, when nippy thaws a frozen a java.sql.Date instance it comes back as a java.util.Date.

for example

(require '[taoensso.nippy :as nippy])
(let [sql (java.sql.Date/valueOf "2023-05-01")]
  (= (type sql)
     (-> sql nippy/freeze nippy/thaw type)))

For a concrete use case of this causing issues in userland: Currently in the https://github.com/nextjournal/clerk project we use nippy to serialize values to a cache. When we serialize some database queries values that have java.sql.Date values in them and the come back as java.util.Date values, it causes postgres engines to choke on the deserialized queries.

[ptaoussanis]

@philomates Hi Phillip, thanks a lot for bringing this to my attention (and for the PR!).

The PR looks good to me. Is there any urgency from your end to get this in a release? Otherwise would get this merged the next time I'm doing some batched work on Nippy.

[philomates]

Thanks for taking the time to check it out There isn't any urgency as we have ways to work around it by modifying code in user land.

I would like to add a small plug for deps.edn. If nippy used it instead of lein, we'd be able to pull the changes from the git repo and more easily test the changes in our system before a formal release. I tried toying around with adding deps.edn myself but ran into some strange issues that maybe have to do with specifics of how you package/release the library (or maybe weird internal analysis via clerk):

 "Execution error (ExceptionInfo) at clojure.tools.analyzer.passes.jvm.validate/validate-tag (validate.clj:235).\nClass not found: (Class/forName \"[B\")\n",

[ptaoussanis]

There isn't any urgency as we have ways to work around it by modifying code in user land.

👍 Thanks for the confirmation. Feel free to ping if something changes and this becomes more urgent for you.

I would like to add a small plug for deps.edn

I'm not aware off-hand of any reason Nippy wouldn't already work via deps.edn. If it doesn't, could you please open a separate issue for that and share any relevant details?

A minimal reproducible example would be ideal, if possible 🙏

Cheers! :-)

[philomates]

Like within my project's deps.edn I'd like to refer to a specific commit to use from nippy, instead of a release on maven:

{... 
 com.taoensso/nippy
 {#_#_:mvn/version "3.2.0"
  :git/url "https://github.com/philomates/nippy.git"
  :git/sha "932534d3a3fd15178e595ed7cfb9a2f0f412b864"}}

I believe this would require nippy to have a deps.edn file itself (I could be mistaken)

[ptaoussanis]

I've not used deps.edn much, so not too familiar with it. Would propose opening a dedicated issue clearly describing the trouble you're having. That way it'll be easier to see if other users have the same issue, and to give an opportunity for others to comment.

If it's really necessary to maintain a separate deps.edn file, maybe someone can recommend a way to do that automatically (lein/deps plugin?). Or I can look next time I'm on batched Nippy work.

[ptaoussanis]

Merged manually, thanks! 🙏