Skip to content
Snippets Groups Projects
Unverified Commit 320e9ab1 authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

Add logic to reconcile temporal types for BigQuery (#11492)

parent 698587aa
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 397 additions and 180 deletions
modules/drivers/bigquery/src/metabase/driver/bigquery.clj
+ 1
− 1
View file @ 320e9ab1
......@@ -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/*))
......
modules/drivers/bigquery/src/metabase/driver/bigquery/query_processor.clj
+ 165
− 73
View file @ 320e9ab1
......@@ -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
[_]
......
modules/drivers/bigquery/test/metabase/driver/bigquery/query_processor_test.clj
+ 131
− 44
View file @ 320e9ab1
......@@ -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"
......
modules/drivers/bigquery/test/metabase/driver/bigquery_test.clj
+ 38
− 10
View file @ 320e9ab1
......@@ -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})))))))))))))
src/metabase/driver/sql/query_processor.clj
+ 26
− 15
View file @ 320e9ab1
......@@ -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]
......
src/metabase/plugins/classloader.clj
+ 1
− 1
View file @ 320e9ab1
......@@ -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)))
......
src/metabase/util/honeysql_extensions.clj
+ 3
− 2
View file @ 320e9ab1
......@@ -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]
......
test/metabase/query_processor_test/date_bucketing_test.clj
+ 1
− 1
View file @ 320e9ab1
......@@ -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)
......
test/metabase/query_processor_test/filter_test.clj
+ 26
− 29
View file @ 320e9ab1
......@@ -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]}))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
......
test/metabase/query_processor_test/parameters_test.clj
+ 5
− 4
View file @ 320e9ab1
......@@ -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)
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment