Port `:vertica` driver to Honey SQL 2 (#28296)

* Port `:vertica` driver to Honey SQL 2

* Fix 

* One last test fix 

.clj-kondo/config.edn

@@ -704,14 +704,13 @@
   ;;           "snowflake"
   ;;           "sparksql"
   ;;           "sqlite"
-  ;;           "sqlserver"
-  ;;           "vertica"]
+  ;;           "sqlserver"]
   ;;          [:? "-test"]
   ;;          [:or #"\." #"$"]]]
   ;;   ".*"))
   ;;
   ;; Please keep this form updated when you change the generated regex! <3
-  {:pattern "^metabase\\.(?!util\\.(?:(?:honeysql-extensions)|(?:honey-sql-1)))(?!query-processor-test)(?!(?:(?:driver)|(?:test\\.data))\\.(?:(?:sql(?:-jdbc)?)|(?:(?:sql(?:-jdbc)?))|(?:athena)|(?:bigquery-cloud-sdk)|(?:h2)|(?:hive-like)|(?:oracle)|(?:presto(?:(?:(?:-common)|(?:-jdbc)))?)|(?:snowflake)|(?:sparksql)|(?:sqlite)|(?:sqlserver)|(?:vertica))(?:-test)?(?:(?:\\.)|(?:$))).*"
+  {:pattern "^metabase\\.(?!util\\.(?:(?:honeysql-extensions)|(?:honey-sql-1)))(?!query-processor-test)(?!(?:(?:driver)|(?:test\\.data))\\.(?:(?:sql(?:-jdbc)?)|(?:(?:sql(?:-jdbc)?))|(?:athena)|(?:bigquery-cloud-sdk)|(?:h2)|(?:hive-like)|(?:oracle)|(?:presto(?:(?:(?:-common)|(?:-jdbc)))?)|(?:snowflake)|(?:sparksql)|(?:sqlite)|(?:sqlserver))(?:-test)?(?:(?:\\.)|(?:$))).*"
    :name    honey-sql-2-namespaces}]
 
  :config-in-ns

modules/drivers/vertica/src/metabase/driver/vertica.clj

@@ -2,7 +2,7 @@
   (:require
    [clojure.java.jdbc :as jdbc]
    [clojure.set :as set]
-   [honeysql.format :as hformat]
+   [honey.sql :as sql]
    [metabase.driver :as driver]
    [metabase.driver.common :as driver.common]
    [metabase.driver.sql-jdbc.common :as sql-jdbc.common]
@@ -17,7 +17,7 @@
    [metabase.query-processor.timezone :as qp.timezone]
    [metabase.util :as u]
    [metabase.util.date-2 :as u.date]
-   [metabase.util.honeysql-extensions :as hx]
+   [metabase.util.honey-sql-2 :as h2x]
    [metabase.util.i18n :refer [trs]]
    [metabase.util.log :as log])
   (:import
@@ -79,13 +79,17 @@
              (dissoc details :host :port :dbname :db :ssl))
       (sql-jdbc.common/handle-additional-options details)))
 
+(defmethod sql.qp/honey-sql-version :vertica
+  [_driver]
+  2)
+
 (defmethod sql.qp/current-datetime-honeysql-form :vertica
-  [_]
-  (hx/with-database-type-info (hx/call :current_timestamp 6) :TimestampTz))
+  [_driver]
+  (h2x/with-database-type-info [:current_timestamp [:inline 6]] "timestamptz"))
 
 (defmethod sql.qp/unix-timestamp->honeysql [:vertica :seconds]
-  [_ _ expr]
-  (hx/call :to_timestamp expr))
+  [_driver _seconds-or-milliseconds honeysql-expr]
+  (h2x/with-database-type-info [:to_timestamp honeysql-expr] "timestamp"))
 
 ;; TODO - not sure if needed or not
 (defn- cast-timestamp
@@ -94,86 +98,92 @@
   no-op."
   [expr]
   (if (instance? java.time.temporal.Temporal expr)
-    (hx/cast :timestamp expr)
+    (h2x/cast :timestamp expr)
     expr))
 
