Fork me on GitHub
Slack Archive

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

2022-07-28

Channels

#honeysql
<
2022-07-28
>
Cora (she/her)21:07:17

I have a fun little issue

Cora (she/her)21:07:32

specifically I have a library that registers a clause before honeysql helpers are loaded

Cora (she/her)21:07:43

and so this assertion fails

Cora (she/her)21:07:53

you can open a repl using clj -Sdeps '{:deps {com.github.seancorfield/honeysql {:mvn/version "2.2.891"}}}'

Cora (she/her)21:07:07

and then use this to trigger the assertion error 

(require '[honey.sql :as sql]
         '[clojure.string :as str])

(sql/register-clause!
 :on
 (fn [_ [table & columns]]
   [(str (sql/sql-kw :on)
         " "
         (sql/format-entity table)
         "("
         (str/join ", "
                   (mapcat sql/format-expr columns))
         ")")])
 nil)

(require '[honey.sql.helpers :as sql.helpers])

Cora (she/her)21:07:11

I'll file an issue

Cora (she/her)21:07:14

and maybe do a PR

Cora (she/her)22:07:37

I ran into this because I have non-helper-generated clauses that I need to merge and wanted to use the helpers to do it

Cora (she/her)22:07:58

it would be super nice to have a merge that would merge each of the clause types between two maps

Cora (she/her)22:07:16

but I'll definitely work something out in my app before considering contributing it back

seancorfield22:07:11

@corasaurus-hex It would probably be better to add a builtin-base-clause-order and use that in the assert and have base-clause-order initialized from that new sequence?

seancorfield03:07:35

Hmm, yeah, I just happened to have that file open right now, looking at another issue. I wonder why I set up the assert on the mutable clause order instead of the base one...?

seancorfield03:07:05

So I think I can just change that one line in the assert and we're all good @corasaurus-hex?

Cora (she/her)03:07:06

I'll bet you started with base-clause-order or current-clause-order and then needed to re-use the same starting structure with the other var, giving you default-clause-order

Cora (she/her)03:07:15

I just pushed up to that branch

Cora (she/her)03:07:19

if you'd rather just merge

seancorfield03:07:44

Heh, I already made the one word change locally but I'll check your PR and take that instead 🙂

Cora (she/her)03:07:16

thanks 😄

seancorfield03:07:41

Updated changelog. There should be a new SNAPSHOT up on Clojars shortly that you can use, or via git deps. I have a few more problems to solve before I release 2.3.x...

seancorfield03:07:21

Much appreciated -- that's a really interesting edge case to have detected. Thank you!

seancorfield22:07:44

(so the builtin list never changes -- and then the assert correctly reflects the internal consistency that is needed there)

Cora (she/her)22:07:01

sure, that would work

Cora (she/her)22:07:03

i'm afk right now but i'll update when i'm home

1