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

Support numbered parameters #405

Closed
seancorfield opened this issue Apr 6, 2022 · 8 comments · Fixed by #446
Closed

Support numbered parameters #405

seancorfield opened this issue Apr 6, 2022 · 8 comments · Fixed by #446
Assignees
Labels
enhancement needs analysis I need to think about this!

Comments

@seancorfield
Copy link
Owner

The PostgreSQL node client doesn't support ? only $N parameters:

https://node-postgres.com/features/queries

@stevenproctor
Copy link

Happy to help with this as needed

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

@seancorfield Hi

I created a POC #444 for discussion. The changes invoked is a bit big and will break the extension api.
May I know what is the initial thoughts of how to implement this on your mind?

@seancorfield
Copy link
Owner Author

I have a design in my head for a solution to this that breaks nothing but there are a couple of edge cases that require a bit more hammock time.

@seancorfield
Copy link
Owner Author

The approach I plan to take with this is as follows:

  • add an option to use numbered parameters (instead of positional)
  • bind *numbered* (atom []) in format to store parameter values
  • in the code that currently puts ? into the SQL, put $n -- where n is (count (swap! *numbered* conj <param>)) and have (->numbered n) for the actual parameter in the generated vector
  • ->numbered will work like ->param but with the index (instead of a keyword) and based on *numbered* (instead of *params*)
  • the existing code that derefs parameters in the vector (in format) will automatically work for this

This shouldn't break any extensions that are already using the low-level format-* API functions and it should also work with caching (with the same restrictions I think).

@jaidetree
Copy link

That sounds like an ideal solution! Good thinking. I was able to make a function that works for now, though I don't know what-I-don't-know here but may be helpful until that gets implemented:

(defn index-params
  ^:private
  [params]
  (let [unique-vals (vec (set params))
        param-index (->> unique-vals
                        (map-indexed #(vector %2 (inc %1)))
                        (into {}))]
    [unique-vals
     (->> params
          (map-indexed 
            (fn [idx value]
              [idx (str (get param-index value))]))
          (into {}))]))

(defn parse-honeysql-query
  ^:private
  "
  The hash-map shape this function takes is defined by the reducer
  in honeysql->pg
  "
  [{:keys [sql in-str escaping params-index index] :as state} char]
  (cond 
    (and (not in-str) (= char "?"))
    (assoc state
           :index (inc index)
           :sql (str sql "$" (get params-index index)))

    (and (not in-str) (or (= char "\"") (= char "'")))
    (assoc state 
           :in-str true
           :sql (str sql char))

    (and in-str (= char "\\"))
    (assoc state 
           :in-str false
           :escaping true
           :sql (str sql char))

    (and in-str (not escaping) (or (= char "\"") (= char "'")))
    (assoc state 
           :in-str false
           :sql (str sql char))

    escaping
    (assoc state
           :escaping false
           :sql (str sql char))

    :else
    (assoc state
           :sql (str sql char))))

(defn pg-vector
  ^:private
  [{:keys [sql params]}]
  [sql params])

(defn honeysql->pg
  [[sql-query & params]]
  (let [[params params-index] (index-params params)]
    (->> (s/split sql-query #"")
         (reduce
           parse-honeysql-query       
           {:index 0
            :sql ""
            :params params
            :params-index params-index
            :in-str false
            :escaping false})
         (pg-vector))))

Which can be composed to use with node's pg like:

(defn query
 [query-map]
 (let [honeysql-vec (sql/format query-map)
       [sql-query params] (honeysql->pg honeysql-vec)]
   (p/-> (.query pool sql-query (clj->js params))
         (js->clj :keywordize-keys true))))

@seancorfield
Copy link
Owner Author

Oh man, that's just horrific and hilarious 🤣 I'm glad you have something that works -- this issue is next on my list of fun stuff to tackle as soon as I carve out some open source time this month 😸

@seancorfield
Copy link
Owner Author

The basic code changes are up at SHA c62f5da for anyone who wants to try it.

Specify an option of :numbered true to format to get numbered parameters.

I'll work on examples and documentation over the weekend and cut a new release with this feature early next week.

@jaidetree
Copy link

Thanks a ton Sean!

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

Successfully merging a pull request may close this issue.

4 participants