-(defn- date-trunc [unit expr] (hx/call :date_trunc (hx/literal unit) (cast-timestamp expr)))
-(defn- extract    [unit expr] (hx/call :extract    unit              (cast-timestamp expr)))
-(defn- datediff   [unit a b]  (hx/call :datediff   (hx/literal unit) (cast-timestamp a) (cast-timestamp b)))
-
-(def ^:private extract-integer (comp hx/->integer extract))
-
-(defmethod sql.qp/date [:vertica :default]         [_ _ expr] expr)
-(defmethod sql.qp/date [:vertica :minute]          [_ _ expr] (date-trunc :minute expr))
-(defmethod sql.qp/date [:vertica :minute-of-hour]  [_ _ expr] (extract-integer :minute expr))
-(defmethod sql.qp/date [:vertica :hour]            [_ _ expr] (date-trunc :hour expr))
-(defmethod sql.qp/date [:vertica :hour-of-day]     [_ _ expr] (extract-integer :hour expr))
-(defmethod sql.qp/date [:vertica :day]             [_ _ expr] (hx/->date expr))
-(defmethod sql.qp/date [:vertica :day-of-month]    [_ _ expr] (extract-integer :day expr))
-(defmethod sql.qp/date [:vertica :day-of-year]     [_ _ expr] (extract-integer :doy expr))
-(defmethod sql.qp/date [:vertica :month]           [_ _ expr] (date-trunc :month expr))
-(defmethod sql.qp/date [:vertica :month-of-year]   [_ _ expr] (extract-integer :month expr))
-(defmethod sql.qp/date [:vertica :quarter]         [_ _ expr] (date-trunc :quarter expr))
-(defmethod sql.qp/date [:vertica :quarter-of-year] [_ _ expr] (extract-integer :quarter expr))
-(defmethod sql.qp/date [:vertica :year]            [_ _ expr] (date-trunc :year expr))
-(defmethod sql.qp/date [:vertica :year-of-era]     [_ _ expr] (extract-integer :year expr))
+(defn- date-trunc [unit expr] [:date_trunc   (h2x/literal unit) (cast-timestamp expr)])
+(defn- extract    [unit expr] [::h2x/extract unit               (cast-timestamp expr)])
+(defn- datediff   [unit a b]  [:datediff     (h2x/literal unit) (cast-timestamp a) (cast-timestamp b)])
+
+(def ^:private extract-integer (comp h2x/->integer extract))
+
+(defmethod sql.qp/date [:vertica :default]         [_driver _unit expr] expr)
+(defmethod sql.qp/date [:vertica :minute]          [_driver _unit expr] (date-trunc :minute expr))
+(defmethod sql.qp/date [:vertica :minute-of-hour]  [_driver _unit expr] (extract-integer :minute expr))
+(defmethod sql.qp/date [:vertica :hour]            [_driver _unit expr] (date-trunc :hour expr))
+(defmethod sql.qp/date [:vertica :hour-of-day]     [_driver _unit expr] (extract-integer :hour expr))
+(defmethod sql.qp/date [:vertica :day]             [_driver _unit expr] (h2x/->date expr))
+(defmethod sql.qp/date [:vertica :day-of-month]    [_driver _unit expr] (extract-integer :day expr))
+(defmethod sql.qp/date [:vertica :day-of-year]     [_driver _unit expr] (extract-integer :doy expr))
+(defmethod sql.qp/date [:vertica :month]           [_driver _unit expr] (date-trunc :month expr))
+(defmethod sql.qp/date [:vertica :month-of-year]   [_driver _unit expr] (extract-integer :month expr))
+(defmethod sql.qp/date [:vertica :quarter]         [_driver _unit expr] (date-trunc :quarter expr))
+(defmethod sql.qp/date [:vertica :quarter-of-year] [_driver _unit expr] (extract-integer :quarter expr))
+(defmethod sql.qp/date [:vertica :year]            [_driver _unit expr] (date-trunc :year expr))
+(defmethod sql.qp/date [:vertica :year-of-era]     [_driver _unit expr] (extract-integer :year expr))
 
 (defmethod sql.qp/date [:vertica :week]
-  [_ _ expr]
+  [_driver _unit expr]
   (sql.qp/adjust-start-of-week :vertica (partial date-trunc :week) (cast-timestamp expr)))
 
