Sqlite honey sql 2 (#28389)

* Convert sqlite driver to honey-sql-2

* Fix bad case statement

* We can't run native queries with AS for oracle

* Dont call ->honeysql on a honeysql-expr

* Fix bad call to h2x

* Identifier needs to be wrapped

* Update kondo config

* Fix test

* Remove hformat fn handler

* Rotate the Snowflake DB prefix AGAIN

* `sql.qp/format-honeysql` needs to wrap stuff in parens when generating SQL snippets

* Update tests to match new expected SQL format

* Sort namespaces

* Fix the flaky paging test

* Fix flaky `metabase.query-processor-test.timezones-test/filter-datetime-by-date-in-timezone-test`

* MySQL test fix 



* Try running Redshift with a beefier runner

---------

Co-authored-by: Cam Saul <github@camsaul.com>
Co-authored-by: Cam Saul <1455846+camsaul@users.noreply.github.com>

.clj-kondo/config.edn

@@ -597,14 +597,13 @@
   ;;           "bigquery-cloud-sdk"
   ;;           [:and "presto" [:? [:or "-common" "-jdbc"]]]
   ;;           "snowflake"
-  ;;           "sqlite"
   ;;           "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)?))|(?:bigquery-cloud-sdk)|(?:presto(?:(?:(?:-common)|(?:-jdbc)))?)|(?:snowflake)|(?:sqlite)|(?:sqlserver))(?:-test)?(?:(?:\\.)|(?:$))).*"
+  {:pattern "^metabase\\.(?!util\\.(?:(?:honeysql-extensions)|(?:honey-sql-1)))(?!query-processor-test)(?!(?:(?:driver)|(?:test\\.data))\\.(?:(?:sql(?:-jdbc)?)|(?:(?:sql(?:-jdbc)?))|(?:bigquery-cloud-sdk)|(?:presto(?:(?:(?:-common)|(?:-jdbc)))?)|(?:snowflake)|(?:sqlserver))(?:-test)?(?:(?:\\.)|(?:$))).*"
    :name    honey-sql-2-namespaces}]
 
  :config-in-ns

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

