diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index e1f5704dd35b83caed946db40de58e3ddd2ce887..d01330019b89bdfa4cf2466b8c2543b85e1b6ef8 100644 --- a/.clj-kondo/config.edn +++ b/.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 diff --git a/modules/drivers/vertica/src/metabase/driver/vertica.clj b/modules/drivers/vertica/src/metabase/driver/vertica.clj index 442db2a083e493b1eff93a95cac6de0fcbe0e5bb..980861e94554def74523504164dfdb2215bd4afd 100644 --- a/modules/drivers/vertica/src/metabase/driver/vertica.clj +++ b/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`. diff --git a/modules/drivers/vertica/test/metabase/driver/vertica_test.clj b/modules/drivers/vertica/test/metabase/driver/vertica_test.clj index 1a9b7009ebf2837b85fef59e8f4393d6f7f8efd5..7da9472f8edacb6eb5c0d5e7546a14dfbe8b7d7e 100644 --- a/modules/drivers/vertica/test/metabase/driver/vertica_test.clj +++ b/modules/drivers/vertica/test/metabase/driver/vertica_test.clj @@ -1,6 +1,8 @@ (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}}" diff --git a/src/metabase/driver/sql/query_processor.clj b/src/metabase/driver/sql/query_processor.clj index 90601c58817c194ccae2f3dc443e4ee63c1d1755..2b1ce79203a5e04934f10b25db32c76838549827 100644 --- a/src/metabase/driver/sql/query_processor.clj +++ b/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) diff --git a/src/metabase/driver/sql_jdbc/execute.clj b/src/metabase/driver/sql_jdbc/execute.clj index e1c90c892ab3f3fdb84929ee17e4c70d462d1dda..b85aa95e7cfdd1faaa7780e72872401c3765e039 100644 --- a/src/metabase/driver/sql_jdbc/execute.clj +++ b/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]