-(defmethod sql.qp/date [:vertica :week-of-year-iso] [_driver _ expr] (hx/call :week_iso expr))
+(defmethod sql.qp/date [:vertica :week-of-year-iso]
+  [_driver _unit expr]
+  [:week_iso expr])
 
 (defmethod sql.qp/date [:vertica :day-of-week]
-  [_ _ expr]
-  (sql.qp/adjust-day-of-week :vertica (hx/call :dayofweek_iso expr)))
+  [_driver _unit expr]
+  (sql.qp/adjust-day-of-week :vertica [:dayofweek_iso expr]))
 
 (defmethod sql.qp/->honeysql [:vertica :convert-timezone]
   [driver [_ arg target-timezone source-timezone]]
   (let [expr         (cast-timestamp (sql.qp/->honeysql driver arg))
-        timestamptz? (hx/is-of-type? expr "timestamptz")]
+        timestamptz? (h2x/is-of-type? expr "timestamptz")]
     (sql.u/validate-convert-timezone-args timestamptz? target-timezone source-timezone)
     (-> (if timestamptz?
           expr
-          (hx/at-time-zone expr (or source-timezone (qp.timezone/results-timezone-id))))
-        (hx/at-time-zone target-timezone)
-        (hx/with-database-type-info "timestamp"))))
+          (h2x/at-time-zone expr (or source-timezone (qp.timezone/results-timezone-id))))
+        (h2x/at-time-zone target-timezone)
+        (h2x/with-database-type-info "timestamp"))))
 
 (defmethod sql.qp/->honeysql [:vertica :concat]
   [driver [_ & args]]
-  (->> args
-       (map (partial sql.qp/->honeysql driver))
-       (reduce (partial hx/call :concat))))
+  (transduce
+   (map #(sql.qp/->honeysql driver %))
+   (completing (fn [x y]
+                 (if (some? x)
+                   [:concat x y]
+                   y)))
+   nil
+   args))
 
 (defmethod sql.qp/datetime-diff [:vertica :year]
   [driver _unit x y]
   (let [months (sql.qp/datetime-diff driver :month x y)]
-    (hx/->integer (hx/call :trunc (hx// months 12)))))
+    (h2x/->integer [:trunc (h2x// months 12)])))
 
 (defmethod sql.qp/datetime-diff [:vertica :quarter]
   [driver _unit x y]
   (let [months (sql.qp/datetime-diff driver :month x y)]
-    (hx/->integer (hx/call :trunc (hx// months 3)))))
+    (h2x/->integer [:trunc (h2x// months 3)])))
 
 (defmethod sql.qp/datetime-diff [:vertica :month]
   [_driver _unit x y]
-  (hx/+ (datediff :month x y)
-        ;; datediff counts month boundaries not whole months, so we need to adjust
-        ;; if x<y but x>y in the month calendar then subtract one month
-        ;; if x>y but x<y in the month calendar then add one month
-        (hx/call
-         :case
-         (hx/call :and
-                  (hx/call :< (cast-timestamp x) (cast-timestamp y))
-                  (hx/call :> (extract :day x) (extract :day y))) -1
-         (hx/call :and
-                  (hx/call :> (cast-timestamp x) (cast-timestamp y))
-                  (hx/call :< (extract :day x) (extract :day y))) 1
-         :else 0)))
+  (h2x/+ (datediff :month x y)
+         ;; datediff counts month boundaries not whole months, so we need to adjust
+         ;; if x<y but x>y in the month calendar then subtract one month
+         ;; if x>y but x<y in the month calendar then add one month
+         [:case
+          [:and
+           [:< (cast-timestamp x) (cast-timestamp y)]
+           [:> (extract :day x) (extract :day y)]] -1
+          [:and
+           [:> (cast-timestamp x) (cast-timestamp y)]
+           [:< (extract :day x) (extract :day y)]] 1
+          :else [:inline 0]]))
 
 (defmethod sql.qp/datetime-diff [:vertica :week]
   [_driver _unit x y]
-  (hx/->integer (hx/call :trunc (hx// (datediff :day x y) 7))))
+  (h2x/->integer [:trunc (h2x// (datediff :day x y) 7)]))
 
 (defmethod sql.qp/datetime-diff [:vertica :day]
   [_driver _unit x y]
@@ -181,35 +191,49 @@
 
 (defmethod sql.qp/datetime-diff [:vertica :hour]
   [_driver _unit x y]
-  (let [seconds (hx/- (extract :epoch y) (extract :epoch x))]
-    (hx/->integer (hx/call :trunc (hx// seconds 3600)))))
+  (let [seconds (h2x/- (extract :epoch y) (extract :epoch x))]
+    (h2x/->integer [:trunc (h2x// seconds 3600)])))
 
 (defmethod sql.qp/datetime-diff [:vertica :minute]
   [_driver _unit x y]
-  (let [seconds (hx/- (extract :epoch y) (extract :epoch x))]
-    (hx/->integer (hx/call :trunc (hx// seconds 60)))))
+  (let [seconds (h2x/- (extract :epoch y) (extract :epoch x))]
+    (h2x/->integer [:trunc (h2x// seconds 60)])))
 
 (defmethod sql.qp/datetime-diff [:vertica :second]
   [_driver _unit x y]
-  (hx/->integer (hx/call :trunc (hx/- (extract :epoch y) (extract :epoch x)))))
+  (h2x/->integer [:trunc (h2x/- (extract :epoch y) (extract :epoch x))]))
 
 (defmethod sql.qp/->honeysql [:vertica :regex-match-first]
   [driver [_ arg pattern]]
-  (hx/call :regexp_substr (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver pattern)))
+  [:regexp_substr (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver pattern)])
+
+(defn- format-percentile
+  [_fn [arg p]]
+  (let [[arg-sql & arg-args] (sql/format-expr arg {:nested true})
+        p                    (if (number? p)
+                               [:inline p]
+                               p)
+        [p-sql & p-args]     (sql/format-expr p {:nested true})]
+    (into [(format "APPROXIMATE_PERCENTILE(%s USING PARAMETERS percentile=%s)"
+                   arg-sql
+                   p-sql)]
+          cat
+          [arg-args p-args])))
+
+(sql/register-fn! ::percentile #'format-percentile)
 
 (defmethod sql.qp/->honeysql [:vertica :percentile]
   [driver [_ arg p]]
-  (hx/raw (format "APPROXIMATE_PERCENTILE(%s USING PARAMETERS percentile=%s)"
-                  (hformat/to-sql (sql.qp/->honeysql driver arg))
-                  (hformat/to-sql (sql.qp/->honeysql driver p)))))
+  (let [arg (sql.qp/->honeysql driver arg)
+        p   (sql.qp/->honeysql driver p)]
+    [::percentile arg p]))
 
 (defmethod sql.qp/->honeysql [:vertica :median]
   [driver [_ arg]]
-  (hx/call :approximate_median (sql.qp/->honeysql driver arg)))
+  [:approximate_median (sql.qp/->honeysql driver arg)])
 
 (defmethod sql.qp/add-interval-honeysql-form :vertica
   [_ hsql-form amount unit]
-  (hx/call :timestampadd unit)
   ;; using `timestampadd` instead of `+ (INTERVAL)` because vertica add inteval for month, or year
   ;; by adding the equivalent number of days, not adding the unit compoinent.
   ;; For example `select date '2004-02-02' + interval '1 year' will return `2005-02-01` because it's adding
@@ -218,8 +242,8 @@
   (let [acceptable-types (case unit
                            (:millisecond :second :minute :hour) #{"time" "timetz" "timestamp" "timestamptz"}
                            (:day :week :month :quarter :year)   #{"date" "timestamp" "timestamptz"})
-        hsql-form        (hx/cast-unless-type-in "timestamp" acceptable-types hsql-form)]
-    (hx/call :timestampadd unit amount hsql-form)))
+        hsql-form        (h2x/cast-unless-type-in "timestamp" acceptable-types hsql-form)]
+    [:timestampadd unit (sql.qp/inline-num amount) hsql-form]))
 
 (defn- materialized-views
   "Fetch the Materialized Views for a Vertica `database`.

modules/drivers/vertica/test/metabase/driver/vertica_test.clj

 (ns metabase.driver.vertica-test
   (:require
+   [clojure.string :as str]
    [clojure.test :refer :all]
+   [metabase.db.query :as mdb.query]
    [metabase.driver :as driver]
    [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
    [metabase.query-processor :as qp]
@@ -23,15 +25,35 @@
                                                              :db                 "birds-near-me"
                                                              :additional-options "ConnectionLoadBalance=1"})))))
 
-(deftest dots-in-column-names-test
+(defn- compile-query [query]
+  (-> (qp/compile query)
+      (update :query #(str/split-lines (mdb.query/format-sql % :vertica)))))
+
+(deftest ^:parallel percentile-test
+  (mt/test-driver :vertica
+    (is (= {:query  ["SELECT"
+                     "  APPROXIMATE_PERCENTILE("
+                     "    \"public\".\"test_data_venues\".\"id\" USING PARAMETERS percentile = 1"
+                     "  ) AS \"percentile\""
+                     "FROM"
+                     "  \"public\".\"test_data_venues\""]
+            :params nil}
+           (compile-query
+            (mt/mbql-query venues
+              {:aggregation [[:percentile $id 1]]}))))))
+
+(deftest ^:parallel dots-in-column-names-test
   (mt/test-driver :vertica
     (testing "Columns with dots in the name should be properly quoted (#13932)"
       (mt/dataset dots-in-names
-        (is (= {:query  (str "SELECT * "
-                             "FROM table "
-                             "WHERE \"public\".\"dots_in_names_objects.stuff\".\"dotted.name\" = ?")
+        (is (= {:query  ["SELECT"
+                         "  *"
+                         "FROM"
+                         "  table"
+                         "WHERE"
+                         "  \"public\".\"dots_in_names_objects.stuff\".\"dotted.name\" = ?"]
                 :params ["ouija_board"]}
-               (qp/compile
+               (compile-query
                 {:database   (mt/id)
                  :type       :native
                  :native     {:query         "SELECT * FROM table WHERE {{x}}"

src/metabase/driver/sql/query_processor.clj

@@ -182,7 +182,7 @@
 
 (defn- format-compiled
   [_compiled [honeysql-expr]]
-  (sql/format-expr honeysql-expr {:inline true}))
+  (sql/format-expr honeysql-expr {:nested true}))
 
 (sql/register-fn! ::compiled #'format-compiled)
 
@@ -333,28 +333,29 @@
   This assumes `day-of-week` as returned by the driver is already between `1` and `7` (adjust it if it's not). It
   adjusts as needed to match `start-of-week` by the [[driver.common/start-of-week-offset]], which comes
   from [[driver/db-start-of-week]]."
-  ([driver day-of-week]
-   (adjust-day-of-week driver day-of-week (driver.common/start-of-week-offset driver)))
+  ([driver day-of-week-honeysql-expr]
+   (adjust-day-of-week driver day-of-week-honeysql-expr (driver.common/start-of-week-offset driver)))
 
-  ([driver day-of-week offset]
-   (adjust-day-of-week driver day-of-week offset hx/mod))
+  ([driver day-of-week-honeysql-expr offset]
+   (adjust-day-of-week driver day-of-week-honeysql-expr offset hx/mod))
 
   ([driver
-    day-of-week
+    day-of-week-honeysql-expr
     offset :- s/Int
     mod-fn :- (s/pred fn?)]
    (cond
-     (zero? offset) day-of-week
-     (neg? offset)  (recur driver day-of-week (+ offset 7) mod-fn)
-     :else          (hx/call :case
-                             (hx/call :=
-                                      (mod-fn (hx/+ day-of-week offset) (inline-num 7))
-                                      (inline-num 0))
-                             (inline-num 7)
-                             :else
-                             (mod-fn
-                              (hx/+ day-of-week offset)
-                              (inline-num 7))))))
+     (inline? offset) (recur driver day-of-week-honeysql-expr (second offset) mod-fn)
+     (zero? offset)   day-of-week-honeysql-expr
+     (neg? offset)    (recur driver day-of-week-honeysql-expr (+ offset 7) mod-fn)
+     :else            (hx/call :case
+                               (hx/call :=
+                                        (mod-fn (hx/+ day-of-week-honeysql-expr offset) (inline-num 7))
+                                        (inline-num 0))
+                               (inline-num 7)
+                               :else
+                               (mod-fn
+                                (hx/+ day-of-week-honeysql-expr offset)
+                                (inline-num 7))))))
 
 (defmulti quote-style
   "Return the quoting style that should be used by [HoneySQL](https://github.com/jkk/honeysql) when building a SQL
@@ -374,13 +375,13 @@
   `:milliseconds`.
 
   There is a default implementation for `:milliseconds` the recursively calls with `:seconds` and `(expr / 1000)`."
-  {:arglists '([driver seconds-or-milliseconds expr]), :added "0.35.0"}
+  {:arglists '([driver seconds-or-milliseconds honeysql-expr]), :added "0.35.0"}
   (fn [driver seconds-or-milliseconds _] [(driver/dispatch-on-initialized-driver driver) seconds-or-milliseconds])
   :hierarchy #'driver/hierarchy)
 
 (defmulti cast-temporal-string
   "Cast a string representing "
-  {:arglists '([driver coercion-strategy expr]), :added "0.38.0"}
+  {:arglists '([driver coercion-strategy honeysql-expr]), :added "0.38.0"}
   (fn [driver coercion-strategy _] [(driver/dispatch-on-initialized-driver driver) coercion-strategy])
   :hierarchy #'driver/hierarchy)
 

src/metabase/driver/sql_jdbc/execute.clj

@@ -348,7 +348,16 @@
 
 (defn- prepared-statement*
   ^PreparedStatement [driver conn sql params canceled-chan]
-  (doto (prepared-statement driver conn sql params)
+  ;; sometimes preparing the statement fails, usually if the SQL syntax is invalid.
+  (doto (try
+          (prepared-statement driver conn sql params)
+          (catch Throwable e
+            (throw (ex-info (tru "Error preparing statement: {0}" (ex-message e))
+                            {:driver driver
+                             :type   qp.error-type/driver
+                             :sql    (str/split-lines (mdb.query/format-sql sql driver))
+                             :params params}
+                            e))))
     (wire-up-canceled-chan-to-cancel-Statement! canceled-chan)))
 
 (defn- use-statement? [driver params]