Snowflake Honey SQL 2 (#28478)

* Snowflake Honey SQL 2

* Test fix 

* Use Honey SQL 2 for GTAP compilation in sandboxing tests

* Test fix 

.clj-kondo/config.edn

@@ -596,14 +596,13 @@
   ;;           #"(?:sql(?:-jdbc)?)"
   ;;           "bigquery-cloud-sdk"
   ;;           [:and "presto" [:? [:or "-common" "-jdbc"]]]
-  ;;           "snowflake"
   ;;           "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)|(?: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)))?)|(?:sqlserver))(?:-test)?(?:(?:\\.)|(?:$))).*"
    :name    honey-sql-2-namespaces}]
 
  :config-in-ns

.github/workflows/drivers.yml

@@ -569,7 +569,7 @@ jobs:
   be-tests-redshift-ee:
     needs: files-changed
     if: github.event.pull_request.draft == false && needs.files-changed.outputs.backend_all == 'true'
-    runs-on: buildjet-2vcpu-ubuntu-2004
+    runs-on: ubuntu-20.04
     timeout-minutes: 60
     env:
       CI: 'true'

enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj

@@ -3,7 +3,6 @@
    [clojure.core.async :as a]
    [clojure.string :as str]
    [clojure.test :refer :all]
-   [honeysql.core :as hsql]
    [medley.core :as m]
    [metabase-enterprise.sandbox.models.group-table-access-policy
     :refer [GroupTableAccessPolicy]]
