diff --git a/modules/drivers/bigquery/src/metabase/driver/bigquery.clj b/modules/drivers/bigquery/src/metabase/driver/bigquery.clj
index ab8a7e9574de716918ca149b71e65da9f13357b2..745286e2dea5baade6fb83205a2f7b46a13dfc2d 100644
--- a/modules/drivers/bigquery/src/metabase/driver/bigquery.clj
+++ b/modules/drivers/bigquery/src/metabase/driver/bigquery.clj
@@ -102,7 +102,7 @@
"STRING" :type/Text
"DATE" :type/Date
"DATETIME" :type/DateTime
- "TIMESTAMP" :type/DateTime
+ "TIMESTAMP" :type/DateTimeWithLocalTZ
"TIME" :type/Time
"NUMERIC" :type/Decimal
:type/*))
diff --git a/modules/drivers/bigquery/src/metabase/driver/bigquery/query_processor.clj b/modules/drivers/bigquery/src/metabase/driver/bigquery/query_processor.clj
index 7faa1f61c425acab5381633e19e9f392ad77681a..78f0ca0730ec58a80609825d926f379a46c9fe4a 100644
--- a/modules/drivers/bigquery/src/metabase/driver/bigquery/query_processor.clj
+++ b/modules/drivers/bigquery/src/metabase/driver/bigquery/query_processor.clj
@@ -15,7 +15,9 @@
[metabase.models
[field :refer [Field]]
[table :as table]]
- [metabase.query-processor.store :as qp.store]
+ [metabase.query-processor
+ [error-type :as error-type]
+ [store :as qp.store]]
[metabase.util
[date-2 :as u.date]
[honeysql-extensions :as hx]
@@ -102,92 +104,157 @@
;;; | SQL Driver Methods |
;;; +----------------------------------------------------------------------------------------------------------------+
-;; EXPERIMENTAL
+(def ^:private temporal-type-hierarchy
+ (-> (make-hierarchy)
+ (derive :date :temporal-type)
+ (derive :time :temporal-type)
+ (derive :datetime :temporal-type)
+ ;; timestamp = datetime with a timezone
+ (derive :timestamp :temporal-type)))
(defmulti ^:private temporal-type
{:arglists '([x])}
- mbql.u/dispatch-by-clause-name-or-class)
+ mbql.u/dispatch-by-clause-name-or-class
+ :hierarchy #'temporal-type-hierarchy)
(defmethod temporal-type LocalDate [_] :date)
(defmethod temporal-type LocalTime [_] :time)
(defmethod temporal-type OffsetTime [_] :time)
(defmethod temporal-type LocalDateTime [_] :datetime)
-(defmethod temporal-type OffsetDateTime [_] :datetime)
-(defmethod temporal-type ZonedDateTime [_] :datetime)
+(defmethod temporal-type OffsetDateTime [_] :timestamp)
+(defmethod temporal-type ZonedDateTime [_] :timestamp)
(defn- base-type->temporal-type [base-type]
(condp #(isa? %2 %1) base-type
- :type/Date :date
- :type/Time :time
- :type/DateTime :datetime
+ :type/Date :date
+ :type/Time :time
+ :type/DateTimeWithTZ :timestamp
+ :type/DateTime :datetime
nil))
(defmethod temporal-type (class Field)
- [{base-type :base_type}]
- (base-type->temporal-type base-type))
+ [{base-type :base_type, database-type :database_type}]
+ (case database-type
+ "TIMESTAMP" :timestamp
+ "DATETIME" :datetime
+ "DATE" :date
+ "TIME" :time
+ (base-type->temporal-type base-type)))
(defmethod temporal-type :absolute-datetime
- [[_ t unit]]
+ [[_ t _]]
(temporal-type t))
(defmethod temporal-type :time
[_]
:time)
+(defmethod temporal-type :datetime-field
+ [[_ field unit]]
+ ;; date extraction operations result in integers, so the type of the expression shouldn't be a temporal type
+ (if (u.date/extract-units unit)
+ nil
+ (temporal-type field)))
+
(defmethod temporal-type :default
[x]
- (or (mbql.u/match-one x
- [:field-id id] (temporal-type (qp.store/field id))
- [:field-literal _ base-type] (base-type->temporal-type base-type))
- (:mbql/temporal-type (meta x))))
+ (if (contains? (meta x) :bigquery/temporal-type)
+ (:bigquery/temporal-type (meta x))
+ (mbql.u/match-one x
+ [:field-id id] (temporal-type (qp.store/field id))
+ [:field-literal _ base-type] (base-type->temporal-type base-type))))
(defmulti ^:private ->temporal-type
{:arglists '([target-type x])}
- (fn [t-type x]
- [t-type (mbql.u/dispatch-by-clause-name-or-class x)]))
-
-(defmethod ->temporal-type [:date LocalDate] [_ t] t)
-(defmethod ->temporal-type [:time LocalDate] [_ t] (throw (UnsupportedOperationException. (tru "Cannot convert date to time"))))
-(defmethod ->temporal-type [:datetime LocalDate] [_ t] (t/local-date-time t (t/local-time 0)))
-(defmethod ->temporal-type [:date LocalTime] [_ t] (throw (UnsupportedOperationException. (tru "Cannot convert time to a date"))))
-(defmethod ->temporal-type [:time LocalTime] [_ t] t)
-(defmethod ->temporal-type [:datetime LocalTime] [_ t] (throw (UnsupportedOperationException. (tru "Cannot convert time to a datetime"))))
-(defmethod ->temporal-type [:date OffsetTime] [_ t] (t/local-date t))
-(defmethod ->temporal-type [:time OffsetTime] [_ t] (t/local-time t))
-(defmethod ->temporal-type [:datetime OffsetTime] [_ t] t)
-(defmethod ->temporal-type [:date LocalDateTime] [_ t] (t/local-date t))
-(defmethod ->temporal-type [:time LocalDateTime] [_ t] (t/local-time t))
-(defmethod ->temporal-type [:datetime LocalDateTime] [_ t] t)
-(defmethod ->temporal-type [:date OffsetDateTime] [_ t] (t/local-date t))
-(defmethod ->temporal-type [:time OffsetDateTime] [_ t] (t/local-time t))
-(defmethod ->temporal-type [:datetime OffsetDateTime] [_ t] t)
-(defmethod ->temporal-type [:date ZonedDateTime] [_ t] (t/local-date t))
-(defmethod ->temporal-type [:time ZonedDateTime] [_ t] (t/local-time t))
-(defmethod ->temporal-type [:datetime ZonedDateTime] [_ t] t)
+ (fn [target-type x]
+ [target-type (mbql.u/dispatch-by-clause-name-or-class x)])
+ :hierarchy #'temporal-type-hierarchy)
+
+(defn- throw-unsupported-conversion [from to]
+ (throw (ex-info (tru "Cannot convert a {0} to a {1}" from to)
+ {:type error-type/invalid-query})))
+
+(defmethod ->temporal-type [:date LocalTime] [_ t] (throw-unsupported-conversion "time" "date"))
+(defmethod ->temporal-type [:date OffsetTime] [_ t] (throw-unsupported-conversion "time" "date"))
+(defmethod ->temporal-type [:date LocalDate] [_ t] t)
+(defmethod ->temporal-type [:date LocalDateTime] [_ t] (t/local-date t))
+(defmethod ->temporal-type [:date OffsetDateTime] [_ t] (t/local-date t))
+(defmethod ->temporal-type [:date ZonedDateTime] [_ t] (t/local-date t))
+
+(defmethod ->temporal-type [:time LocalTime] [_ t] t)
+(defmethod ->temporal-type [:time OffsetTime] [_ t] (t/local-time t))
+(defmethod ->temporal-type [:time LocalDate] [_ t] (throw-unsupported-conversion "date" "time"))
+(defmethod ->temporal-type [:time LocalDateTime] [_ t] (t/local-time t))
+(defmethod ->temporal-type [:time OffsetDateTime] [_ t] (t/local-time t))
+(defmethod ->temporal-type [:time ZonedDateTime] [_ t] (t/local-time t))
+
+(defmethod ->temporal-type [:datetime LocalTime] [_ t] (throw-unsupported-conversion "time" "datetime"))
+(defmethod ->temporal-type [:datetime OffsetTime] [_ t] (throw-unsupported-conversion "time" "datetime"))
+(defmethod ->temporal-type [:datetime LocalDate] [_ t] (t/local-date-time t (t/local-time 0)))
+(defmethod ->temporal-type [:datetime LocalDateTime] [_ t] t)
+(defmethod ->temporal-type [:datetime OffsetDateTime] [_ t] (t/local-date-time t))
+(defmethod ->temporal-type [:datetime ZonedDateTime] [_ t] (t/local-date-time t))
+
+;; Not sure whether we should be converting local dates/datetimes to ones with UTC timezone or with the report timezone?
+(defmethod ->temporal-type [:timestamp LocalTime] [_ t] (throw-unsupported-conversion "time" "timestamp"))
+(defmethod ->temporal-type [:timestamp OffsetTime] [_ t] (throw-unsupported-conversion "time" "timestamp"))
+(defmethod ->temporal-type [:timestamp LocalDate] [_ t] (t/zoned-date-time t (t/local-time 0) (t/zone-id "UTC")))
+(defmethod ->temporal-type [:timestamp LocalDateTime] [_ t] (t/zoned-date-time t (t/zone-id "UTC")))
+(defmethod ->temporal-type [:timestamp OffsetDateTime] [_ t] t)
+(defmethod ->temporal-type [:timestamp ZonedDateTime] [_ t] t)
(defmethod ->temporal-type :default
- [t-type x]
- (if (= (temporal-type x) t-type)
- x
- (case t-type
- nil x
- :date (vary-meta (hx/cast :date (sql.qp/->honeysql :bigquery x)) assoc :mbql/temporal-type :date)
- :time (vary-meta (hx/cast :time (sql.qp/->honeysql :bigquery x)) assoc :mbql/temporal-type :time)
- :datetime (vary-meta (hx/cast :timestamp (sql.qp/->honeysql :bigquery x)) assoc :mbql/temporal-type :timestamp))))
+ [target-type x]
+ (cond
+ (nil? x)
+ nil
+
+ (= (temporal-type x) target-type)
+ (vary-meta x assoc :bigquery/temporal-type target-type)
+
+ :else
+ (let [hsql-form (sql.qp/->honeysql :bigquery x)
+ bigquery-type (case target-type
+ :date :date
+ :time :time
+ :datetime :datetime
+ :timestamp :timestamp
+ nil)]
+ (cond
+ (nil? hsql-form)
+ nil
+
+ (= (temporal-type hsql-form) target-type)
+ (vary-meta hsql-form assoc :bigquery/temporal-type target-type)
+
+ bigquery-type
+ (do
+ (log/tracef "Casting %s (temporal type = %s) to %s" (binding [*print-meta* true] (pr-str x)) (temporal-type x) bigquery-type)
+ (with-meta (hx/cast bigquery-type (sql.qp/->honeysql :bigquery x))
+ {:bigquery/temporal-type target-type}))
+
+ :else
+ x))))
+
+(defmethod ->temporal-type [:temporal-type :absolute-datetime]
+ [target-type [_ t unit]]
+ [:absolute-datetime (->temporal-type target-type t) unit])
(defn- trunc
- "Generate a SQL call to `timestamp_truc`, `date_trunc`, or `time_trunc` (depending on the `temporal-type` of `expr`)."
+ "Generate a SQL call an appropriate truncation function, depending on the temporal type of `expr`."
[unit expr]
(let [expr-type (or (temporal-type expr) :datetime)
f (case expr-type
- :date :date_trunc
- :time :time_trunc
- :datetime :timestamp_trunc)]
- (hsql/call f (->temporal-type expr-type expr) (hsql/raw (name unit)))))
+ :date :date_trunc
+ :time :time_trunc
+ :datetime :datetime_trunc
+ :timestamp :timestamp_trunc)]
+ (with-meta (hsql/call f (->temporal-type expr-type expr) (hsql/raw (name unit)))
+ {:bigquery/temporal-type expr-type})))
(defn- extract [unit expr]
- ;; implemenation of extract() in `metabase.util.honeysql-extensions` handles actual conversion to raw SQL (!)
- (hsql/call :extract unit (hx/->timestamp expr)))
+ (with-meta (hsql/call :extract unit (->temporal-type :timestamp expr))
+ {:bigquery/temporal-type nil}))
(defmethod sql.qp/date [:bigquery :minute] [_ _ expr] (trunc :minute expr))
(defmethod sql.qp/date [:bigquery :minute-of-hour] [_ _ expr] (extract :minute expr))
@@ -206,13 +273,11 @@
(defmethod sql.qp/date [:bigquery :quarter-of-year] [_ _ expr] (extract :quarter expr))
(defmethod sql.qp/date [:bigquery :year] [_ _ expr] (trunc :year expr))
-(defmethod sql.qp/unix-timestamp->timestamp [:bigquery :seconds]
- [_ _ expr]
- (hsql/call :timestamp_seconds expr))
-
-(defmethod sql.qp/unix-timestamp->timestamp [:bigquery :milliseconds]
- [_ _ expr]
- (hsql/call :timestamp_millis expr))
+(doseq [[unix-timestamp-type bigquery-fn] {:seconds :timestamp_seconds
+ :milliseconds :timestamp_millis}]
+ (defmethod sql.qp/unix-timestamp->timestamp [:bigquery unix-timestamp-type]
+ [_ _ expr]
+ (vary-meta (hsql/call bigquery-fn expr) assoc :bigquery/temporal-type :timestamp)))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -245,6 +310,12 @@
(>= (count components) 2))
true))
+(defmethod sql.qp/->honeysql [:bigquery (class Field)]
+ [driver field]
+ (let [parent-method (get-method sql.qp/->honeysql [:sql (class Field)])
+ identifier (parent-method driver field)]
+ (vary-meta identifier assoc :bigquery/temporal-type (temporal-type field))))
+
(defmethod sql.qp/->honeysql [:bigquery Identifier]
[_ identifier]
(if-not (should-qualify-identifier? identifier)
@@ -255,6 +326,12 @@
more)))
(vary-meta assoc ::already-qualified? true))))
+(doseq [clause-type [:datetime-field :field-literal :field-id]]
+ (defmethod sql.qp/->honeysql [:bigquery clause-type]
+ [driver clause]
+ (let [hsql-form ((get-method sql.qp/->honeysql [:sql clause-type]) driver clause)]
+ (vary-meta hsql-form assoc :bigquery/temporal-type (temporal-type clause)))))
+
(s/defn ^:private honeysql-form->sql :- s/Str
[driver, honeysql-form :- su/Map]
(let [[sql & args :as sql+args] (sql.qp/format-honeysql driver honeysql-form)]
@@ -325,8 +402,7 @@
;; TODO - we should make sure these are in the QP store somewhere and then could at least batch the calls
(let [table-name (db/select-one-field :name table/Table :id (u/get-id table-id))]
(with-meta (hx/identifier :field table-name field-name)
- ;; EXPERIMENTAL
- {:mbql/temporal-type (temporal-type field)})))
+ {:bigquery/temporal-type (temporal-type field)})))
(defmethod sql.qp/apply-top-level-clause [:bigquery :breakout]
[driver _ honeysql-form {breakouts :breakout, fields :fields}]
@@ -353,16 +429,23 @@
(defmethod sql.qp/->honeysql [:bigquery :asc] [driver clause] (alias-order-by-field driver clause))
(defmethod sql.qp/->honeysql [:bigquery :desc] [driver clause] (alias-order-by-field driver clause))
-;; because between clauses are generated for SQL params we need to be careful that the temporal types line up
-(defmethod sql.qp/->honeysql [:bigquery :between]
- [driver [_ f x y :as clause]]
- ((get-method sql.qp/->honeysql [:sql :between])
- driver
- (if-let [f-type (or (temporal-type f) (temporal-type x))]
- (do
- (log/tracef "Coercing args in %s to temporal type %s" (binding [*print-meta* true] (pr-str clause)) f-type)
- [:between (->temporal-type f-type f) (->temporal-type f-type x) (->temporal-type f-type y)])
- clause)))
+(defn- reconcile-temporal-types
+ "Make sure the temporal types of fields and values in filter clauses line up."
+ [[clause-type f & args :as clause]]
+ (if-let [target-type (or (temporal-type f) (some temporal-type args))]
+ (do
+ (log/tracef "Coercing args in %s to temporal type %s" (binding [*print-meta* true] (pr-str clause)) target-type)
+ (u/prog1 (into [clause-type] (map (partial ->temporal-type target-type) (cons f args)))
+ (when-not (= clause <>)
+ (log/tracef "Coerced -> %s" (binding [*print-meta* true] (pr-str <>))))))
+ clause))
+
+(doseq [filter-type [:between := :!= :> :>= :< :<=]]
+ (defmethod sql.qp/->honeysql [:bigquery filter-type]
+ [driver clause]
+ ((get-method sql.qp/->honeysql [:sql filter-type])
+ driver
+ (reconcile-temporal-types clause))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -370,8 +453,17 @@
;;; +----------------------------------------------------------------------------------------------------------------+
(defmethod driver/date-add :bigquery
- [_ dt amount unit]
- (hsql/call :datetime_add (hx/->datetime dt) (hsql/raw (format "INTERVAL %d %s" (int amount) (name unit)))))
+ [driver expr amount unit]
+ (let [add-fn (case (temporal-type expr)
+ :timestamp :timestamp_add
+ :datetime :datetime_add
+ :date :date_add
+ :time :time_add
+ nil)]
+ (if-not add-fn
+ (driver/date-add driver (->temporal-type :datetime expr) amount unit)
+ (with-meta (hsql/call add-fn expr (hsql/raw (format "INTERVAL %d %s" (int amount) (name unit))))
+ {:bigquery/temporal-type (temporal-type expr)}))))
(defmethod driver/mbql->native :bigquery
[driver
@@ -392,7 +484,7 @@
(defmethod sql.qp/current-datetime-fn :bigquery
[_]
- :%current_timestamp)
+ (with-meta (hsql/call :current_timestamp) {:bigquery/temporal-type :timestamp}))
(defmethod sql.qp/quote-style :bigquery
[_]
diff --git a/modules/drivers/bigquery/test/metabase/driver/bigquery/query_processor_test.clj b/modules/drivers/bigquery/test/metabase/driver/bigquery/query_processor_test.clj
index ddb87a3d056e30ae741de9445ab77112cc4a09dd..caeb7bf28197dfae66e1a15a0822cef4a08ee0c8 100644
--- a/modules/drivers/bigquery/test/metabase/driver/bigquery/query_processor_test.clj
+++ b/modules/drivers/bigquery/test/metabase/driver/bigquery/query_processor_test.clj
@@ -4,27 +4,21 @@
[java-time :as t]
[metabase
[driver :as driver]
+ [models :refer [Database Field]]
[query-processor :as qp]
[query-processor-test :as qp.test]
+ [test :as mt]
[util :as u]]
[metabase.driver.bigquery :as bigquery]
+ [metabase.driver.bigquery.query-processor :as bigquery.qp]
[metabase.driver.sql.query-processor :as sql.qp]
- [metabase.models
- [database :refer [Database]]
- [field :refer [Field]]]
- [metabase.query-processor
- [store :as qp.store]
- [test-util :as qp.test-util]]
- [metabase.test
- [data :as data]
- [util :as tu]]
- [metabase.test.data.datasets :as datasets]
+ [metabase.query-processor.store :as qp.store]
[metabase.test.util.timezone :as tu.tz]
[metabase.util.honeysql-extensions :as hx]
[toucan.util.test :as tt]))
(deftest native-query-test
- (datasets/test-driver :bigquery
+ (mt/test-driver :bigquery
(is (= [[100]
[99]]
(get-in
@@ -34,7 +28,7 @@
"ORDER BY `test_data.venues`.`id` DESC "
"LIMIT 2;")}
:type :native
- :database (data/id)})
+ :database (mt/id)})
[:data :rows])))
(is (= [{:name "venue_id"
@@ -60,24 +54,24 @@
"FROM `test_data.checkins` "
"LIMIT 2")}
:type :native
- :database (data/id)})))
+ :database (mt/id)})))
(str "make sure that BigQuery native queries maintain the column ordering specified in the SQL -- "
"post-processing ordering shouldn't apply (Issue #2821)"))))
(deftest aggregations-test
- (datasets/test-driver :bigquery
+ (mt/test-driver :bigquery
(testing (str "make sure queries with two or more of the same aggregation type still work. Aggregations used to be "
"deduplicated here in the BigQuery driver; now they are deduplicated as part of the main QP "
"middleware, but no reason not to keep a few of these tests just to be safe")
(let [{:keys [rows columns]} (qp.test/rows+column-names
- (data/run-mbql-query checkins
+ (mt/run-mbql-query checkins
{:aggregation [[:sum $user_id] [:sum $user_id]]}))]
(is (= ["sum" "sum_2"]
columns))
(is (= [[7929 7929]]
rows)))
(let [{:keys [rows columns]} (qp.test/rows+column-names
- (data/run-mbql-query checkins
+ (mt/run-mbql-query checkins
{:aggregation [[:sum $user_id] [:sum $user_id] [:sum $user_id]]}))]
(is (= ["sum" "sum_2" "sum_3"]
columns))
@@ -92,10 +86,10 @@
:from [(hx/identifier :table "test_data.venues")]
:group-by [(hx/identifier :field-alias "price")]
:order-by [[(hx/identifier :field-alias "avg") :asc]]}
- (qp.test-util/with-everything-store
+ (mt/with-everything-store
(#'sql.qp/mbql->honeysql
:bigquery
- (data/mbql-query venues
+ (mt/mbql-query venues
{:aggregation [[:avg $category_id]]
:breakout [$price]
:order-by [[:asc [:aggregation 0]]]})))))
@@ -108,11 +102,11 @@
:table-name "venues"
:mbql? true}
(qp/query->native
- (data/mbql-query venues
+ (mt/mbql-query venues
{:aggregation [[:avg $category_id]], :breakout [$price], :order-by [[:asc [:aggregation 0]]]})))))))
(deftest join-alias-test
- (datasets/test-driver :bigquery
+ (mt/test-driver :bigquery
(is (= (str "SELECT `categories__via__category_id`.`name` AS `name`,"
" count(*) AS `count` "
"FROM `test_data.venues` "
@@ -123,9 +117,9 @@
;; normally for test purposes BigQuery doesn't support foreign keys so override the function that checks
;; that and make it return `true` so this test proceeds as expected
(with-redefs [driver/supports? (constantly true)]
- (tu/with-temp-vals-in-db Field (data/id :venues :category_id) {:fk_target_field_id (data/id :categories :id)
+ (mt/with-temp-vals-in-db Field (mt/id :venues :category_id) {:fk_target_field_id (mt/id :categories :id)
:special_type "type/FK"}
- (let [results (data/run-mbql-query venues
+ (let [results (mt/run-mbql-query venues
{:aggregation [:count]
:breakout [$category_id->categories.name]})]
(get-in results [:data :native_form :query] results)))))
@@ -143,15 +137,15 @@
ffirst))
(deftest parsed-date-timezone-handling-test
- (datasets/test-driver :bigquery
+ (mt/test-driver :bigquery
(is (= "2018-08-31T00:00:00Z"
- (native-timestamp-query (data/id) "2018-08-31 00:00:00" "UTC"))
+ (native-timestamp-query (mt/id) "2018-08-31 00:00:00" "UTC"))
"A UTC date is returned, we should read/return it as UTC")
(is (= "2018-08-31T00:00:00-05:00"
(tu.tz/with-system-timezone-id "America/Chicago"
(tt/with-temp* [Database [db {:engine :bigquery
- :details (assoc (:details (data/db))
+ :details (assoc (:details (mt/db))
:use-jvm-timezone true)}]]
(native-timestamp-query db "2018-08-31 00:00:00-05" "America/Chicago"))))
(str "This test includes a `use-jvm-timezone` flag of true that will assume that the date coming from BigQuery "
@@ -161,7 +155,7 @@
(is (= "2018-08-31T00:00:00+07:00"
(tu.tz/with-system-timezone-id "Asia/Jakarta"
(tt/with-temp* [Database [db {:engine :bigquery
- :details (assoc (:details (data/db))
+ :details (assoc (:details (mt/db))
:use-jvm-timezone true)}]]
(native-timestamp-query db "2018-08-31 00:00:00+07" "Asia/Jakarta"))))
"Similar to the above test, but covers a positive offset")))
@@ -173,16 +167,16 @@
(with-redefs [bigquery/process-native* (fn [_ sql]
(reset! native-query sql)
(throw (Exception. "Done.")))]
- (qp/process-query {:database (data/id)
+ (qp/process-query {:database (mt/id)
:type :query
- :query {:source-table (data/id :venues)
+ :query {:source-table (mt/id :venues)
:limit 1}
:info {:executed-by 1000
:query-hash (byte-array [1 2 3 4])}})
@native-query)))
(deftest remark-test
- (datasets/test-driver :bigquery
+ (mt/test-driver :bigquery
(is (= (str
"-- Metabase:: userID: 1000 queryType: MBQL queryHash: 01020304\n"
"SELECT `test_data.venues`.`id` AS `id`,"
@@ -194,20 +188,20 @@
"FROM `test_data.venues` "
"LIMIT 1")
(query->native
- {:database (data/id)
+ {:database (mt/id)
:type :query
- :query {:source-table (data/id :venues)
+ :query {:source-table (mt/id :venues)
:limit 1}
:info {:executed-by 1000
:query-hash (byte-array [1 2 3 4])}}))
"if I run a BigQuery query, does it get a remark added to it?")))
(deftest unprepare-params-test
- (datasets/test-driver :bigquery
+ (mt/test-driver :bigquery
(is (= [["Red Medicine"]]
(qp.test/rows
(qp/process-query
- {:database (data/id)
+ {:database (mt/id)
:type :native
:native {:query (str "SELECT `test_data.venues`.`name` AS `name` "
"FROM `test_data.venues` "
@@ -216,17 +210,110 @@
(str "Do we properly unprepare, and can we execute, queries that still have parameters for one reason or "
"another? (EE #277)"))))
+(def ^:private reconcile-test-values
+ [{:value (t/local-date "2019-12-10")
+ :type :date
+ :as {:datetime (t/local-date-time "2019-12-10T00:00:00")
+ :timestamp (t/zoned-date-time "2019-12-10T00:00:00Z[UTC]")}}
+ {:value (t/local-date-time "2019-12-10T14:47:00")
+ :type :datetime
+ :as {:date (t/local-date "2019-12-10")
+ :timestamp (t/zoned-date-time "2019-12-10T14:47:00Z[UTC]")}}
+ {:value (t/zoned-date-time "2019-12-10T14:47:00Z[UTC]")
+ :type :timestamp
+ :as {:date (t/local-date "2019-12-10")
+ :datetime (t/local-date-time "2019-12-10T14:47:00")}}
+ {:value (t/offset-date-time "2019-12-10T14:47:00Z")
+ :type :timestamp
+ :as {:date (t/local-date "2019-12-10")
+ :datetime (t/local-date-time "2019-12-10T14:47:00")}}
+ (let [unix-ts (sql.qp/unix-timestamp->timestamp :bigquery :seconds :some_field)]
+ {:value unix-ts
+ :type :timestamp
+ :as {:date (hx/cast :date unix-ts)
+ :datetime (hx/cast :datetime unix-ts)}})
+ (let [unix-ts (sql.qp/unix-timestamp->timestamp :bigquery :milliseconds :some_field)]
+ {:value unix-ts
+ :type :timestamp
+ :as {:date (hx/cast :date unix-ts)
+ :datetime (hx/cast :datetime unix-ts)}})])
+
+(deftest temporal-type-test
+ (testing "Make sure we can detect temporal types correctly"
+ (doseq [[expr expected-type] {[:field-literal "x" :type/DateTime] :datetime
+ [:datetime-field [:field-literal "x" :type/DateTime] :day-of-week] nil}]
+ (testing (format "\n(temporal-type %s)" (binding [*print-meta* true] (pr-str expr)))
+ (is (= expected-type
+ (#'bigquery.qp/temporal-type expr)))))))
+
+(deftest reconcile-temporal-types-test
+ (mt/with-everything-store
+ (tt/with-temp* [Field [date-field {:name "date", :base_type :type/Date}]
+ Field [datetime-field {:name "datetime", :base_type :type/DateTime}]
+ Field [timestamp-field {:name "timestamp", :base_type :type/DateTimeWithLocalTZ}]]
+ ;; bind `*table-alias*` so the BigQuery QP doesn't try to look up the current dataset name when converting
+ ;; `hx/identifier`s to SQL
+ (binding [sql.qp/*table-alias* "ABC"
+ *print-meta* true]
+ (let [fields {:date date-field
+ :datetime datetime-field
+ :timestamp timestamp-field}]
+ (doseq [clause [{:args 2, :mbql :=, :honeysql :=}
+ {:args 2, :mbql :!=, :honeysql :not=}
+ {:args 2, :mbql :>, :honeysql :>}
+ {:args 2, :mbql :>=, :honeysql :>=}
+ {:args 2, :mbql :<, :honeysql :<}
+ {:args 2, :mbql :<=, :honeysql :<=}
+ {:args 3, :mbql :between, :honeysql :between}]]
+ (testing (format "\n%s filter clause" (:mbql clause))
+ (doseq [[temporal-type field] fields
+ field [field
+ [:field-id (:id field)]
+ [:datetime-field [:field-id (:id field)] :default]
+ [:field-literal (:name field) (:base_type field)]
+ [:datetime-field [:field-literal (:name field) (:base_type field)] :default]]]
+ (testing (format "\nField = %s %s"
+ temporal-type
+ (if (map? field) (format "<Field %s>" (pr-str (:name field))) field))
+ (doseq [{filter-value :value, :as value} reconcile-test-values
+ filter-value (cons filter-value
+ (when (instance? java.time.temporal.Temporal filter-value)
+ [[:absolute-datetime filter-value :default]]))]
+ (testing (format "\nValue = %s %s" (:type value) (pr-str filter-value))
+ (let [filter-clause (into [(:mbql clause) field]
+ (repeat (dec (:args clause)) filter-value))
+ expected-identifier (hx/identifier :field "ABC" (name temporal-type))
+ expected-value (get-in value [:as temporal-type] (:value value))
+ expected-clause (into [(:honeysql clause) expected-identifier]
+ (repeat (dec (:args clause)) expected-value))]
+ (testing (format "\nreconcile %s -> %s"
+ (into [(:mbql clause) temporal-type] (repeat (dec (:args clause)) (:type value)))
+ (into [(:mbql clause) temporal-type] (repeat (dec (:args clause)) temporal-type)))
+ (testing (format "\ninferred field type = %s, inferred value type = %s"
+ (#'bigquery.qp/temporal-type field)
+ (#'bigquery.qp/temporal-type filter-value))
+ (is (= expected-clause
+ (sql.qp/->honeysql :bigquery filter-clause))))))))))))
+ (testing "\ndate extraction filters"
+ (doseq [[temporal-type field] fields
+ :let [identifier (hx/identifier :field "ABC" (name temporal-type))
+ expected-identifier (if (= temporal-type :timestamp)
+ identifier
+ (hx/cast :timestamp identifier))]]
+ (is (= [:= (hsql/call :extract :dayofweek expected-identifier) 1]
+ (sql.qp/->honeysql :bigquery [:= [:datetime-field [:field-id (:id field)] :day-of-week] 1]))))))))))
+
(deftest between-test
(testing "Make sure :between clauses reconcile the temporal types of their args"
(letfn [(between->sql [clause]
(sql.qp/format-honeysql :bigquery
{:where (sql.qp/->honeysql :bigquery clause)}))]
- (testing "Should look for `:mbql/temporal-type` metadata"
+ (testing "Should look for `:bigquery/temporal-type` metadata"
(is (= ["WHERE field BETWEEN ? AND ?"
(t/local-date-time "2019-11-11T00:00")
(t/local-date-time "2019-11-12T00:00")]
(between->sql [:between
- (with-meta (hsql/raw "field") {:mbql/temporal-type :datetime})
+ (with-meta (hsql/raw "field") {:bigquery/temporal-type :datetime})
(t/local-date "2019-11-11")
(t/local-date "2019-11-12")]))))
(testing "If first arg has no temporal-type info, should look at next arg"
@@ -242,31 +329,31 @@
(t/local-date "2019-11-11")
(t/local-date "2019-11-12")]
(between->sql [:between
- (with-meta (hsql/raw "field") {:mbql/temporal-type :date})
+ (with-meta (hsql/raw "field") {:bigquery/temporal-type :date})
(t/local-date "2019-11-11")
(t/local-date "2019-11-12")]))))
- (datasets/test-driver :bigquery
- (qp.test-util/with-everything-store
+ (mt/test-driver :bigquery
+ (mt/with-everything-store
(let [expected ["WHERE `test_data.checkins`.`date` BETWEEN ? AND ?"
- (t/local-date-time "2019-11-11T00:00")
- (t/local-date-time "2019-11-12T00:00")]]
+ (t/zoned-date-time "2019-11-11T00:00Z[UTC]")
+ (t/zoned-date-time "2019-11-12T00:00Z[UTC]")]]
(testing "Should be able to get temporal type from a FieldInstance"
(is (= expected
(between->sql [:between
- (qp.store/field (data/id :checkins :date))
+ (qp.store/field (mt/id :checkins :date))
(t/local-date "2019-11-11")
(t/local-date "2019-11-12")]))))
(testing "Should be able to get temporal type from a :field-id"
(is (= expected
(between->sql [:between
- [:field-id (data/id :checkins :date)]
+ [:field-id (mt/id :checkins :date)]
(t/local-date "2019-11-11")
(t/local-date "2019-11-12")]))))
(testing "Should be able to get temporal type from a wrapped field-id"
- (is (= (cons "WHERE timestamp_trunc(CAST(`test_data.checkins`.`date` AS timestamp), day) BETWEEN ? AND ?"
+ (is (= (cons "WHERE timestamp_trunc(`test_data.checkins`.`date`, day) BETWEEN ? AND ?"
(rest expected))
(between->sql [:between
- [:datetime-field [:field-id (data/id :checkins :date)] :day]
+ [:datetime-field [:field-id (mt/id :checkins :date)] :day]
(t/local-date "2019-11-11")
(t/local-date "2019-11-12")]))))
(testing "Should work with a field literal"
diff --git a/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj b/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
index 67f3f85a5a4e2cbc603521cf9f6b1fef5eddb5f7..6eb1fdc1d7d8e02ea62692d3197a2101a7d752d9 100644
--- a/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
+++ b/modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
@@ -2,13 +2,11 @@
(:require [clojure.test :refer :all]
[metabase
[driver :as driver]
+ [models :refer [Field Table]]
[query-processor :as qp]
[query-processor-test :as qp.test]
[sync :as sync]]
[metabase.db.metadata-queries :as metadata-queries]
- [metabase.models
- [field :refer [Field]]
- [table :refer [Table]]]
[metabase.test
[data :as data]
[util :as tu]]
@@ -61,13 +59,8 @@
(deftest sync-views-test
(datasets/test-driver :bigquery
(with-view [view-name]
- (is (= {:tables
- #{{:schema nil, :name "categories"}
- {:schema nil, :name "checkins"}
- {:schema nil, :name "users"}
- {:schema nil, :name "venues"}
- {:schema nil, :name view-name}}}
- (driver/describe-database :bigquery (data/db)))
+ (is (contains? (:tables (driver/describe-database :bigquery (data/db)))
+ {:schema nil, :name view-name})
"`describe-database` should see the view")
(is (= {:schema nil
:name view-name
@@ -99,3 +92,38 @@
{:fields [$id $date]
:filter [:= $date "2015-11-19"]
:order-by [[:asc $id]]})))))))))
+
+(defn- do-with-datetime-timestamp-table [f]
+ (driver/with-driver :bigquery
+ (let [table-name (name (munge (gensym "table_")))]
+ (data/with-temp-copy-of-db
+ (try
+ (bigquery.tx/execute!
+ (format "CREATE TABLE `test_data.%s` ( ts TIMESTAMP, dt DATETIME )" table-name))
+ (bigquery.tx/execute!
+ (format "INSERT INTO `test_data.%s` (ts, dt) VALUES (TIMESTAMP \"2020-01-01 00:00:00 UTC\", DATETIME \"2020-01-01 00:00:00\")"
+ table-name))
+ (sync/sync-database! (data/db))
+ (f table-name)
+ (finally
+ (bigquery.tx/execute! "DROP TABLE IF EXISTS `test_data.%s`" table-name)))))))
+
+(deftest filter-by-datetime-timestamp-test
+ (datasets/test-driver :bigquery
+ ;; there are more tests in the `bigquery.query-processor-test` namespace
+ (testing "Make sure we can filter against different types of BigQuery temporal columns (#11222)"
+ (do-with-datetime-timestamp-table
+ (fn [table-name]
+ (doseq [column [:ts :dt]]
+ (testing (format "Filtering against %s column" column)
+ (doseq [s ["2020-01-01" "2020-01-01T00:00:00"]
+ field [[:field-id (data/id table-name column)]
+ [:datetime-field [:field-id (data/id table-name column)] :default]
+ [:datetime-field [:field-id (data/id table-name column)] :day]]
+ :let [filter-clause [:= field s]]]
+ (testing (format "\nMBQL filter clause = %s" (pr-str filter-clause))
+ (is (= [["2020-01-01T00:00:00Z" "2020-01-01T00:00:00Z"]]
+ (qp.test/rows
+ (data/run-mbql-query nil
+ {:source-table (data/id table-name)
+ :filter filter-clause})))))))))))))
diff --git a/src/metabase/driver/sql/query_processor.clj b/src/metabase/driver/sql/query_processor.clj
index f07ef47d69b8cf1d6b162eff677a4c8ffcb4b959..4d320f4805cda314d5b57601ae0db6f17131853f 100644
--- a/src/metabase/driver/sql/query_processor.clj
+++ b/src/metabase/driver/sql/query_processor.clj
@@ -444,45 +444,56 @@
[value :- (s/constrained mbql.s/value #(string? (second %)) "string value"), f]
(update value 1 f))
-(defmethod ->honeysql [:sql :starts-with] [driver [_ field value options]]
+(defmethod ->honeysql [:sql :starts-with]
+ [driver [_ field value options]]
(like-clause driver (->honeysql driver field) (update-string-value value #(str % \%)) options))
-(defmethod ->honeysql [:sql :contains] [driver [_ field value options]]
+(defmethod ->honeysql [:sql :contains]
+ [driver [_ field value options]]
(like-clause driver (->honeysql driver field) (update-string-value value #(str \% % \%)) options))
-(defmethod ->honeysql [:sql :ends-with] [driver [_ field value options]]
+(defmethod ->honeysql [:sql :ends-with]
+ [driver [_ field value options]]
(like-clause driver (->honeysql driver field) (update-string-value value #(str \% %)) options))
-(defmethod ->honeysql [:sql :between] [driver [_ field min-val max-val]]
+(defmethod ->honeysql [:sql :between]
+ [driver [_ field min-val max-val]]
[:between (->honeysql driver field) (->honeysql driver min-val) (->honeysql driver max-val)])
-
-(defmethod ->honeysql [:sql :>] [driver [_ field value]]
+(defmethod ->honeysql [:sql :>]
+ [driver [_ field value]]
[:> (->honeysql driver field) (->honeysql driver value)])
-(defmethod ->honeysql [:sql :<] [driver [_ field value]]
+(defmethod ->honeysql [:sql :<]
+ [driver [_ field value]]
[:< (->honeysql driver field) (->honeysql driver value)])
-(defmethod ->honeysql [:sql :>=] [driver [_ field value]]
+(defmethod ->honeysql [:sql :>=]
+ [driver [_ field value]]
[:>= (->honeysql driver field) (->honeysql driver value)])
-(defmethod ->honeysql [:sql :<=] [driver [_ field value]]
+(defmethod ->honeysql [:sql :<=]
+ [driver [_ field value]]
[:<= (->honeysql driver field) (->honeysql driver value)])
-(defmethod ->honeysql [:sql :=] [driver [_ field value]]
+(defmethod ->honeysql [:sql :=]
+ [driver [_ field value]]
[:= (->honeysql driver field) (->honeysql driver value)])
-(defmethod ->honeysql [:sql :!=] [driver [_ field value]]
+(defmethod ->honeysql [:sql :!=]
+ [driver [_ field value]]
[:not= (->honeysql driver field) (->honeysql driver value)])
-
-(defmethod ->honeysql [:sql :and] [driver [_ & subclauses]]
+(defmethod ->honeysql [:sql :and]
+ [driver [_ & subclauses]]
(apply vector :and (map (partial ->honeysql driver) subclauses)))
-(defmethod ->honeysql [:sql :or] [driver [_ & subclauses]]
+(defmethod ->honeysql [:sql :or]
+ [driver [_ & subclauses]]
(apply vector :or (map (partial ->honeysql driver) subclauses)))
-(defmethod ->honeysql [:sql :not] [driver [_ subclause]]
+(defmethod ->honeysql [:sql :not]
+ [driver [_ subclause]]
[:not (->honeysql driver subclause)])
(defmethod apply-top-level-clause [:sql :filter]
diff --git a/src/metabase/plugins/classloader.clj b/src/metabase/plugins/classloader.clj
index 9e73640008c5aa29dd83934d9bf335ed11b46662..d63538927ca1d2468ca4d025718fe2beb69f87d8 100644
--- a/src/metabase/plugins/classloader.clj
+++ b/src/metabase/plugins/classloader.clj
@@ -81,7 +81,7 @@
current-thread-context-classloader))
;; otherwise set the current thread's context classloader to the shared context classloader
(let [shared-classloader @shared-context-classloader]
- (log/debug
+ (log/trace
(deferred-trs "Setting current thread context classloader to shared classloader {0}..." shared-classloader))
(.setContextClassLoader (Thread/currentThread) shared-classloader)
shared-classloader)))
diff --git a/src/metabase/util/honeysql_extensions.clj b/src/metabase/util/honeysql_extensions.clj
index a85fa3d167852f6efbf79db1409b6c0079eaef9b..61ede48659ef7d2887cae307d4965be4f1232436 100644
--- a/src/metabase/util/honeysql_extensions.clj
+++ b/src/metabase/util/honeysql_extensions.clj
@@ -186,8 +186,9 @@
(require 'honeysql.types)
(extend-protocol PrettyPrintable
honeysql.types.SqlCall
- (pretty [{fn-name :name, args :args}]
- (apply list 'hsql/call fn-name args)))
+ (pretty [{fn-name :name, args :args, :as this}]
+ (with-meta (apply list 'hsql/call fn-name args)
+ (meta this))))
(defmethod print-method honeysql.types.SqlCall
[call writer]
diff --git a/test/metabase/query_processor_test/date_bucketing_test.clj b/test/metabase/query_processor_test/date_bucketing_test.clj
index 2e9dd86faa981a96fd2d8efdcaea804ea65e79af..c2a97e5249320d3a1f7e9142ffc4c0008fef10b2 100644
--- a/test/metabase/query_processor_test/date_bucketing_test.clj
+++ b/test/metabase/query_processor_test/date_bucketing_test.clj
@@ -1057,7 +1057,7 @@
(testing "Additional tests for filtering against various datetime bucketing units that aren't tested above"
(mt/test-drivers (mt/normal-drivers)
(doseq [[expected-count unit filter-value] addition-unit-filtering-vals]
- (testing unit
+ (testing (format "\nunit = %s" unit)
(let [result (count-of-checkins unit filter-value)]
(if (integer? expected-count)
(is (= expected-count result)
diff --git a/test/metabase/query_processor_test/filter_test.clj b/test/metabase/query_processor_test/filter_test.clj
index 78a7e36905b8524f6183ce63a1e23aff118465c4..621d8f39e336c81188b1c09a119193852d55df62 100644
--- a/test/metabase/query_processor_test/filter_test.clj
+++ b/test/metabase/query_processor_test/filter_test.clj
@@ -3,7 +3,8 @@
(:require [clojure.test :refer :all]
[metabase
[driver :as driver]
- [query-processor-test :as qp.test]]
+ [query-processor-test :as qp.test]
+ [test :as mt]]
[metabase.test.data :as data]
[metabase.test.data.datasets :as datasets]))
@@ -119,34 +120,30 @@
(data/run-mbql-query venues
{:filter [:inside $latitude $longitude 10.0649 -165.379 10.0641 -165.371]})))
-;;; FILTER - `is-null` & `not-null` on datetime columns
-(qp.test/expect-with-non-timeseries-dbs
- [1000]
- (qp.test/first-row
- (qp.test/format-rows-by [int]
- (data/run-mbql-query checkins
- {:aggregation [[:count]]
- :filter [:not-null $date]}))))
-
-;; Creates a query that uses a field-literal. Normally our test queries will use a field placeholder, but
-;; https://github.com/metabase/metabase/issues/7381 is only triggered by a field literal
-(qp.test/expect-with-non-timeseries-dbs
- [1000]
- (qp.test/first-row
- (qp.test/format-rows-by [int]
- (data/run-mbql-query checkins
- {:aggregation [[:count]]
- :filter ["NOT_NULL"
- ["field-id"
- ["field-literal" (data/format-name "date") "type/DateTime"]]]}))))
-
-(qp.test/expect-with-non-timeseries-dbs
- true
- (let [result (qp.test/first-row (data/run-mbql-query checkins
- {:aggregation [[:count]]
- :filter [:is-null $date]}))]
- ;; Some DBs like Mongo don't return any results at all in this case, and there's no easy workaround
- (contains? #{[0] [0M] [nil] nil} result)))
+(deftest is-null-test
+ (mt/test-drivers (mt/normal-drivers)
+ (let [result (qp.test/first-row (data/run-mbql-query checkins
+ {:aggregation [[:count]]
+ :filter [:is-null $date]}))]
+ ;; Some DBs like Mongo don't return any results at all in this case, and there's no easy workaround
+ (is (= true
+ (contains? #{[0] [0M] [nil] nil} result))))))
+
+(deftest not-null-test
+ (mt/test-drivers (mt/normal-drivers)
+ (is (= [1000]
+ (qp.test/first-row
+ (qp.test/format-rows-by [int]
+ (data/run-mbql-query checkins
+ {:aggregation [[:count]]
+ :filter [:not-null $date]})))))
+ (testing "Make sure :not-null filters work correctly with field literals (#7381)"
+ (is (= [1000]
+ (qp.test/first-row
+ (qp.test/format-rows-by [int]
+ (data/run-mbql-query checkins
+ {:aggregation [[:count]]
+ :filter [:not-null *date]}))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/test/metabase/query_processor_test/parameters_test.clj b/test/metabase/query_processor_test/parameters_test.clj
index 103dda8d51c37a4670d48db6c54d7b3e44d6cf09..27e256dfbf0c1869e9db04dd1cf6594d5abae138 100644
--- a/test/metabase/query_processor_test/parameters_test.clj
+++ b/test/metabase/query_processor_test/parameters_test.clj
@@ -39,13 +39,14 @@
:aggregation [[:count]]
:filter (into [:and]
(for [[i [field]] (map-indexed vector field->type+value)]
- [:= (mt/id table field) i]))})
+ [:= [:field-id (mt/id table field)] i]))})
{:keys [query]} (qp/query->native mbql-query)
query (reduce
- (fn [query [i [field]]]
- (str/replace query (re-pattern (format "= %d" i)) (format "= {{%s}}" (name field))))
+ (fn [query [field]]
+ ;; TODO — currently only supports one field
+ (str/replace query (re-pattern #"= .*") (format "= {{%s}}" (name field))))
query
- (map-indexed vector field->type+value))]
+ field->type+value)]
(log/tracef "%s\n->\n%s\n->\n%s"
(pr-str (list 'native-count-query driver table field->type+value))
(pr-str mbql-query)