Register special-syntax function

[k3nj1g]

Hi! I'm registering trim function with special syntax. My implementation now is:

(hsql/register-fn! :trim
                   (fn [_ [text position character]]
                     (let [[sql-x & params-x] (hsql/format-expr [:inline text] {:nested true})
                           [sql-a & params-a] (hsql/format-expr position {:nested true})
                           [sql-b & params-b] (hsql/format-expr [:inline character] {:nested true})
                           position           (str/upper-case (name sql-a))]
                       (cond
                         character
                         (-> [(format "TRIM(%s %s FROM %s)" position sql-b sql-x)]
                             (into params-a)
                             (into params-b)
                             (into params-x))

                         (not (= "NULL" position))
                         (-> [(format "TRIM(%s FROM %s)" position sql-x)]
                             (into params-a)
                             (into params-x))

                         :else
                         (-> [(format "TRIM(%s)" sql-x)])))))

(hsql/format-expr [:trim "text"])

I don't feel comfortable with that i have to check second argument on NULL and third argument comes as nil.

Is there another approach?

[seancorfield]

I would expect the original position to come in as nil if it is omitted -- as character does -- but you are shadowing that with your position based on sql-a. That doesn't look like a good way to handle that. I would also recommend against using :inline here since you're opening yourself up the SQL injection attacks.

I'd probably write it like this:

(hsql/register-fn! :trim
                   (fn [_ [text position character]]
                     (let [[sql-t & params-t] (hsql/format-expr text {:nested true})
                           [sql-p & params-p] (when position (hsql/format-expr position {:nested true}))
                           [sql-c & params-c] (when character (hsql/format-expr [:inline character] {:nested true}))]
                       (cond
                         character
                         (-> [(format "TRIM(%s %s FROM %s)" sql-p sql-c sql-t)]
                             (into params-p)
                             (into params-c)
                             (into params-t))

                         position
                         (-> [(format "TRIM(%s FROM %s)" sql-p sql-t)]
                             (into params-p)
                             (into params-t))

                         :else
                         (-> [(format "TRIM(%s)" sql-t)])))))

If position and/or character are nil, don't bother generating SQL for them.