@@ -72,61 +71,66 @@
                    ;; have one.) HACK
                    (= driver/*driver* :sparksql)
                    (update :from (fn [[table]]
-                                   [[table (sql.qp/->honeysql
-                                            :sparksql
-                                            (hx/identifier :table-alias @(resolve 'metabase.driver.sparksql/source-table-alias)))]])))]
-    (first (hsql/format honeysql, :quoting (sql.qp/quote-style driver/*driver*), :allow-dashed-names? true))))
+                                   [[table [(sql.qp/->honeysql
+                                             :sparksql
+                                             (hx/identifier :table-alias @(resolve 'metabase.driver.sparksql/source-table-alias)))]]])))]
+    (first (sql.qp/format-honeysql driver/*driver* honeysql))))
 
 (defn- venues-category-native-gtap-def []
   (driver/with-driver (or driver/*driver* :h2)
     (assert (driver/supports? driver/*driver* :native-parameters))
-    {:query (mt/native-query
-             {:query
-              (format-honeysql
-               {:select   [:*]
-                :from     [(identifier :venues)]
-                :where    [:= (identifier :venues :category_id) (hx/raw "{{cat}}")]
-                :order-by [(identifier :venues :id)]})
-
-              :template_tags
-              {:cat {:name "cat" :display_name "cat" :type "number" :required true}}})
-     :remappings {:cat ["variable" ["template-tag" "cat"]]}}))
+    (binding [hx/*honey-sql-version* (sql.qp/honey-sql-version driver/*driver*)]
+      {:query (mt/native-query
+                {:query
+                 (format-honeysql
+                  {:select   [:*]
+                   :from     [(sql.qp/maybe-wrap-unaliased-expr (identifier :venues))]
+                   :where    [:= (identifier :venues :category_id) (hx/raw "{{cat}}")]
+                   :order-by [[(identifier :venues :id) :asc]]})
+
+                 :template_tags
+                 {:cat {:name "cat" :display_name "cat" :type "number" :required true}}})
+       :remappings {:cat ["variable" ["template-tag" "cat"]]}})))
 
 (defn- parameterized-sql-with-join-gtap-def []
   (driver/with-driver (or driver/*driver* :h2)
     (assert (driver/supports? driver/*driver* :native-parameters))
-    {:query (mt/native-query
-             {:query
-              (format-honeysql
-               {:select    [(identifier :checkins :id)
-                            (identifier :checkins :user_id)
-                            (identifier :venues :name)
-                            (identifier :venues :category_id)]
-                :from      [(identifier :checkins)]
-                :left-join [(identifier :venues)
-                            [:= (identifier :checkins :venue_id) (identifier :venues :id)]]
-                :where     [:= (identifier :checkins :user_id) (hx/raw "{{user}}")]
-                :order-by  [[(identifier :checkins :id) :asc]]})
-
-              :template_tags
-              {"user" {:name         "user"
-                       :display-name "User ID"
-                       :type         :number
-                       :required     true}}})
-     :remappings {:user ["variable" ["template-tag" "user"]]}}))
+    (binding [hx/*honey-sql-version* (sql.qp/honey-sql-version driver/*driver*)]
+      {:query (mt/native-query
+                {:query
+                 (format-honeysql
+                  {:select    [(sql.qp/maybe-wrap-unaliased-expr (identifier :checkins :id))
+                               (sql.qp/maybe-wrap-unaliased-expr (identifier :checkins :user_id))
+                               (sql.qp/maybe-wrap-unaliased-expr (identifier :venues :name))
+                               (sql.qp/maybe-wrap-unaliased-expr (identifier :venues :category_id))]
+                   :from      [(sql.qp/maybe-wrap-unaliased-expr (identifier :checkins))]
+                   :left-join [(sql.qp/maybe-wrap-unaliased-expr (identifier :venues))
+                               [:= (identifier :checkins :venue_id) (identifier :venues :id)]]
+                   :where     [:= (identifier :checkins :user_id) (hx/raw "{{user}}")]
+                   :order-by  [[(identifier :checkins :id) :asc]]})
+
+                 :template_tags
+                 {"user" {:name         "user"
+                          :display-name "User ID"
+                          :type         :number
+                          :required     true}}})
+       :remappings {:user ["variable" ["template-tag" "user"]]}})))
 
 (defn- venue-names-native-gtap-def []
-  {:query (mt/native-query
-            {:query
-             (format-honeysql
-              {:select   [(identifier :venues :id) (identifier :venues :name)]
-               :from     [(identifier :venues)]
-               :order-by [(identifier :venues :id)]})})})
+  (driver/with-driver (or driver/*driver* :h2)
+    (binding [hx/*honey-sql-version* (sql.qp/honey-sql-version driver/*driver*)]
+      {:query (mt/native-query
+                {:query
+                 (format-honeysql
+                  {:select   [(sql.qp/maybe-wrap-unaliased-expr (identifier :venues :id))
+                              (sql.qp/maybe-wrap-unaliased-expr (identifier :venues :name))]
+                   :from     [(sql.qp/maybe-wrap-unaliased-expr (identifier :venues))]
+                   :order-by [[(identifier :venues :id) :asc]]})})})))
 
 (defn- run-venues-count-query []
   (mt/format-rows-by [int]
     (mt/rows
-      (mt/run-mbql-query venues {:aggregation [[:count]]}))))
+     (mt/run-mbql-query venues {:aggregation [[:count]]}))))
 
 (defn- run-checkins-count-broken-out-by-price-query []
   (mt/format-rows-by [#(some-> % int) int]
@@ -229,7 +233,7 @@
                   :query    {:aggregation  [[:count]]
                              :source-query {:native (str "SELECT * FROM \"PUBLIC\".\"VENUES\" "
                                                          "WHERE \"PUBLIC\".\"VENUES\".\"CATEGORY_ID\" = 50 "
-                                                         "ORDER BY \"PUBLIC\".\"VENUES\".\"ID\"")
+                                                         "ORDER BY \"PUBLIC\".\"VENUES\".\"ID\" ASC")
                                             :params []}}
 
                   ::row-level-restrictions/original-metadata [{:base_type     :type/BigInteger
@@ -354,15 +358,16 @@
                   "card) to see if row level permissions apply. This was broken when it wasn't expecting a card and "
                   "only expecting resolved source-tables")
       (mt/with-temp Card [card {:dataset_query (mt/mbql-query venues)}]
-        (mt/with-test-user :rasta
-          (is (= [[100]]
-                 (mt/format-rows-by [int]
-                   (mt/rows
-                    (qp/process-query
-                     {:database (mt/id)
-                      :type     :query
-                      :query    {:source-table (format "card__%s" (u/the-id card))
-                                 :aggregation  [["count"]]}}))))))))))
+        (let [query {:database (mt/id)
+                     :type     :query
+                     :query    {:source-table (format "card__%s" (u/the-id card))
+                                :aggregation  [["count"]]}}]
+          (mt/with-test-user :rasta
+            (mt/with-native-query-testing-context query
+              (is (= [[100]]
+                     (mt/format-rows-by [int]
+                       (mt/rows
+                        (qp/process-query query))))))))))))
 
 ;; Test that we can follow FKs to related tables and breakout by columns on those related tables. This test has
 ;; several things wrapped up which are detailed below

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

@@ -25,6 +25,8 @@
    [metabase.query-processor.util.add-alias-info :as add]
    [metabase.util :as u]
    [metabase.util.date-2 :as u.date]
+   [metabase.util.honey-sql-2 :as h2x]
+   #_{:clj-kondo/ignore [:discouraged-namespace]}
    [metabase.util.honeysql-extensions :as hx]
    [metabase.util.i18n :refer [trs tru]]
    [metabase.util.log :as log]
@@ -33,8 +35,7 @@
    (java.io File)
    (java.nio.charset StandardCharsets)
    (java.sql ResultSet Types)
-   (java.time OffsetDateTime ZonedDateTime)
-   (metabase.util.honey_sql_1 Identifier)))
+   (java.time OffsetDateTime ZonedDateTime)))
 
 (set! *warn-on-reflection* true)
 
@@ -174,23 +175,27 @@
     :OBJECT                     :type/Dictionary
     :ARRAY                      :type/*} base-type))
 
-(defmethod sql.qp/unix-timestamp->honeysql [:snowflake :seconds]      [_ _ expr] (hx/call :to_timestamp expr))
-(defmethod sql.qp/unix-timestamp->honeysql [:snowflake :milliseconds] [_ _ expr] (hx/call :to_timestamp expr 3))
-(defmethod sql.qp/unix-timestamp->honeysql [:snowflake :microseconds] [_ _ expr] (hx/call :to_timestamp expr 6))
+(defmethod sql.qp/honey-sql-version :snowflake
+  [_driver]
+  2)
+
+(defmethod sql.qp/unix-timestamp->honeysql [:snowflake :seconds]      [_ _ expr] [:to_timestamp expr])
+(defmethod sql.qp/unix-timestamp->honeysql [:snowflake :milliseconds] [_ _ expr] [:to_timestamp expr 3])
+(defmethod sql.qp/unix-timestamp->honeysql [:snowflake :microseconds] [_ _ expr] [:to_timestamp expr 6])
 
 (defmethod sql.qp/current-datetime-honeysql-form :snowflake
   [_]
-  (hx/with-database-type-info :%current_timestamp :TIMESTAMPTZ))
+  (h2x/with-database-type-info :%current_timestamp :TIMESTAMPTZ))
 
 (defmethod sql.qp/add-interval-honeysql-form :snowflake
   [_ hsql-form amount unit]
-  (hx/call :dateadd
-           (hx/raw (name unit))
-           (hx/raw (int amount))
-           (hx/->timestamp hsql-form)))
+  [:dateadd
+   [:raw (name unit)]
+   [:raw (int amount)]
+   (h2x/->timestamp hsql-form)])
 
-(defn- extract    [unit expr] (hx/call :date_part unit (hx/->timestamp expr)))
-(defn- date-trunc [unit expr] (hx/call :date_trunc unit (hx/->timestamp expr)))
+(defn- extract    [unit expr] [:date_part  unit (h2x/->timestamp expr)])
+(defn- date-trunc [unit expr] [:date_trunc unit (h2x/->timestamp expr)])
 
 (defmethod sql.qp/date [:snowflake :default]         [_ _ expr] expr)
 (defmethod sql.qp/date [:snowflake :minute]          [_ _ expr] (date-trunc :minute expr))
@@ -235,55 +240,61 @@
    before calculating date boundaries. This is needed when an argument could be of
    timestamptz type and the unit is day, week, month, quarter or year."
   [unit x y]
-  (let [x (cond->> x
-            (hx/is-of-type? x "timestamptz")
-            (hx/call :convert_timezone (qp.timezone/results-timezone-id)))
-        y (cond->> y
-            (hx/is-of-type? y "timestamptz")
-            (hx/call :convert_timezone (qp.timezone/results-timezone-id)))]
-    (hx/call :datediff (hx/raw (name unit)) x y)))
+  (let [x (if (h2x/is-of-type? x "timestamptz")
+            [:convert_timezone (qp.timezone/results-timezone-id) x]
+            x)
+        y (if (h2x/is-of-type? y "timestamptz")
+            [:convert_timezone (qp.timezone/results-timezone-id) y]
+            y)]
+    [:datediff [:raw (name unit)] x y]))
 
 (defn- time-zoned-extract
   "Same as `extract` but converts the arg to the results time zone if it's a timestamptz."
   [unit x]
-  (let [x (cond->> x
-            (hx/is-of-type? x "timestamptz")
-            (hx/call :convert_timezone (qp.timezone/results-timezone-id)))]
+  (let [x (if (h2x/is-of-type? x "timestamptz")
+            [:convert_timezone (qp.timezone/results-timezone-id) x]
+            x)]
     (extract unit x)))
 
 (defn- sub-day-datediff
   "Same as snowflake's `datediff`, but accurate to the millisecond for sub-day units."
   [unit x y]
-  (let [milliseconds (hx/call :datediff (hx/raw "milliseconds") x y)]
+  (let [milliseconds [:datediff [:raw "milliseconds"] x y]]
     ;; millseconds needs to be cast to float because division rounds incorrectly with large integers
-    (hx/call :trunc (hx// (hx/cast :float milliseconds)
-                          (case unit :hour 3600000 :minute 60000 :second 1000)))))
+    [:trunc (h2x// (h2x/cast :float milliseconds)
+                   (case unit :hour 3600000 :minute 60000 :second 1000))]))
 
 (defmethod sql.qp/datetime-diff [:snowflake :year]
   [driver _unit x y]
-  (hx/call :trunc (hx// (sql.qp/datetime-diff driver :month x y) 12)))
+  [:trunc (h2x// (sql.qp/datetime-diff driver :month x y) 12)])
 
 (defmethod sql.qp/datetime-diff [:snowflake :quarter]
   [driver _unit x y]
-  (hx/call :trunc (hx// (sql.qp/datetime-diff driver :month x y) 3)))
+  [:trunc (h2x// (sql.qp/datetime-diff driver :month x y) 3)])
 
 (defmethod sql.qp/datetime-diff [:snowflake :month]
   [_driver _unit x y]
-  (hx/+ (time-zoned-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 :< x y) (hx/call :> (time-zoned-extract :day x) (time-zoned-extract :day y)))
-         -1
-         (hx/call :and (hx/call :> x y) (hx/call :< (time-zoned-extract :day x) (time-zoned-extract :day y)))
-         1
-         :else 0)))
+  (h2x/+ (time-zoned-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
+           [:< x y]
+           [:> (time-zoned-extract :day x) (time-zoned-extract :day y)]]
+          -1
+
+          [:and
+           [:> x y]
+           [:< (time-zoned-extract :day x) (time-zoned-extract :day y)]]
+          1
+
+          :else
+          0]))
 
 (defmethod sql.qp/datetime-diff [:snowflake :week]
   [_driver _unit x y]
-  (hx/call :trunc (hx// (time-zoned-datediff :day x y) 7)))
+  [:trunc (h2x// (time-zoned-datediff :day x y) 7)])
 
 (defmethod sql.qp/datetime-diff [:snowflake :day]
   [_driver _unit x y]
@@ -295,7 +306,7 @@
 
 (defmethod sql.qp/->honeysql [:snowflake :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)])
 
 (defmethod sql.qp/->honeysql [:snowflake :median]
   [driver [_ arg]]
@@ -327,13 +338,16 @@
 
 ;; This takes care of Table identifiers. We handle Field identifiers in the [[sql.qp/->honeysql]] method for `[:sql
 ;; :field]` below.
-(defmethod sql.qp/->honeysql [:snowflake Identifier]
-  [_ {:keys [identifier-type], :as identifier}]
+(defn- qualify-identifier [[_identifier identifier-type components, :as identifier]]
+  {:pre [(h2x/identifier? identifier)]}
+  (apply h2x/identifier identifier-type (query-db-name) components))
+
+(defmethod sql.qp/->honeysql [:snowflake ::h2x/identifier]
+  [_driver [_identifier identifier-type :as identifier]]
   (let [qualify? (and (seq (query-db-name))
                       (= identifier-type :table))]
     (cond-> identifier
-      qualify?
-      (update :components (partial cons (query-db-name))))))
+      qualify? qualify-identifier)))
 
 (defmethod sql.qp/->honeysql [:snowflake :field]
   [driver [_ _ {::add/keys [source-table]} :as field-clause]]
@@ -346,30 +360,31 @@
                        (integer? source-table))
         identifier (parent-method driver field-clause)]
     (cond-> identifier
-      qualify? (update :components (partial cons (query-db-name))))))
+      (and qualify? (h2x/identifier? identifier))
+      qualify-identifier)))
 
 (defmethod sql.qp/->honeysql [:snowflake :time]
   [driver [_ value _unit]]
-  (hx/->time (sql.qp/->honeysql driver value)))
+  (h2x/->time (sql.qp/->honeysql driver value)))
 
 (defmethod sql.qp/->honeysql [:snowflake :convert-timezone]
   [driver [_ arg target-timezone source-timezone]]
   (let [hsql-form    (sql.qp/->honeysql driver arg)
-        timestamptz? (hx/is-of-type? hsql-form "timestamptz")]
+        timestamptz? (h2x/is-of-type? hsql-form "timestamptz")]
     (sql.u/validate-convert-timezone-args timestamptz? target-timezone source-timezone)
     (-> (if timestamptz?
-          (hx/call :convert_timezone target-timezone hsql-form)
-          (->> hsql-form
-               (hx/call :convert_timezone (or source-timezone (qp.timezone/results-timezone-id)) target-timezone)
-               (hx/call :to_timestamp_ntz)))
-        (hx/with-database-type-info "timestampntz"))))
+          [:convert_timezone target-timezone hsql-form]
+          [:to_timestamp_ntz
+           [:convert_timezone (or source-timezone (qp.timezone/results-timezone-id)) target-timezone hsql-form]])
+        (h2x/with-database-type-info "timestampntz"))))
 
 (defmethod driver/table-rows-seq :snowflake
   [driver database table]
   (sql-jdbc/query driver database {:select [:*]
-                                   :from   [(qp.store/with-store
-                                              (qp.store/fetch-and-store-database! (u/the-id database))
-                                              (sql.qp/->honeysql driver table))]}))
+                                   :from   [[(qp.store/with-store
+                                               (qp.store/fetch-and-store-database! (u/the-id database))
+                                               (binding [hx/*honey-sql-version* (sql.qp/honey-sql-version driver)]
+                                                 (sql.qp/->honeysql driver table)))]]}))
 
 (defmethod driver/describe-database :snowflake [driver database]
   (let [db-name          (db-name database)

modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj

@@ -10,6 +10,7 @@
    [metabase.models.database :refer [Database]]
    [metabase.query-processor :as qp]
    [metabase.sync :as sync]
+   [metabase.sync.util :as sync-util]
    [metabase.test :as mt]
    [metabase.test.data.dataset-definitions :as defs]
    [metabase.test.data.interface :as tx]
@@ -17,10 +18,29 @@
    [metabase.test.data.sql :as sql.tx]
    [metabase.test.data.sql.ddl :as ddl]
    [metabase.util :as u]
+   #_{:clj-kondo/ignore [:discouraged-namespace]}
+   [metabase.util.honeysql-extensions :as hx]
    [toucan.db :as db]))
 
 (set! *warn-on-reflection* true)
 
+(use-fixtures :each (fn [thunk]
+                      ;; 1. If sync fails when loading a test dataset, don't swallow the error; throw an Exception so we
+                      ;;    can debug it. This is much less confusing when trying to fix broken tests.
+                      ;;
+                      ;; 2. Make sure we're in Honey SQL 2 mode for all the little SQL snippets we're compiling in these
+                      ;;    tests.
+                      (binding [sync-util/*log-exceptions-and-continue?* false
+                                hx/*honey-sql-version*                   2]
+                        (thunk))))
+
+(deftest sanity-check-test
+  (mt/test-driver :snowflake
+    (is (= [100]
+           (mt/first-row
+            (mt/run-mbql-query venues
+              {:aggregation [[:count]]}))))))
+
 (deftest ^:parallel ddl-statements-test
   (testing "make sure we didn't break the code that is used to generate DDL statements when we add new test datasets"
     (binding [test.data.snowflake/*database-prefix-fn* (constantly "v3_")]

src/metabase/driver/sql_jdbc/sync/describe_database.clj

@@ -73,6 +73,7 @@
   [driver ^Connection conn [sql & params]]
   {:pre [(string? sql)]}
   (with-open [stmt (sql-jdbc.sync.common/prepare-statement driver conn sql params)]
+    (log/tracef "[%s] %s" (name driver) sql)
     ;; attempting to execute the SQL statement will throw an Exception if we don't have permissions; otherwise it will
     ;; truthy wheter or not it returns a ResultSet, but we can ignore that since we have enough info to proceed at
     ;; this point.
@@ -92,8 +93,8 @@
       (execute-select-probe-query driver conn sql-args)
       (log/trace "SELECT privileges confirmed")
       true
-      (catch Throwable _
-        (log/trace "No SELECT privileges")
+      (catch Throwable e
+        (log/trace e "Assuming no SELECT privileges: caught exception")
         false))))
 
 (defn- db-tables

test/metabase/query_processor_test/filter_test.clj

@@ -122,66 +122,66 @@
   (testing "Should be able to use temporal arithmetic expressions in filters (#22531)"
     (mt/test-drivers (timezone-arithmetic-drivers)
       (mt/dataset attempted-murders
-        (doseq [offset-unit [:year :day]
-                  interval-unit [:year :day]
-                  compare-op [:between := :< :<= :> :>=]
-                  compare-order (cond-> [:field-first]
-                                  (not= compare-op :between) (conj :value-first))]
-            (let [query (mt/mbql-query attempts
-                          {:aggregation [[:count]]
-                           :filter      (cond-> [compare-op]
-                                          (= compare-order :field-first)
-                                          (conj [:+ !default.datetime_tz [:interval 3 offset-unit]]
-                                                [:relative-datetime -7 interval-unit])
-                                          (= compare-order :value-first)
-                                          (conj [:relative-datetime -7 interval-unit]
-                                                [:+ !default.datetime_tz [:interval 3 offset-unit]])
-                                          (= compare-op :between)
-                                          (conj [:relative-datetime 0 interval-unit]))})]
-              ;; we are not interested in the exact result, just want to check
-              ;; that the query can be compiled and executed
-              (mt/with-native-query-testing-context query
-                (let [[[result]] (mt/formatted-rows [int]
-                                   (qp/process-query query))]
-                  (if (= driver/*driver* :mongo)
-                    (is (or (nil? result)
-                            (pos-int? result)))
-                    (is (nat-int? result)))))))))))
+        (doseq [offset-unit   [:year :day]
+                interval-unit [:year :day]
+                compare-op    [:between := :< :<= :> :>=]
+                compare-order (cond-> [:field-first]
+                                (not= compare-op :between) (conj :value-first))]
+          (let [query (mt/mbql-query attempts
+                        {:aggregation [[:count]]
+                         :filter      (cond-> [compare-op]
+                                        (= compare-order :field-first)
+                                        (conj [:+ !default.datetime_tz [:interval 3 offset-unit]]
+                                              [:relative-datetime -7 interval-unit])
+                                        (= compare-order :value-first)
+                                        (conj [:relative-datetime -7 interval-unit]
+                                              [:+ !default.datetime_tz [:interval 3 offset-unit]])
+                                        (= compare-op :between)
+                                        (conj [:relative-datetime 0 interval-unit]))})]
+            ;; we are not interested in the exact result, just want to check
+            ;; that the query can be compiled and executed
+            (mt/with-native-query-testing-context query
+              (let [[[result]] (mt/formatted-rows [int]
+                                 (qp/process-query query))]
+                (if (= driver/*driver* :mongo)
+                  (is (or (nil? result)
+                          (pos-int? result)))
+                  (is (nat-int? result)))))))))))
 
 (deftest nonstandard-temporal-arithmetic-test
   (testing "Nonstandard temporal arithmetic should also be supported"
     (mt/test-drivers (timezone-arithmetic-drivers)
       (mt/dataset attempted-murders
-        (doseq [offset-unit [:year :day]
-                  interval-unit [:year :day]
-                  compare-op [:between := :< :<= :> :>=]
-                  add-op [:+ #_:-] ; TODO support subtraction like sql.qp/add-interval-honeysql-form (#23423)
-                  compare-order (cond-> [:field-first]
-                                  (not= compare-op :between) (conj :value-first))]
-            (let [add-fn (fn [field interval]
-                           (if (= add-op :-)
-                             [add-op field interval interval]
-                             [add-op interval field interval]))
-                  query (mt/mbql-query attempts
-                          {:aggregation [[:count]]
-                           :filter      (cond-> [compare-op]
-                                          (= compare-order :field-first)
-                                          (conj (add-fn !default.datetime_tz [:interval 3 offset-unit])
-                                                [:relative-datetime -7 interval-unit])
-                                          (= compare-order :value-first)
-                                          (conj [:relative-datetime -7 interval-unit]
-                                                (add-fn !default.datetime_tz [:interval 3 offset-unit]))
-                                          (= compare-op :between)
-                                          (conj [:relative-datetime 0 interval-unit]))})]
-              ;; we are not interested in the exact result, just want to check
-              ;; that the query can be compiled and executed
-              (mt/with-native-query-testing-context query
-                (let [[[result]] (mt/formatted-rows [int]
-                                   (qp/process-query query))]
-                  (if (= driver/*driver* :mongo)
-                    (is (or (nil? result)
-                            (pos-int? result)))
-                    (is (nat-int? result)))))))))))
+        (doseq [offset-unit   [:year :day]
+                interval-unit [:year :day]
+                compare-op    [:between := :< :<= :> :>=]
+                add-op        [:+ #_:-] ; TODO support subtraction like sql.qp/add-interval-honeysql-form (#23423)
+                compare-order (cond-> [:field-first]
+                                (not= compare-op :between) (conj :value-first))]
+          (let [add-fn (fn [field interval]
+                         (if (= add-op :-)
+                           [add-op field interval interval]
+                           [add-op interval field interval]))
+                query  (mt/mbql-query attempts
+                         {:aggregation [[:count]]
+                          :filter      (cond-> [compare-op]
+                                         (= compare-order :field-first)
+                                         (conj (add-fn !default.datetime_tz [:interval 3 offset-unit])
+                                               [:relative-datetime -7 interval-unit])
+                                         (= compare-order :value-first)
+                                         (conj [:relative-datetime -7 interval-unit]
+                                               (add-fn !default.datetime_tz [:interval 3 offset-unit]))
+                                         (= compare-op :between)
+                                         (conj [:relative-datetime 0 interval-unit]))})]
+            ;; we are not interested in the exact result, just want to check
+            ;; that the query can be compiled and executed
+            (mt/with-native-query-testing-context query
+              (let [[[result]] (mt/formatted-rows [int]
+                                 (qp/process-query query))]
+                (if (= driver/*driver* :mongo)
+                  (is (or (nil? result)
+                          (pos-int? result)))
+                  (is (nat-int? result)))))))))))
 
 (deftest or-test
   (mt/test-drivers (mt/normal-drivers)