@@ -2,7 +2,6 @@
   (:require
    [clojure.java.io :as io]
    [clojure.string :as str]
-   [honeysql.format :as hformat]
    [java-time :as t]
    [metabase.config :as config]
    [metabase.driver :as driver]
@@ -16,7 +15,7 @@
    [metabase.driver.sql.query-processor :as sql.qp]
    [metabase.query-processor.error-type :as qp.error-type]
    [metabase.util.date-2 :as u.date]
-   [metabase.util.honeysql-extensions :as hx]
+   [metabase.util.honey-sql-2 :as h2x]
    [metabase.util.i18n :refer [tru]]
    [schema.core :as s])
   (:import
@@ -28,6 +27,10 @@
 
 (driver/register! :sqlite, :parent :sql-jdbc)
 
+(defmethod sql.qp/honey-sql-version :sqlite
+  [_driver]
+  2)
+
 ;; SQLite does not support a lot of features, so do not show the options in the interface
 (doseq [[feature supported?] {:right-join                             false
                               :full-join                              false
@@ -109,75 +112,68 @@
 (defmethod sql-jdbc.sync/fallback-metadata-query :sqlite
   [driver schema table]
   (sql.qp/format-honeysql driver {:select [:*]
-                                  :from   [(sql.qp/->honeysql driver (hx/identifier :table schema table))]
+                                  :from   [[(h2x/identifier :table schema table)]]
                                   :limit  1}))
 
-;; register the SQLite concatenation operator `||` with HoneySQL as `sqlite-concat`
-;;
-;;    (hsql/format (hx/call :sqlite-concat :a :b)) -> "(a || b)"
-(defmethod hformat/fn-handler "sqlite-concat"
-  [_ & args]
-  (str "(" (str/join " || " (map hformat/to-sql args)) ")"))
-
-(def ^:private ->date     (partial hx/call :date))
-(def ^:private ->datetime (partial hx/call :datetime))
-(def ^:private ->time     (partial hx/call :time))
+(def ^:private ->date     (partial conj [:date]))
+(def ^:private ->datetime (partial conj [:datetime]))
+(def ^:private ->time     (partial conj [:time]))
 
 (defn- strftime [format-str expr]
-  (hx/call :strftime (hx/literal format-str) expr))
+  [:strftime (h2x/literal format-str) expr])
 
 ;; See also the [SQLite Date and Time Functions Reference](http://www.sqlite.org/lang_datefunc.html).
 
 (defmethod sql.qp/date [:sqlite :default] [_driver _unit expr] expr)
 
 (defmethod sql.qp/date [:sqlite :second]
-  [driver _ expr]
-  (->datetime (strftime "%Y-%m-%d %H:%M:%S" (sql.qp/->honeysql driver expr))))
+  [_driver _ expr]
+  (->datetime (strftime "%Y-%m-%d %H:%M:%S" expr)))
 
 (defmethod sql.qp/date [:sqlite :second-of-minute]
-  [driver _ expr]
-  (hx/->integer (strftime "%S" (sql.qp/->honeysql driver expr))))
+  [_driver _ expr]
+  (h2x/->integer (strftime "%S" expr)))
 
 (defmethod sql.qp/date [:sqlite :minute]
-  [driver _ expr]
-  (->datetime (strftime "%Y-%m-%d %H:%M" (sql.qp/->honeysql driver expr))))
+  [_driver _ expr]
+  (->datetime (strftime "%Y-%m-%d %H:%M" expr)))
 
 (defmethod sql.qp/date [:sqlite :minute-of-hour]
-  [driver _ expr]
-  (hx/->integer (strftime "%M" (sql.qp/->honeysql driver expr))))
+  [_driver _ expr]
+  (h2x/->integer (strftime "%M" expr)))
 
 (defmethod sql.qp/date [:sqlite :hour]
-  [driver _ expr]
-  (->datetime (strftime "%Y-%m-%d %H:00" (sql.qp/->honeysql driver expr))))
+  [_driver _ expr]
+  (->datetime (strftime "%Y-%m-%d %H:00" expr)))
 
 (defmethod sql.qp/date [:sqlite :hour-of-day]
-  [driver _ expr]
-  (hx/->integer (strftime "%H" (sql.qp/->honeysql driver expr))))
+  [_driver _ expr]
+  (h2x/->integer (strftime "%H" expr)))
 
 (defmethod sql.qp/date [:sqlite :day]
-  [driver _ expr]
-  (->date (sql.qp/->honeysql driver expr)))
+  [_driver _ expr]
+  (->date expr))
 
 ;; SQLite day of week (%w) is Sunday = 0 <-> Saturday = 6. We want 1 - 7 so add 1
 (defmethod sql.qp/date [:sqlite :day-of-week]
-  [driver _ expr]
-  (sql.qp/adjust-day-of-week :sqlite (hx/->integer (hx/inc (strftime "%w" (sql.qp/->honeysql driver expr))))))
+  [_driver _ expr]
+  (sql.qp/adjust-day-of-week :sqlite (h2x/->integer (h2x/inc (strftime "%w" expr)))))
 
 (defmethod sql.qp/date [:sqlite :day-of-month]
-  [driver _ expr]
-  (hx/->integer (strftime "%d" (sql.qp/->honeysql driver expr))))
+  [_driver _ expr]
+  (h2x/->integer (strftime "%d" expr)))
 
 (defmethod sql.qp/date [:sqlite :day-of-year]
-  [driver _ expr]
-  (hx/->integer (strftime "%j" (sql.qp/->honeysql driver expr))))
+  [_driver _ expr]
+  (h2x/->integer (strftime "%j" expr)))
 
 (defmethod sql.qp/date [:sqlite :week]
   [_ _ expr]
   (let [week-extract-fn (fn [expr]
                           ;; Move back 6 days, then forward to the next Sunday
-                          (->date (sql.qp/->honeysql :sqlite expr)
-                                  (hx/literal "-6 days")
-                                  (hx/literal "weekday 0")))]
+                          (->date expr
+                                  (h2x/literal "-6 days")
+                                  (h2x/literal "weekday 0")))]
     (sql.qp/adjust-start-of-week :sqlite week-extract-fn expr)))
 
 (defmethod sql.qp/date [:sqlite :week-of-year-iso]
@@ -189,12 +185,12 @@
                    :type   qp.error-type/invalid-query})))
 
 (defmethod sql.qp/date [:sqlite :month]
-  [driver _ expr]
-  (->date (sql.qp/->honeysql driver expr) (hx/literal "start of month")))
+  [_driver _ expr]
+  (->date expr (h2x/literal "start of month")))
 
 (defmethod sql.qp/date [:sqlite :month-of-year]
-  [driver _ expr]
-  (hx/->integer (strftime "%m" (sql.qp/->honeysql driver expr))))
+  [_driver _ expr]
+  (h2x/->integer (strftime "%m" expr)))
 
 ;;    DATE(DATE(%s, 'start of month'), '-' || ((STRFTIME('%m', %s) - 1) % 3) || ' months')
 ;; -> DATE(DATE('2015-11-16', 'start of month'), '-' || ((STRFTIME('%m', '2015-11-16') - 1) % 3) || ' months')
