Skip to content
Snippets Groups Projects
Commit 2d938fcb authored by Ryan Senior's avatar Ryan Senior
Browse files

Added BQ style test data setup. 

Still not working. The most recent error is one where Snowflake is
expecting us to declare which database we're using. The error
references including a `USE DATABASE` statement before querying. It
looks like we were trying to solve that with a connection parameter,
but it doesn't appear to be working.
parent 768d43ea
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/driver/generic_sql.clj
+ 17
− 6
View file @ 2d938fcb
......@@ -288,6 +288,12 @@
(let ~(vec (interleave (map first binding-pairs) (map #(list `~jdbc/result-set-seq %) rs-syms)))
~@body))))
(defn get-catalogs
"Returns a set of all of the catalogs found via `metadata`"
[^DatabaseMetaData metadata]
(with-resultset-open [rs-seq (.getCatalogs metadata)]
(set (map :table_cat rs-seq))))
(defn- get-tables
"Fetch a JDBC Metadata ResultSet of tables in the DB, optionally limited to ones belonging to a given schema."
^ResultSet [^DatabaseMetaData metadata, ^String schema-or-nil, ^String database-name-or-nil]
......@@ -318,7 +324,7 @@
(defn post-filtered-active-tables
"Alternative implementation of `ISQLDriver/active-tables` best suited for DBs with little or no support for schemas.
Fetch *all* Tables, then filter out ones whose schema is in `excluded-schemas` Clojure-side."
[driver, ^DatabaseMetaData metadata]
[driver, ^DatabaseMetaData metadata & [database-name-or-nil]]
(set (for [table (filter #(not (contains? (excluded-schemas driver) (:table_schem %)))
(get-tables metadata nil nil))]
(let [remarks (:remarks table)]
......@@ -343,8 +349,10 @@
(str "Invalid type: " special-type))
special-type))
(defn- describe-table-fields [^DatabaseMetaData metadata, driver, {schema :schema, table-name :name}]
(with-resultset-open [rs-seq (.getColumns metadata nil schema table-name nil)]
(defn describe-table-fields
"Returns a set of column metadata for `schema` and `table-name` using `metadata`. "
[^DatabaseMetaData metadata, driver, {schema :schema, table-name :name}, & [database-name-or-nil]]
(with-resultset-open [rs-seq (.getColumns metadata database-name-or-nil schema table-name nil)]
(set (for [{database-type :type_name, column-name :column_name, remarks :remarks} rs-seq]
(merge {:name column-name
:database-type database-type
......@@ -354,7 +362,8 @@
(when-let [special-type (calculated-special-type driver column-name database-type)]
{:special-type special-type}))))))
(defn- add-table-pks
(defn add-table-pks
"Using `metadata` find any primary keys for `table` and assoc `:pk?` to true for those columns."
[^DatabaseMetaData metadata, table]
(with-resultset-open [rs-seq (.getPrimaryKeys metadata nil nil (:name table))]
(let [pks (set (map :column_name rs-seq))]
......@@ -380,9 +389,11 @@
;; find PKs and mark them
(add-table-pks metadata))))
(defn- describe-table-fks [driver database table]
(defn describe-table-fks
"Default implementation of `describe-table-fks` for JDBC based drivers."
[driver database table & [database-name-or-nil]]
(with-metadata [metadata driver database]
(with-resultset-open [rs-seq (.getImportedKeys metadata nil (:schema table) (:name table))]
(with-resultset-open [rs-seq (.getImportedKeys metadata database-name-or-nil (:schema table) (:name table))]
(set (for [result rs-seq]
{:fk-column-name (:fkcolumn_name result)
:dest-table {:name (:pktable_name result)
......
src/metabase/driver/snowflake.clj
+ 23
− 6
View file @ 2d938fcb
......@@ -12,19 +12,20 @@
(defn connection-details->spec
"Create a database specification for a snowflake database."
[{:keys [account regionid dbname] :as opts}]
[{:keys [account regionid db] :as opts}]
(let [host (if regionid
(str account "." regionid)
account)]
(merge {:subprotocol "snowflake"
:classname "net.snowflake.client.jdbc.SnowflakeDriver"
:subname (str "//" host ".snowflakecomputing.com/")
:db dbname
;; This was dbname, not sure why as I didn't see any dbname populated, just db
:db db
:client_metadata_request_use_connection_ctx true
:ssl true
;; other SESSION parameters
:week_start 7}
(dissoc opts :host :port :dbname))))
(dissoc opts :host :port :db))))
(defrecord SnowflakeDriver []
:load-ns true
......@@ -84,7 +85,10 @@
:milliseconds (hsql/call :to_timestamp expr 3)))
(defn- date-interval [unit amount]
(hsql/raw (format "dateadd(%s, %d, current_timestamp())" (name unit) (int amount))))
(hsql/call :dateadd
(hsql/raw (name unit))
(hsql/raw (int amount))
:%current_timestamp))
(defn- extract [unit expr] (hsql/call :date_part unit (hx/->timestamp expr)))
(defn- date-trunc [unit expr] (hsql/call :date_trunc unit (hx/->timestamp expr)))
......@@ -115,6 +119,16 @@
(sql/with-metadata [metadata driver database]
{:tables (sql/fast-active-tables driver metadata (:name database))}))
(defn- describe-table [driver database table]
(sql/with-metadata [metadata driver database]
(->> (assoc (select-keys table [:name :schema])
:fields (sql/describe-table-fields metadata driver table (:name database)))
;; find PKs and mark them
(sql/add-table-pks metadata))))
(defn- describe-table-fks [driver database table]
(sql/describe-table-fks driver database table (:name database)))
(u/strict-extend SnowflakeDriver
driver/IDriver
(merge (sql/IDriverSQLDefaultsMixin)
......@@ -152,8 +166,11 @@
:current-db-time (driver/make-current-db-time-fn
snowflake-db-time-query
snowflake-date-formatters)
:format-aggregation-column-name (u/drop-first-arg str/upper-case)
:describe-database describe-database})
;; This appears to be overwritten by `format-custom-field-name`
:format-aggregation-column-name (u/drop-first-arg str/lower-case)
:describe-database describe-database
:describe-table describe-table
:describe-table-fks describe-table-fks})
sql/ISQLDriver
(merge (sql/ISQLDriverDefaultsMixin)
......
src/metabase/query_processor/middleware/annotate.clj
+ 2
− 2
View file @ 2d938fcb
......@@ -132,11 +132,11 @@
;; for unnamed normal aggregations, the column alias is always the same as the ag type except for `:distinct` with
;; is called `:count` (WHY?)
[:distinct _]
"count"
(driver/format-aggregation-column-name i/*driver* "count")
;; for any other aggregation just use the name of the clause e.g. `sum`
[clause-name & _]
(name clause-name)))
(driver/format-aggregation-column-name i/*driver* (name clause-name))))
(defn- ag->name-info [ag]
(let [ag-name (aggregation-name ag)]
......
test/metabase/query_processor_test/date_bucketing_test.clj
+ 24
− 21
View file @ 2d938fcb
......@@ -35,13 +35,14 @@
(long x)
x))
(defn- oracle-or-redshift?
"We currently have a bug in how report-timezone is used in Oracle. The timeone is applied correctly, but the date
operations that we use aren't using that timezone. It's written up as
https://github.com/metabase/metabase/issues/5789. This function is used to differentiate Oracle from the other
report-timezone databases until that bug can get fixed. Redshift also has this issue."
(defn- tz-shifted-engine-bug?
"Returns true if `engine` is affected by the bug originally observed in
Oracle (https://github.com/metabase/metabase/issues/5789) but later found in Redshift and Snowflake. The timezone is
applied correctly, but the date operations that we use aren't using that timezone. This function is used to
differentiate Oracle from the other report-timezone databases until that bug can get fixed. Redshift and Snowflake
also have this issue."
[engine]
(contains? #{:oracle :redshift} engine))
(contains? #{:snowflake :oracle :redshift} engine))
(defn- sad-toucan-incidents-with-bucketing
"Returns 10 sad toucan incidents grouped by `UNIT`"
......@@ -128,7 +129,7 @@
(sad-toucan-result (source-date-formatter utc-tz) result-date-formatter-without-tz)
;; There's a bug here where we are reading in the UTC time as pacific, so we're 7 hours off
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(sad-toucan-result (source-date-formatter pacific-tz) (result-date-formatter pacific-tz))
;; When the reporting timezone is applied, the same datetime value is returned, but set in the pacific timezone
......@@ -148,7 +149,7 @@
(contains? #{:sqlite :crate} *engine*)
(sad-toucan-result (source-date-formatter utc-tz) result-date-formatter-without-tz)
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(sad-toucan-result (source-date-formatter eastern-tz) (result-date-formatter eastern-tz))
;; The time instant is the same as UTC (or pacific) but should be offset by the eastern timezone
......@@ -172,7 +173,7 @@
(contains? #{:sqlite :crate} *engine*)
(sad-toucan-result (source-date-formatter utc-tz) result-date-formatter-without-tz)
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(sad-toucan-result (source-date-formatter eastern-tz) (result-date-formatter eastern-tz))
;; The JVM timezone should have no impact on a database that uses a report timezone
......@@ -197,7 +198,7 @@
(contains? #{:sqlite :crate} *engine*)
(sad-toucan-result (source-date-formatter utc-tz) result-date-formatter-without-tz)
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(sad-toucan-result (source-date-formatter pacific-tz) (result-date-formatter pacific-tz))
(supports-report-timezone? *engine*)
......@@ -261,7 +262,7 @@
(results-by-hour (source-date-formatter utc-tz)
result-date-formatter-without-tz)
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(results-by-hour (source-date-formatter pacific-tz) (result-date-formatter pacific-tz))
(supports-report-timezone? *engine*)
......@@ -283,7 +284,7 @@
;; first three results of the pacific results to the last three of the
;; UTC results (i.e. pacific is 7 hours back of UTC at that time)
(expect-with-non-timeseries-dbs
(if (and (not (oracle-or-redshift? *engine*))
(if (and (not (tz-shifted-engine-bug? *engine*))
(supports-report-timezone? *engine*))
[[0 8] [1 9] [2 7] [3 10] [4 10] [5 9] [6 6] [7 5] [8 7] [9 7]]
[[0 13] [1 8] [2 4] [3 7] [4 5] [5 13] [6 10] [7 8] [8 9] [9 7]])
......@@ -387,7 +388,7 @@
date-formatter-without-time
[6 10 4 9 9 8 8 9 7 9])
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(results-by-day (tformat/with-zone date-formatter-without-time pacific-tz)
(result-date-formatter pacific-tz)
[6 10 4 9 9 8 8 9 7 9])
......@@ -420,7 +421,7 @@
date-formatter-without-time
[6 10 4 9 9 8 8 9 7 9])
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(results-by-day (tformat/with-zone date-formatter-without-time eastern-tz)
(result-date-formatter eastern-tz)
[6 10 4 9 9 8 8 9 7 9])
......@@ -453,7 +454,7 @@
date-formatter-without-time
[6 10 4 9 9 8 8 9 7 9])
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(results-by-day (tformat/with-zone date-formatter-without-time pacific-tz)
(result-date-formatter pacific-tz)
[6 10 4 9 9 8 8 9 7 9])
......@@ -478,7 +479,7 @@
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(expect-with-non-timeseries-dbs
(if (and (not (oracle-or-redshift? *engine*))
(if (and (not (tz-shifted-engine-bug? *engine*))
(supports-report-timezone? *engine*))
[[1 29] [2 36] [3 33] [4 29] [5 13] [6 38] [7 22]]
[[1 28] [2 38] [3 29] [4 27] [5 24] [6 30] [7 24]])
......@@ -495,7 +496,7 @@
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(expect-with-non-timeseries-dbs
(if (and (not (oracle-or-redshift? *engine*))
(if (and (not (tz-shifted-engine-bug? *engine*))
(supports-report-timezone? *engine*))
[[1 8] [2 9] [3 9] [4 4] [5 11] [6 8] [7 6] [8 10] [9 6] [10 10]]
[[1 6] [2 10] [3 4] [4 9] [5 9] [6 8] [7 8] [8 9] [9 7] [10 9]])
......@@ -512,7 +513,7 @@
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(expect-with-non-timeseries-dbs
(if (and (not (oracle-or-redshift? *engine*))
(if (and (not (tz-shifted-engine-bug? *engine*))
(supports-report-timezone? *engine*))
[[152 8] [153 9] [154 9] [155 4] [156 11] [157 8] [158 6] [159 10] [160 6] [161 10]]
[[152 6] [153 10] [154 4] [155 9] [156 9] [157 8] [158 8] [159 9] [160 7] [161 9]])
......@@ -582,7 +583,7 @@
date-formatter-without-time
[46 47 40 60 7])
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(results-by-week (tformat/with-zone date-formatter-without-time pacific-tz)
(result-date-formatter pacific-tz)
[46 47 40 60 7])
......@@ -614,7 +615,7 @@
date-formatter-without-time
[46 47 40 60 7])
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(results-by-week (tformat/with-zone date-formatter-without-time eastern-tz)
(result-date-formatter eastern-tz)
[46 47 40 60 7])
......@@ -643,7 +644,7 @@
date-formatter-without-time
[46 47 40 60 7])
(oracle-or-redshift? *engine*)
(tz-shifted-engine-bug? *engine*)
(results-by-week (tformat/with-zone date-formatter-without-time pacific-tz)
(result-date-formatter pacific-tz)
[46 47 40 60 7])
......@@ -669,6 +670,8 @@
(expect-with-non-timeseries-dbs
;; Not really sure why different drivers have different opinions on these </3
(cond
(= :snowflake *engine*)
[[22 46] [23 47] [24 40] [25 60] [26 7]]
(contains? #{:sqlserver :sqlite :crate :oracle :sparksql} *engine*)
[[23 54] [24 46] [25 39] [26 61]]
......
test/metabase/test/data/generic_sql.clj
+ 31
− 30
View file @ 2d938fcb
......@@ -359,46 +359,47 @@
(when (seq statement)
(execute! driver context dbdef (s/replace statement #"⅋" ";"))))))
(defn- create-db!
(defn default-create-db!
([driver database-definition]
(create-db! driver database-definition false))
(default-create-db! driver database-definition false))
([driver {:keys [table-definitions], :as dbdef} skip-drop-db?]
(when-not skip-drop-db?
;; Exec SQL for creating the DB
(execute-sql! driver :server dbdef (str (drop-db-if-exists-sql driver dbdef) ";\n"
(create-db-sql driver dbdef))))
;; Build combined statement for creating tables + FKs + comments
(let [statements (atom [])]
;; Add the SQL for creating each Table
(doseq [tabledef table-definitions]
(swap! statements conj (drop-table-if-exists-sql driver dbdef tabledef)
(create-table-sql driver dbdef tabledef)))
;; Add the SQL for adding FK constraints
(doseq [{:keys [field-definitions], :as tabledef} table-definitions]
(doseq [{:keys [fk], :as fielddef} field-definitions]
(when fk
(swap! statements conj (add-fk-sql driver dbdef tabledef fielddef)))))
;; Add the SQL for adding table comments
(doseq [{:keys [table-comment], :as tabledef} table-definitions]
(when table-comment
(swap! statements conj (standalone-table-comment-sql driver dbdef tabledef))))
;; Add the SQL for adding column comments
(doseq [{:keys [field-definitions], :as tabledef} table-definitions]
(doseq [{:keys [field-comment], :as fielddef} field-definitions]
(when field-comment
(swap! statements conj (standalone-column-comment-sql driver dbdef tabledef fielddef)))))
;; exec the combined statement
(execute-sql! driver :db dbdef (s/join ";\n" (map hx/unescape-dots @statements))))
(when-not skip-drop-db?
;; Exec SQL for creating the DB
(execute-sql! driver :server dbdef (str (drop-db-if-exists-sql driver dbdef) ";\n"
(create-db-sql driver dbdef))))
;; Build combined statement for creating tables + FKs + comments
(let [statements (atom [])]
;; Add the SQL for creating each Table
(doseq [tabledef table-definitions]
(swap! statements conj (drop-table-if-exists-sql driver dbdef tabledef)
(create-table-sql driver dbdef tabledef)))
;; Add the SQL for adding FK constraints
(doseq [{:keys [field-definitions], :as tabledef} table-definitions]
(doseq [{:keys [fk], :as fielddef} field-definitions]
(when fk
(swap! statements conj (add-fk-sql driver dbdef tabledef fielddef)))))
;; Add the SQL for adding table comments
(doseq [{:keys [table-comment], :as tabledef} table-definitions]
(when table-comment
(swap! statements conj (standalone-table-comment-sql driver dbdef tabledef))))
;; Add the SQL for adding column comments
(doseq [{:keys [field-definitions], :as tabledef} table-definitions]
(doseq [{:keys [field-comment], :as fielddef} field-definitions]
(when field-comment
(swap! statements conj (standalone-column-comment-sql driver dbdef tabledef fielddef)))))
;; exec the combined statement
(execute-sql! driver :db dbdef (s/join ";\n" (map hx/unescape-dots @statements))))
;; Now load the data for each Table
(doseq [tabledef table-definitions]
(du/profile (format "load-data for %s %s %s" (name driver) (:database-name dbdef) (:table-name tabledef))
(load-data! driver dbdef tabledef)))))
(load-data! driver dbdef tabledef)))))
(def IDriverTestExtensionsMixin
"Mixin for `IGenericSQLTestExtensions` types to implement `create-db!` from `IDriverTestExtensions`."
(merge i/IDriverTestExtensionsDefaultsMixin
{:create-db! create-db!}))
{:create-db! default-create-db!}))
;;; ## Various Util Fns
......
test/metabase/test/data/snowflake.clj
+ 51
− 20
View file @ 2d938fcb
(ns metabase.test.data.snowflake
(:require [clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[metabase.driver :as driver]
[metabase.driver.generic-sql :as sql]
[metabase.test.data
[generic-sql :as generic]
[interface :as i]]
[metabase.util :as u])
(:import metabase.driver.snowflake.SnowflakeDriver))
(def driver (metabase.driver.snowflake.SnowflakeDriver.))
(def ^:private ^:const field-base-type->sql-type
{:type/BigInteger "BIGINT"
:type/Boolean "BOOLEAN"
......@@ -18,29 +22,27 @@
:type/Text "TEXT"
:type/Time "TIME"})
;; Since all tests share the same Snowflake server let's make sure we don't stomp over any tests running
;; simultaneously by creating different databases for different tests. We'll append a random suffix to each DB name
;; created for the duration of this test which should hopefully be enough to prevent collision.
(defonce ^:private session-db-suffix
(str "__" (rand-int 100)))
(defn- database->connection-details [context {:keys [database-name]}]
(defn- database->connection-details [context {:keys [database-name] :as foo}]
(merge {:account (i/db-test-env-var-or-throw :snowflake :account)
:user (i/db-test-env-var-or-throw :snowflake :user)
:password (i/db-test-env-var-or-throw :snowflake :password)
:warehouse (i/db-test-env-var-or-throw :snowflake :warehouse)
;; SESSION parameters
:quoted_identifiers_ignore_case true
;;
;; Disabling the next param as it basically uppercases everything, tables, schemas,
;; column literals in results which breaks our tests
;;
;; :quoted_identifiers_ignore_case true
:timezone "UTC"}
(when (= context :db)
{:db (str database-name session-db-suffix)})))
{:db database-name})))
;; Snowflake requires you identify an object with db-name.schema-name.table-name
(defn- qualified-name-components
([_ db-name] [(str db-name session-db-suffix)])
([_ db-name table-name] [(str db-name session-db-suffix) "public" table-name])
([_ db-name table-name field-name] [(str db-name session-db-suffix) "public" table-name field-name]))
([_ db-name] [db-name])
([_ db-name table-name] [db-name "PUBLIC" table-name])
([_ db-name table-name field-name] [db-name "PUBLIC" table-name field-name]))
(defn- create-db-sql [driver {:keys [database-name]}]
(let [db (generic/qualify+quote-name driver database-name)]
......@@ -49,13 +51,40 @@
(defn- load-data! [driver {:keys [database-name], :as dbdef} {:keys [table-name], :as tabledef}]
(jdbc/with-db-connection [conn (generic/database->spec driver :db dbdef)]
(.setAutoCommit (jdbc/get-connection conn) false)
(let [table (generic/qualify+quote-name driver database-name table-name)
rows (generic/add-ids (generic/load-data-get-rows driver dbdef tabledef))
cols (keys (first rows))
vals (for [row rows]
(map row cols))]
(let [table (generic/qualify+quote-name driver database-name table-name)
rows (generic/add-ids (generic/load-data-get-rows driver dbdef tabledef))
col-kwds (keys (first rows))
cols (map (comp #(generic/quote-name driver %) name) col-kwds)
vals (for [row rows]
(map row col-kwds))]
(jdbc/insert-multi! conn table cols vals))))
(defn- expected-base-type->actual [base-type]
(if (isa? base-type :type/Integer)
:type/Number
base-type))
(def ^:private existing-datasets
(atom #{}))
(defn- drop-database [db-name]
(let [db-spec (sql/connection-details->spec driver (database->connection-details nil nil))]
(with-open [conn (jdbc/get-connection db-spec)]
(jdbc/execute! db-spec [(str "DROP DATABASE \"" db-name "\"")]))))
(defn existing-dataset-names []
(let [db-spec (sql/connection-details->spec driver (database->connection-details nil nil))]
(with-open [conn (jdbc/get-connection db-spec)]
(sql/get-catalogs (.getMetaData conn)))))
(defn- create-db!
[driver {:keys [database-name] :as db-def}]
(when-not (seq @existing-datasets)
(reset! existing-datasets (set (existing-dataset-names)))
(println "These Snowflake datasets have already been loaded:\n" (u/pprint-to-str (sort @existing-datasets))))
(let [db-name (str/upper-case database-name)]
(when-not (contains? @existing-datasets db-name)
(generic/default-create-db! driver db-def))))
(u/strict-extend SnowflakeDriver
generic/IGenericSQLTestExtensions
......@@ -70,6 +99,8 @@
i/IDriverTestExtensions
(merge generic/IDriverTestExtensionsMixin
{:database->connection-details (u/drop-first-arg database->connection-details)
:format-name (u/drop-first-arg str/upper-case)
:default-schema (constantly "public")
:engine (constantly :snowflake)}))
:default-schema (constantly "PUBLIC")
:engine (constantly :snowflake)
:id-field-type (constantly :type/Number)
:expected-base-type->actual (u/drop-first-arg expected-base-type->actual)
:create-db! create-db!}))
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