fix MS SQL table hints in FROM clause (e.g. NOLOCK) #522

Signed-off-by: Sean Corfield <sean@corfield.org>

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Changes
 
 * 2.5.next in progress
+  * Address [#522](https://github.com/seancorfield/honeysql/issues/522) by supporting metadata on table specifications in `:from` and `:join` clauses to provide index hints (SQL Server).
   * Address [#520](https://github.com/seancorfield/honeysql/issues/520) by expanding how `:inline` works, to support a sequence of arguments.
   * Fix [#518](https://github.com/seancorfield/honeysql/issues/518) by moving temporal clause before alias.
   * Address [#495](https://github.com/seancorfield/honeysql/issues/495) by adding (experimental) `format&` and `formatv` macros (`.clj` only!) -- purely for discussion: may be removed in a subsequent release!

doc/clause-reference.md

@@ -828,6 +828,23 @@ user=> (sql/format {:select [:u.username]
 ["SELECT u.username FROM user FOR SYSTEM_TIME FROM '2019-08-01 15:23:00' TO '2019-08-01 15:24:00' AS u WHERE u.id = ?" 9]
 ```
 
+As of 2.5.next, HoneySQL supports metadata on a table expression to provide
+database-specific hints, such as SQL Server's `WITH (..)` clause:
+
+```clojure
+user=> (sql/format {:select [:col]
+                    :from [^:nolock [:table]]
+                    :where [:= :id 9]})
+["SELECT col FROM table WITH (NOLOCK) WHERE id = ?" 9]
+user=> (sql/format {:select [:col]
+                    :from [^:nolock [:table :t]]
+                    :where [:= :id 9]})
+["SELECT col FROM table AS t WITH (NOLOCK) WHERE id = ?" 9]
+```
+
+Since you cannot put metadata on a keyword, the table name must be written as
+a vector even when you have no alias.
+
 > Note: the actual formatting of a `:from` clause is currently identical to the formatting of a `:select` clause.
 
 If you are using inheritance, you can specify `ONLY(table)` as a function
@@ -918,6 +935,25 @@ user=> (sql/format {:select [:t.ref :pp.code]
 ["SELECT t.ref, pp.code FROM transaction AS t LEFT JOIN paypal_tx AS pp USING (id) WHERE ? = pp.status" "settled"]
 ```
 
+As of 2.5.next, HoneySQL supports metadata on a table expression to provide
+database-specific hints, such as SQL Server's `WITH (..)` clause:
+
+```clojure
+user=> (sql/format {:select [:col]
+                    :from [:table]
+                    :join [^:nolock [:extra] [:= :table.extra_id :extra.id]]
+                    :where [:= :id 9]})
+["SELECT col FROM table INNER JOIN extra WITH (NOLOCK) ON table.extra_id = extra.id WHERE id = ?" 9]
+user=> (sql/format {:select [:col]
+                    :from [[:table :t]]
+                    :join [^:nolock [:extra :x] [:= :t.extra_id :x.id]]
+                    :where [:= :id 9]})
+["SELECT col FROM table AS t INNER JOIN extra AS x WITH (NOLOCK) ON t.extra_id = x.id WHERE id = ?" 9]
+```
+
+Since you cannot put metadata on a keyword, the table name must be written as
+a vector even when you have no alias.
+
 If you are using inheritance, you can specify `ONLY(table)` as a function
 call: `[:only :table]`.
 

src/honey/sql.cljc

@@ -548,12 +548,43 @@
   (format-temporal [:for :system-time :between [:inline "2000-12-16"] :and [:inline "2000-12-17"]])
   )
 
+(defn- format-meta
+  "If the expression has metadata, format it as a sequence of keywords,
+   treating `:foo true` as `FOO` and `:foo :bar` as `FOO BAR`.
+   Return nil if there is no metadata."
+  [x & [sep]]
+  (when-let [data (meta x)]
+    (let [items (reduce-kv (fn [acc k v]
+                             (if (true? v)
+                               (conj acc k)
+                               (conj acc k v)))
+                           []
+                           (reduce dissoc
+                                   data
+                                   (into [; remove the somewhat "standard" metadata:
+                                          :line :column :file
+                                          :end-line :end-column]
+                                         *ignored-metadata*)))]
+      (when (seq items)
+        (str/join (str sep " ") (mapv sql-kw items))))))
+
+(comment
+  (format-meta ^{:foo true :bar :baz} [])
+
+  (binding [*ignored-metadata* [:bar]]
+    (format-meta ^{:foo true :bar :baz} []))
+
+  (format-meta [])
+  (format-meta ^:nolock ^:uncommited [] ",")
+  )
+
 (defn- format-item-selection
   "Format all the possible ways to represent a table/column selection."
   [x as]
   (if (bigquery-*-except-replace? x)
     (format-bigquery-*-except-replace x)
-    (let [[selectable alias temporal] (split-alias-temporal x)
+    (let [hints (format-meta x ",")
+          [selectable alias temporal] (split-alias-temporal x)
           _ (when (= ::too-many! temporal)
               (throw (ex-info "illegal syntax in select expression"
                               {:symbol selectable :alias alias :unexpected (nnext x)})))
@@ -569,16 +600,18 @@
                                (format-temporal temporal))]
 
       (-> [(str sql
-                (when sql''
+                (when sql'' ; temporal
                   (str " " sql''))
-                (when sql'
+                (when sql' ; alias
                   (str (if as
                          (if (and (contains? *dialect* :as)
                                   (not (:as *dialect*)))
                            " "
                            " AS ")
                          " ")
-                       sql')))]
+                       sql'))
+                (when hints
+                  (str " WITH (" hints ")")))]
           (into params)
           (into params')
           (into params'')))))
@@ -689,35 +722,6 @@
     (let [[sqls params] (format-expr-list xs {:drop-ns true})]
       (into [(str "(" (str/join ", " sqls) ")")] params))))
 
-(defn- format-meta
-  "If the expression has metadata, format it as a sequence of keywords,
-   treating `:foo true` as `FOO` and `:foo :bar` as `FOO BAR`.
-   Return nil if there is no metadata."
-  [x]
-  (when-let [data (meta x)]
-    (let [items (reduce-kv (fn [acc k v]
-                             (if (true? v)
-                               (conj acc k)
-                               (conj acc k v)))
-                           []
-                           (reduce dissoc
-                                   data
-                                   (into [; remove the somewhat "standard" metadata:
-                                          :line :column :file
-                                          :end-line :end-column]
-                                         *ignored-metadata*)))]
-      (when (seq items)
-        (str/join " " (mapv sql-kw items))))))
-
-(comment
-  (format-meta ^{:foo true :bar :baz} [])
-
-  (binding [*ignored-metadata* [:bar]]
-    (format-meta ^{:foo true :bar :baz} []))
-
-  (format-meta [])
-  )
-
 (defn- format-selects-common [prefix as xs]
   (let [qualifier (format-meta xs)
         prefix    (if prefix

test/honey/sql_test.cljc

@@ -1320,6 +1320,20 @@ ORDER BY id = ? DESC
     (is (= ["SELECT ['a', 'b', 'c'] AS x"]
            (sut/format '{select [[[inline [lift ["a" "b" "c"]]] x]]})))))
 
+(deftest issue-522
+  (testing "from with metadata"
+    (is (= ["SELECT * FROM table WITH (HINT)"]
+           (sut/format {:select [:*] :from [^:hint [:table]]})))
+    ;; hash map (metadata) is unordered:
+    (is (or (= ["SELECT * FROM table WITH (ABC, DEF)"]
+               (sut/format {:select [:*] :from [^:abc ^:def [:table]]}))
+            (= ["SELECT * FROM table WITH (DEF, ABC)"]
+               (sut/format {:select [:*] :from [^:abc ^:def [:table]]}))))
+    (is (or (= ["SELECT * FROM table WITH (ABC, DEF)"]
+               (sut/format {:select [:*] :from [^{:abc true :def true} [:table]]}))
+            (= ["SELECT * FROM table WITH (DEF, ABC)"]
+               (sut/format {:select [:*] :from [^{:abc true :def true} [:table]]}))))))
+
 (comment
   ;; partial (incorrect!) workaround for #407:
   (sut/format {:select :f.* :from [[:foo [:f :for :system-time]]] :where [:= :f.id 1]})