@@ -203,30 +199,29 @@
 ;; -> DATE('2015-11-01', '-1 months')
 ;; -> '2015-10-01'
 (defmethod sql.qp/date [:sqlite :quarter]
-  [driver _ expr]
-  (let [v (sql.qp/->honeysql driver expr)]
-    (->date
-     (->date v (hx/literal "start of month"))
-     (hx/call :sqlite-concat
-              (hx/literal "-")
-              (hx/mod (hx/dec (strftime "%m" v))
-                      3)
-              (hx/literal " months")))))
+  [_driver _ expr]
+  (->date
+    (->date expr (h2x/literal "start of month"))
+    [:||
+     (h2x/literal "-")
+     (h2x/mod (h2x/dec (strftime "%m" expr))
+              3)
+     (h2x/literal " months")]))
 
 ;; q = (m + 2) / 3
 (defmethod sql.qp/date [:sqlite :quarter-of-year]
-  [driver _ expr]
-  (hx// (hx/+ (strftime "%m" (sql.qp/->honeysql driver expr))
-              2)
-        3))
+  [_driver _ expr]
+  (h2x// (h2x/+ (strftime "%m" expr)
+                2)
+         3))
 
 (defmethod sql.qp/date [:sqlite :year]
-  [driver _ expr]
-  (->date (sql.qp/->honeysql driver expr) (hx/literal "start of year")))
+  [_driver _ expr]
+  (->date expr (h2x/literal "start of year")))
 
 (defmethod sql.qp/date [:sqlite :year-of-era]
-  [driver _ expr]
-  (hx/->integer (strftime "%Y" (sql.qp/->honeysql driver expr))))
+  [_driver _ expr]
+  (h2x/->integer (strftime "%Y" expr)))
 
 (defmethod sql.qp/add-interval-honeysql-form :sqlite
   [_driver hsql-form amount unit]
@@ -239,11 +234,11 @@
                                    :month   [1 "months"]
                                    :quarter [3 "months"]
                                    :year    [1 "years"])]
