Fork me on GitHub
#sql
<
2023-02-09
>
Cam Saul18:02:46

@seancorfield another difference I noticed between HoneySQL 1 and Honey SQL 2 is that the format accepts > 2 args for predicates like :/ , while the latter explicitly disallows it...

(honeysql.core/format {:select [[(honeysql.core/call :/ 1 2 3) :x]]})
;; => ["SELECT (1 / 2 / 3) AS x"]

(honey.sql/format {:select [[[:/ 1 2 3] :x]]})
;; => only binary :/ is supported
Would you accept a PR to add this to the "Differences from 1.x" dox? Or should Honey SQL 2 support this? After all, it works in Clojure:
(/ 1 2 3) => 1/6 ; the same as (/ (/ 1 2) 3)

seancorfield18:02:30

I guess there's no real reason to restrict a lot of operators to pure binary. I did it initially because I thought it might catch possible bugs in code and would force people to be more explicit...

seancorfield18:02:50

...I recently made :- variadic I think?

seancorfield18:02:52

There's some special-case code for := and :<> in the binary case https://github.com/seancorfield/honeysql/blob/develop/src/honey/sql.cljc#L1564 but that could be lifted into the variadic case and all operators could be allowed to be variadic, not just these: https://github.com/seancorfield/honeysql/blob/develop/src/honey/sql.cljc#L1282

seancorfield18:02:27

Although, being variadic or not is exposed in the API for register-op!...

Cam Saul18:02:41

I'm happy to open an issue or submit a PR to either add it to the dox or support it RE exposing things in the API: this is a little bit tangential, but I would love a way to be able to check whether a function is registered or not. In some validation code I'm working on I'm basically doing (contains? @@#'sql/special-syntax f) to verify that methods are emitting valid Honey SQL clauses.

(defn- registered-honey-sql-2-clause?
  [x]
  (and (vector? x)
       (let [f (first x)]
         (and (keyword? f)
              ;; this is a bit hacky but there's no other way AFAIK to tell whether a function is registered in Honey
              ;; SQL 2.
              (contains? @@#'sql/special-syntax f)))))
It's icky and if there was an official function to do that I would definitely prefer to use it. Happy to open an issue or a PR for that as well

seancorfield18:02:09

Haha... you crazy Metabase peeps! 🙂 Sure, open an issue to add a way to test for that, and for a registered operator. Also, open an issue about variadic operators -- I'll need to give that one a bit more thought.

Cam Saul18:02:53

opened https://github.com/seancorfield/honeysql/issues/458 and https://github.com/seancorfield/honeysql/issues/459, let me know if I can help with anything else. Happy to submit PRs or add more context to the issues

2
seancorfield00:02:53

For anyone following this thread but not #C66EM8D5H both of these issues were addressed today in the 2.4.979 release.