Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate SQL with arrays / support (document?) inline for *format-expr* #403

Closed
ieugen opened this issue Apr 1, 2022 · 8 comments
Closed
Assignees
Labels
documentation I need to write something up! needs analysis I need to think about this!

Comments

@ieugen
Copy link

ieugen commented Apr 1, 2022

Originally asked on CLojurians slack https://clojurians-log.clojureverse.org/honeysql/2022-04-01 .

Is there an inline version of (sql/format-expr [:array (range 5)]) ?
Or another way to generate an inline SQL for update my_table set tags = ARRAY['tag1,'tag2'] where .... ?

Context:

I'm trying to generate an SQL file for lots of data that I can then bulk import as a transaction.
The file should update a column that is of type PostgreSQL text array

(let [tags ["1" "2" "3" "99"]
        id 5]
    (sql/format {:update [:my_table]
                 :set {:tags :?tags}
                 :where [:= :id :?id]}
                {:inline true
                 :params {:id id :tags tags}}))

gives me

["UPDATE my_table SET tags = ['1', '2', '3', '99'] WHERE id = 5"]

Which is not the good syntax.

I found a way to handle this but it's not straight forward / documented ?!

(binding [sql/*inline* true]
    (sql/format-expr [:array ["1" "2" "3" "99"]]))

=>

["ARRAY['1', '2', '3', '99']"]

The end result I have looks like this:
( I get the first value from format-expr with inline) )

IMO sql/format should allow for a simpler way of supporting :array syntax

(defn tags-update
  [uri source tags]
  (let [tags-array (first (binding [sql/*inline* true]
                            (sql/format-expr [:array tags])))]
    (sql/format {:update [:my_table]
                 :set {:tags [:raw tags-array]}
                 :where [:and
                         [:= :uri :?uri]
                         [:= :source :?source]]}
                {:inline true
                 :params {:uri uri
                          :source source}})))
@ieugen
Copy link
Author

ieugen commented Apr 1, 2022

I got a reply from Sean with a solution that works.

dev=> (let [tags ["1" "2" "3" "99"]
 #_=>         id 5]
 #_=>     (sql/format {:update [:my_table]
 #_=>                  :set {:tags [:array tags]}
 #_=>                  :where [:= :id :?id]
 #_=>                  :returning :*}
 #_=>                 {:inline true
 #_=>                  :params {:id id}}))
["UPDATE my_table SET tags = ARRAY['1', '2', '3', '99'] WHERE id = 5 RETURNING *"]

That was not obvious to me.
I tried initially with params but :array :?tags ( :array with params) does not work.

:set {:tags [:array :?tags]}

and got

; Execution error (IllegalArgumentException) at honey.sql/format-expr-list (sql.cljc:397).
; Don't know how to create ISeq from: clojure.lang.Keyword

@seancorfield
Copy link
Owner

Given that [:array ..] introduces an array literal, the named parameter approach can only be made to work if it forces inline rendering of the parameter value -- which wouldn't match the semantics of named parameters anywhere else in HoneySQL.

I think what I will do here is clearly document for the :array syntax, that you must provide a sequence of actual values -- not a named parameter -- but I will also enhance format-expr-list to provide a better error message here.

@seancorfield seancorfield self-assigned this Apr 6, 2022
@seancorfield seancorfield added the documentation I need to write something up! label Apr 6, 2022
@seancorfield
Copy link
Owner

It looks like there is a precedent: format-in performs an auto-unwrap of a named parameter to produce a list of placeholders. However, that also breaks caching and introducing the same auto-unwrap here would also break caching. I need to give this some thought.

@seancorfield seancorfield added the needs analysis I need to think about this! label Apr 6, 2022
@seancorfield
Copy link
Owner

Using :in with a named parameter (via :params) throws an exception if you try to use caching. It's fine if you are not using caching. I'm inclined to take the same approach here: allow named a parameter as the argument to :array and unwrap it if you are not attempting to cache the statement, just as :in allows.

@ieugen
Copy link
Author

ieugen commented Jul 1, 2022

So not sure what happened, (I believe i updated to latest honeysql),
I had to update my code to convert the set to a vector:

(sql/format {:update [:articles]
               ;; https://github.com/seancorfield/honeysql/issues/403
               :set {:tags [:array (vec tags)]}
               :where [:and
                       [:= :uri :?uri]
                       [:= :source :?source]]}
              {:inline true
               :params {:uri uri
                        :source source}})

Otherwise I would get this error:

; Execution error (ExceptionInfo) at honey.sql/format-expr-list (sql.cljc:418).
; format-expr-list expects a sequence of expressions, found: class clojure.lang.PersistentHashSet

honey.sql/format-expr-list (sql.cljc:418)
honey.sql/format-expr-list (sql.cljc:403)
honey.sql/fn (sql.cljc:1230)
honey.sql/fn (sql.cljc:1229)
honey.sql/format-expr (sql.cljc:1383)
honey.sql/format-expr (sql.cljc:1329)
honey.sql/format-set-exprs (sql.cljc:731)
clojure.lang.PersistentArrayMap/kvreduce (PersistentArrayMap.java:429)
clojure.core/fn (core.clj:6908)
clojure.core/fn (core.clj:6888)
clojure.core.protocols/fn (protocols.clj:175)
clojure.core/reduce-kv (core.clj:6919)
clojure.core/reduce-kv (core.clj:6910)
honey.sql/format-set-exprs (sql.cljc:730)
honey.sql/format-set-exprs (sql.cljc:728)
clojure.lang.Var/invoke (Var.java:388)
honey.sql/format-dsl (sql.cljc:1052)
clojure.lang.PersistentVector/reduce (PersistentVector.java:343)
clojure.core/reduce (core.clj:6885)
clojure.core/reduce (core.clj:6868)
honey.sql/format-dsl (sql.cljc:1046)
honey.sql/format-dsl (sql.cljc:1036)
honey.sql/format (sql.cljc:1472)
honey.sql/format (sql.cljc:1431)

@seancorfield
Copy link
Owner

A set is not sequential - it has no defined order. HoneySQL is pretty consistent about requiring sequential values everywhere.

@ieugen
Copy link
Author

ieugen commented Jul 1, 2022

Thanks, in the case above we are abusing the Array data type in postgres to hold a set.
Ordering is not important.
It surprised me in the sense that it was a change that I noticed.

@seancorfield
Copy link
Owner

After thinking about this on and off for a few months, I've decided that it isn't worth the extra complexity to make :array also dereference named :params like :in because: a) it adds more exceptions to caching b) it adds more code to HoneySQL 😄 c) it adds more special cases to document and d) I'm not sure that ARRAY[..] syntax in PostgreSQL is used widely enough to warrant this addition (compared to :in -- and since there is a workaround available).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation I need to write something up! needs analysis I need to think about this!
Projects
None yet
Development

No branches or pull requests

2 participants