-    (->datetime hsql-form (hx/literal (format "%+d %s" (* amount multiplier) sqlite-unit)))))
+    (->datetime hsql-form (h2x/literal (format "%+d %s" (* amount multiplier) sqlite-unit)))))
 
 (defmethod sql.qp/unix-timestamp->honeysql [:sqlite :seconds]
   [_ _ expr]
-  (->datetime expr (hx/literal "unixepoch")))
+  (->datetime expr (h2x/literal "unixepoch")))
 
 (defmethod sql.qp/cast-temporal-string [:sqlite :Coercion/ISO8601->DateTime]
   [_driver _semantic_type expr]
@@ -280,26 +275,26 @@
 (defmethod sql.qp/->honeysql [:sqlite :substring]
   [driver [_ arg start length]]
   (if length
-    (hx/call :substr (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver start) (sql.qp/->honeysql driver length))
-    (hx/call :substr (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver start))))
+    [:substr (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver start) (sql.qp/->honeysql driver length)]
+    [:substr (sql.qp/->honeysql driver arg) (sql.qp/->honeysql driver start)]))
 
 (defmethod sql.qp/->honeysql [:sqlite :concat]
   [driver [_ & args]]
-  (apply
-   hx/call :sqlite-concat
+  (into
+   [:||]
    (mapv (partial sql.qp/->honeysql driver) args)))
 
 (defmethod sql.qp/->honeysql [:sqlite :floor]
   [_driver [_ arg]]
-  (hx/call :round (hx/call :- arg 0.5)))
+  [:round (h2x/- arg 0.5)])
 
 (defmethod sql.qp/->honeysql [:sqlite :ceil]
   [_driver [_ arg]]
-  (hx/call :case
-           ;; if we're ceiling a whole number, just cast it to an integer
-           ;; [:ceil 1.0] should returns 1
-           (hx/call := (hx/call :round arg) arg) (hx/->integer arg)
-           :else                                     (hx/call :round (hx/call :+ arg 0.5))))
+  [:case
+   ;; if we're ceiling a whole number, just cast it to an integer
+   ;; [:ceil 1.0] should returns 1
+   [:= [:round arg] arg] (h2x/->integer arg)
+   :else                 [:round (h2x/+ arg 0.5)]])
 
 ;; See https://sqlite.org/lang_datefunc.html
 
