seancorfield / honeysql

Turn Clojure data structures into SQL
https://cljdoc.org/d/com.github.seancorfield/honeysql/CURRENT
1.77k stars 174 forks source link

sql.helpers load assertion fails if custom clauses are already registered #552

Open SilentSnacker opened 6 hours ago

SilentSnacker commented 6 hours ago

The honey.sql.helpers namespace has the following assertions near the bottom of the file.

#?(:clj
   (do
     ;; #409 this assert is only valid when :doc metadata is not elided:
     (when (-> #'generic-helper-unary meta :doc)
       ;; ensure #295 stays true (all public functions have docstring):
       (assert (empty? (->> (ns-publics *ns*) (vals) (c/filter (comp not :doc meta))))))
     ;; ensure all public functions match clauses:
     (assert (= (c/set (conj @#'honey.sql/default-clause-order
                             :composite :filter :lateral :over :within-group
                             :upsert
                             :generic-helper-variadic :generic-helper-unary))
                (c/set (conj (map keyword (keys (ns-publics *ns*)))
                             :nest :raw))))))

It seems like this will fail if custom clauses have already been registered via the honey.sql/register-clause! function.

It results in a somewhat confusing exception when trying to require honey.sql.helpers in some other ns declaration.

...
Caused by: java.lang.AssertionError: Assert failed: (= (clojure.core/set (conj (clojure.core/deref (clojure.core/deref (var honey.sql/base-clause-order))) :composite :filter :lateral :over :within-group :upsert :generic-helper-variadic :generic-helper-unary)) (clojure.core/set (conj (map keyword (keys (ns-publics *ns*))) :nest :raw)))
        at honey.sql.helpers$eval120529.invokeStatic(helpers.cljc:1031)
        at honey.sql.helpers$eval120529.invoke(helpers.cljc:1031)

It took me a while to figure out what was happening so thought I would create this issue here.

I am not sure what the appropriate fix would be. Maybe one of:

Thank you for this library!

SilentSnacker commented 6 hours ago

This seems like a rare corner case. I worked around it by just manually requiring the helpers ns before registering our custom clause.

seancorfield commented 4 hours ago

Thanks for reporting this. I'll have to have a think about the best way to handle this -- you're probably right that the simplest solution would be to make this an actual test since it really was intended to be a development aid for me...