Skip to content
New issue

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

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

Already on GitHub? # 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).

# for free to join this conversation on GitHub. Already have an account? # 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