@@ -314,33 +309,33 @@
 
 (defmethod sql.qp/->honeysql [:sqlite LocalDate]
   [_ t]
-  (hx/call :date (hx/literal (u.date/format-sql t))))
+  [:date (h2x/literal (u.date/format-sql t))])
 
 (defmethod sql.qp/->honeysql [:sqlite LocalDateTime]
   [driver t]
   (if (zero-time? t)
     (sql.qp/->honeysql driver (t/local-date t))
-    (hx/call :datetime (hx/literal (u.date/format-sql t)))))
+    [:datetime (h2x/literal (u.date/format-sql t))]))
 
 (defmethod sql.qp/->honeysql [:sqlite LocalTime]
   [_ t]
-  (hx/call :time (hx/literal (u.date/format-sql t))))
+  [:time (h2x/literal (u.date/format-sql t))])
 
 (defmethod sql.qp/->honeysql [:sqlite OffsetDateTime]
   [driver t]
   (if (zero-time? t)
     (sql.qp/->honeysql driver (t/local-date t))
-    (hx/call :datetime (hx/literal (u.date/format-sql t)))))
+    [:datetime (h2x/literal (u.date/format-sql t))]))
 
 (defmethod sql.qp/->honeysql [:sqlite OffsetTime]
   [_ t]
-  (hx/call :time (hx/literal (u.date/format-sql t))))
+  [:time (h2x/literal (u.date/format-sql t))])
 
 (defmethod sql.qp/->honeysql [:sqlite ZonedDateTime]
   [driver t]
   (if (zero-time? t)
     (sql.qp/->honeysql driver (t/local-date t))
-    (hx/call :datetime (hx/literal (u.date/format-sql t)))))
+    [:datetime (h2x/literal (u.date/format-sql t))]))
 
 ;; SQLite defaults everything to UTC
 (defmethod driver.common/current-db-time-date-formatters :sqlite
@@ -361,56 +356,55 @@
 
 (defmethod sql.qp/current-datetime-honeysql-form :sqlite
   [_]
-  (hx/call :datetime (hx/literal :now)))
+  [:datetime (h2x/literal :now)])
 
 (defmethod sql.qp/datetime-diff [:sqlite :year]
   [driver _unit x y]
-  (hx// (sql.qp/datetime-diff driver :month x y) 12))
+  (h2x// (sql.qp/datetime-diff driver :month x y) 12))
 
 (defmethod sql.qp/datetime-diff [:sqlite :quarter]
   [driver _unit x y]
-  (hx// (sql.qp/datetime-diff driver :month x y) 3))
+  (h2x// (sql.qp/datetime-diff driver :month x y) 3))
 
 (defmethod sql.qp/datetime-diff [:sqlite :month]
   [driver _unit x y]
   (let [extract            (fn [unit x] (sql.qp/date driver unit x))
-        year-diff          (hx/- (extract :year y) (extract :year x))
-        month-of-year-diff (hx/- (extract :month-of-year y) (extract :month-of-year x))
-        total-month-diff   (hx/+ month-of-year-diff (hx/* year-diff 12))]
-    (hx/+ total-month-diff
+        year-diff          (h2x/- (extract :year y) (extract :year x))
+        month-of-year-diff (h2x/- (extract :month-of-year y) (extract :month-of-year x))
+        total-month-diff   (h2x/+ month-of-year-diff (h2x/* year-diff 12))]
+    (h2x/+ total-month-diff
           ;; total-month-diff 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 :< x y) (hx/call :> (extract :day-of-month x) (extract :day-of-month y)))
+          [:case
+           [:and [:< x y] [:> (extract :day-of-month x) (extract :day-of-month y)]]
            -1
-           (hx/call :and (hx/call :> x y) (hx/call :< (extract :day-of-month x) (extract :day-of-month y)))
+           [:and [:> x y] [:< (extract :day-of-month x) (extract :day-of-month y)]]
            1
-           :else 0))))
+           :else 0])))
 
 (defmethod sql.qp/datetime-diff [:sqlite :week]
   [driver _unit x y]
-  (hx// (sql.qp/datetime-diff driver :day x y) 7))
+  (h2x// (sql.qp/datetime-diff driver :day x y) 7))
 
 (defmethod sql.qp/datetime-diff [:sqlite :day]
   [_driver _unit x y]
-  (hx/->integer
-   (hx/- (hx/call :julianday y (hx/literal "start of day"))
-         (hx/call :julianday x (hx/literal "start of day")))))
+  (h2x/->integer
+    (h2x/- [:julianday y (h2x/literal "start of day")]
+           [:julianday x (h2x/literal "start of day")])))
 
 (defmethod sql.qp/datetime-diff [:sqlite :hour]
   [driver _unit x y]
-  (hx// (sql.qp/datetime-diff driver :second x y) 3600))
+  (h2x// (sql.qp/datetime-diff driver :second x y) 3600))
 
 (defmethod sql.qp/datetime-diff [:sqlite :minute]
   [driver _unit x y]
-  (hx// (sql.qp/datetime-diff driver :second x y) 60))
+  (h2x// (sql.qp/datetime-diff driver :second x y) 60))
 
 (defmethod sql.qp/datetime-diff [:sqlite :second]
   [_driver _unit x y]
   ;; strftime strftime('%s', <timestring>) returns the unix time as an integer.
-  (hx/- (strftime "%s" y) (strftime "%s" x)))
+  (h2x/- (strftime "%s" y) (strftime "%s" x)))
 
 ;; SQLite's JDBC driver is fussy and won't let you change connections to read-only after you create them. So skip that
 ;; step. SQLite doesn't have a notion of session timezones so don't do that either. The only thing we're doing here from

modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj

@@ -216,7 +216,7 @@
     (mt/test-driver :sqlite
       (mt/dataset sample-dataset
         (is (= '{:select   [source.CATEGORY_2 AS CATEGORY_2
-                            count (*)         AS count]
+                            COUNT (*)         AS count]
                  :from     [{:select [products.id              AS id
                                       products.ean             AS ean
                                       products.title           AS title
@@ -225,9 +225,9 @@
                                       products.price           AS price
                                       products.rating          AS rating
                                       products.created_at      AS created_at
-                                      (products.category || ?) AS CATEGORY_2]
+                                      products.category || ?  AS CATEGORY_2]
                              :from   [products]}
-                            source]
+                            AS source]
                  :group-by [source.CATEGORY_2]
                  :order-by [source.CATEGORY_2 ASC]
                  :limit    [1]}