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

Mega sync performance improvements (#13746) 

* Add a Postgres test [ci postgres]

* Mega-optimized SELECT probe query [ci drivers]

* Mega sync improvements [ci drivers]

* Even more amazing sync perf improvements [ci drivers]

* Fix bad edit [ci drivers]

* Test/lint fixes :wrench:

* Test fix :wrench: [ci drivers]

* Some driver fixes :wrench: [ci drivers]

* Address PR feedback [ci drivers]

* Test fixes for everything except for Oracle [ci drivers]

* Fix Oracle [ci drivers]

* Fix Redshift [ci redshift]
parent 435ef211
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 753 additions and 335 deletions
modules/drivers/presto/src/metabase/driver/presto.clj
+ 20
− 10
View file @ a81ae199
......@@ -19,7 +19,7 @@
[metabase.driver.sql
[query-processor :as sql.qp]
[util :as sql.u]]
[metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
[metabase.driver.sql-jdbc.sync.describe-database :as sql-jdbc.describe-database]
[metabase.driver.sql.util.unprepare :as unprepare]
[metabase.query-processor
[context :as context]
......@@ -199,12 +199,14 @@
(def ^:private presto-metadata-sync-query-timeout
(u/minutes->ms 2))
(defmethod sql-jdbc.sync/execute-query-for-sync :presto
[_ details query]
(defn- execute-query-for-sync
[details query]
(let [result-chan (a/promise-chan)
query (if (string? query)
query
(first query))]
(do
(assert (empty? (second query)) "Presto doesn't allow parameterized queries")
(first query)))]
(execute-presto-query details query nil (fn [cols rows]
(a/>!! result-chan {:cols cols, :rows rows})))
(let [[val] (a/alts!! [result-chan (a/timeout presto-metadata-sync-query-timeout)])]
......@@ -215,7 +217,7 @@
(s/defmethod driver/can-connect? :presto
[driver {:keys [catalog] :as details} :- PrestoConnectionDetails]
(let [{[[v]] :rows} (sql-jdbc.sync/execute-query-for-sync :presto details
(let [{[[v]] :rows} (execute-query-for-sync details
(format "SHOW SCHEMAS FROM %s LIKE 'information_schema'"
(sql.u/quote-name driver :database catalog)))]
(= v "information_schema")))
......@@ -228,14 +230,22 @@
"Return a set of all schema names in this `database`."
[driver {{:keys [catalog schema] :as details} :details :as database}]
(let [sql (str "SHOW SCHEMAS FROM " (sql.u/quote-name driver :database catalog))
{:keys [rows]} (sql-jdbc.sync/execute-query-for-sync :presto details sql)]
{:keys [rows]} (execute-query-for-sync details sql)]
(set (map first rows))))
(defn- have-select-privilege? [driver details schema table-name]
(try
(let [sql-args (sql-jdbc.describe-database/simple-select-probe-query driver schema table-name)]
;; if the query completes without throwing an Exception, we can SELECT from this table
(execute-query-for-sync details sql-args)
true)
(catch Throwable _
false)))
(defn- describe-schema [driver {{:keys [catalog user] :as details} :details :as db} {:keys [schema]}]
(let [sql (str "SHOW TABLES FROM " (sql.u/quote-name driver :schema catalog schema))]
(set (for [[table-name & _] (:rows (sql-jdbc.sync/execute-query-for-sync :presto details sql))
:when (sql-jdbc.sync/have-select-privilege? driver details {:table_schem schema
:table_name table-name})]
(set (for [[table-name & _] (:rows (execute-query-for-sync details sql))
:when (have-select-privilege? driver details schema table-name)]
{:name table-name
:schema schema}))))
......@@ -250,7 +260,7 @@
(defmethod driver/describe-table :presto
[driver {{:keys [catalog] :as details} :details} {schema :schema, table-name :name}]
(let [sql (str "DESCRIBE " (sql.u/quote-name driver :table catalog schema table-name))
{:keys [rows]} (sql-jdbc.sync/execute-query-for-sync :presto details sql)]
{:keys [rows]} (execute-query-for-sync details sql)]
{:schema schema
:name table-name
:fields (set (for [[idx [name type]] (m/indexed rows)]
......
modules/drivers/presto/test/metabase/test/data/presto.clj
+ 2
− 3
View file @ a81ae199
......@@ -9,7 +9,6 @@
[config :as config]
[driver :as driver]]
[metabase.driver.presto :as presto]
[metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
[metabase.driver.sql.util :as sql.u]
[metabase.driver.sql.util.unprepare :as unprepare]
[metabase.test.data
......@@ -95,7 +94,7 @@
(defmethod tx/create-db! :presto
[driver {:keys [table-definitions database-name] :as dbdef} & {:keys [skip-drop-db?]}]
(let [details (tx/dbdef->connection-details driver :db dbdef)
execute! (partial sql-jdbc.sync/execute-query-for-sync :presto details)]
execute! (partial #'presto/execute-query-for-sync details)]
(doseq [tabledef table-definitions
:let [rows (:rows tabledef)
;; generate an ID for each row because we don't have auto increments
......@@ -111,7 +110,7 @@
(defmethod tx/destroy-db! :presto
[driver {:keys [database-name table-definitions], :as dbdef}]
(let [details (tx/dbdef->connection-details driver :db dbdef)
execute! (partial sql-jdbc.sync/execute-query-for-sync :presto details)]
execute! (partial #'presto/execute-query-for-sync details)]
(doseq [{:keys [table-name], :as tabledef} table-definitions]
(println (format "[Presto] destroying %s.%s" (pr-str database-name) (pr-str table-name)))
(execute! (sql.tx/drop-table-if-exists-sql driver dbdef tabledef))
......
modules/drivers/redshift/test/metabase/driver/redshift_test.clj
+ 10
− 6
View file @ a81ae199
......@@ -10,17 +10,21 @@
[metabase.driver.sql-jdbc.execute :as execute]
[metabase.plugins.jdbc-proxy :as jdbc-proxy]
[metabase.test.data.redshift :as rstest]
[metabase.test.fixtures :as fixtures]))
[metabase.test.fixtures :as fixtures])
(:import metabase.plugins.jdbc_proxy.ProxyDriver))
(use-fixtures :once (fixtures/initialize :plugins))
(use-fixtures :once (fixtures/initialize :db))
(deftest correct-driver-test
(mt/test-driver
:redshift
(is (= "com.amazon.redshift.jdbc.Driver"
(.getName (class (jdbc-proxy/wrapped-driver (java.sql.DriverManager/getDriver "jdbc:redshift://host:5432/testdb")))))
"Make sure we're using the correct driver for Redshift")))
(mt/test-driver :redshift
(testing "Make sure we're using the correct driver for Redshift"
(let [driver (java.sql.DriverManager/getDriver "jdbc:redshift://host:5432/testdb")
driver (if (instance? metabase.plugins.jdbc_proxy.ProxyDriver driver)
(jdbc-proxy/wrapped-driver driver)
driver)]
(is (= "com.amazon.redshift.jdbc.Driver"
(.getName (class driver))))))))
(defn- query->native [query]
(let [native-query (atom nil)]
......
modules/drivers/snowflake/src/metabase/driver/snowflake.clj
+ 23
− 17
View file @ a81ae199
......@@ -25,7 +25,7 @@
[metabase.util
[date-2 :as u.date]
[honeysql-extensions :as hx]
[i18n :refer [tru]]])
[i18n :refer [trs tru]]])
(:import [java.sql ResultSet Types]
[java.time OffsetDateTime ZonedDateTime]
metabase.util.honeysql_extensions.Identifier))
......@@ -46,7 +46,6 @@
[_]
:sunday)
(defmethod sql-jdbc.conn/connection-details->spec :snowflake
[_ {:keys [account regionid], :as opts}]
(let [host (if regionid
......@@ -248,24 +247,31 @@
excluded-schemas (set (sql-jdbc.sync/excluded-schemas driver))]
(qp.store/with-store
(qp.store/fetch-and-store-database! (u/get-id database))
{:tables (set (for [table (jdbc/query
(sql-jdbc.conn/db->pooled-connection-spec database)
(format "SHOW OBJECTS IN DATABASE \"%s\"" db-name))
:when (and (not (contains? excluded-schemas (:schema_name table)))
(sql-jdbc.sync/have-select-privilege? driver database
{:table_name (:name table)
:table_schem (:schema_name table)}))]
{:name (:name table)
:schema (:schema_name table)
:description (not-empty (:comment table))}))})))
(let [spec (sql-jdbc.conn/db->pooled-connection-spec database)
sql (format "SHOW OBJECTS IN DATABASE \"%s\"" db-name)]
(with-open [conn (jdbc/get-connection spec)]
{:tables (into
#{}
(comp (filter (fn [{schema :schema_name, table-name :name}]
(and (not (contains? excluded-schemas schema))
(sql-jdbc.sync/have-select-privilege? driver conn schema table-name))))
(map (fn [{schema :schema_name, table-name :name, remark :comment}]
{:name table-name
:schema schema
:description (not-empty remark)})))
(try
(jdbc/reducible-query {:connection conn} sql)
(catch Throwable e
(throw (ex-info (trs "Error executing query") {:sql sql} e)))))})))))
(defmethod driver/describe-table :snowflake
[driver database table]
(jdbc/with-db-metadata [metadata (sql-jdbc.conn/db->pooled-connection-spec database)]
(->> (assoc (select-keys table [:name :schema])
:fields (sql-jdbc.sync/describe-table-fields metadata driver table (db-name database)))
;; find PKs and mark them
(sql-jdbc.sync/add-table-pks metadata))))
(let [spec (sql-jdbc.conn/db->pooled-connection-spec database)]
(with-open [conn (jdbc/get-connection spec)]
(->> (assoc (select-keys table [:name :schema])
:fields (sql-jdbc.sync/describe-table-fields driver conn table (db-name database)))
;; find PKs and mark them
(sql-jdbc.sync/add-table-pks (.getMetaData conn))))))
(defmethod driver/describe-table-fks :snowflake
[driver database table]
......
modules/drivers/sqlite/src/metabase/driver/sqlite.clj
+ 8
− 0
View file @ a81ae199
......@@ -82,6 +82,14 @@
[_ database-type]
(database-type->base-type database-type))
;; The normal SELECT * FROM table WHERE 1 <> 1 LIMIT 0 query doesn't return any information for SQLite views -- it
;; seems to be the case that the query has to return at least one row
(defmethod sql-jdbc.sync/fallback-metadata-query :sqlite
[driver schema table]
(sql.qp/format-honeysql driver {:select [:*]
:from [(sql.qp/->honeysql driver (hx/identifier :table schema table))]
:limit 1}))
;; register the SQLite concatnation operator `||` with HoneySQL as `sqlite-concat`
;; (hsql/format (hsql/call :sqlite-concat :a :b)) -> "(a || b)"
(defmethod hformat/fn-handler "sqlite-concat"
......
modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj
+ 11
− 10
View file @ a81ae199
......@@ -6,18 +6,13 @@
[query-processor-test :as qp.test]
[sync :as sync]
[test :as mt]]
[metabase.driver.sql-jdbc
[connection :as sql-jdbc.conn]
[execute :as sql-jdbc.execute]]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.models
[database :refer [Database]]
[field :refer [Field]]
[table :refer [Table]]]
[metabase.query-processor-test :as qp.test]
[metabase.test
[data :as data]
[util :as tu]]
[metabase.test.data.datasets :as datasets]
[toucan
[db :as db]
[hydrate :refer [hydrate]]]))
......@@ -59,8 +54,12 @@
(testing "Make sure we correctly infer complex types in views (#8630, #9276, #12191, #12547, #10681)"
(let [details (mt/dbdef->connection-details :sqlite :db {:database-name "views_test"})
spec (sql-jdbc.conn/connection-details->spec :sqlite details)]
(mt/with-temp Database [{db-id :id :as database} {:engine :sqlite, :details (assoc details :dbname "viwes_test")}]
(doseq [statement ["create table if not exists src(id integer, time text);"
(mt/with-temp Database [{db-id :id :as database} {:engine :sqlite, :details (assoc details :dbname "views_test")}]
(doseq [statement ["drop view if exists v_groupby_test;"
"drop table if exists groupby_test;"
"drop view if exists v_src;"
"drop table if exists src;"
"create table if not exists src(id integer, time text);"
"create view if not exists v_src as select id, strftime('%s', time) as time from src;"
"insert into src values(1, '2020-03-01 12:20:35');"]]
(jdbc/execute! spec [statement]))
......@@ -77,8 +76,10 @@
:base_type :type/Text}]}]
(->> (hydrate (db/select Table :db_id db-id {:order-by [:name]}) :fields)
(map table-fingerprint))))
(doseq [statement ["CREATE TABLE IF NOT EXISTS groupby_test (
id INTEGER primary key unique,
(doseq [statement ["drop view if exists v_groupby_test;"
"drop table if exists groupby_test;"
"CREATE TABLE IF NOT EXISTS groupby_test (
id INTEGER primary key unique,
symbol VARCHAR,
dt DATETIME,
value FLOAT);"
......
src/metabase/driver/sql/query_processor.clj
+ 7
− 2
View file @ a81ae199
......@@ -17,6 +17,7 @@
[field :as field :refer [Field]]
[table :refer [Table]]]
[metabase.query-processor
[error-type :as qp.error-type]
[interface :as i]
[store :as qp.store]]
[metabase.query-processor.middleware
......@@ -24,7 +25,7 @@
[wrap-value-literals :as value-literal]]
[metabase.util
[honeysql-extensions :as hx]
[i18n :refer [deferred-tru]]
[i18n :refer [deferred-tru tru]]
[schema :as su]]
[potemkin.types :as p.types]
[pretty.core :refer [PrettyPrintable]]
......@@ -815,7 +816,11 @@
"\n"
(u/pprint-to-str honeysql-form))))
(finally
(throw e))))))
(throw (ex-info (tru "Error compiling HoneySQL form")
{:driver driver
:form honeysql-form
:type qp.error-type/driver}
e)))))))
(defn- add-default-select
"Add `SELECT *` to `honeysql-form` if no `:select` clause is present."
......
src/metabase/driver/sql_jdbc/execute.clj
+ 2
− 3
View file @ a81ae199
......@@ -11,10 +11,9 @@
[metabase
[driver :as driver]
[util :as u]]
[metabase.driver.sql-jdbc
[connection :as sql-jdbc.conn]
[sync :as sql-jdbc.sync]]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql-jdbc.execute.old-impl :as execute.old]
[metabase.driver.sql-jdbc.sync.interface :as sql-jdbc.sync]
[metabase.mbql.util :as mbql.u]
[metabase.query-processor
[context :as context]
......
src/metabase/driver/sql_jdbc/sync.clj
+ 29
− 266
View file @ a81ae199
(ns metabase.driver.sql-jdbc.sync
"Implementations for sync-related driver multimethods for SQL JDBC drivers, using JDBC DatabaseMetaData."
(:require [clojure
[set :as set]
[string :as str]]
[clojure.java.jdbc :as jdbc]
[clojure.tools.logging :as log]
[medley.core :as m]
[metabase
[driver :as driver]
[util :as u]]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.util.honeysql-extensions :as hx])
(:import (java.sql Connection DatabaseMetaData ResultSetMetaData)))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Interface (Multimethods) |
;;; +----------------------------------------------------------------------------------------------------------------+
(defmulti active-tables
"Return a set of maps containing information about the active tables/views, collections, or equivalent that currently
exist in a database. Each map should contain the key `:name`, which is the string name of the table. For databases
that have a concept of schemas, this map should also include the string name of the table's `:schema`.
Two different implementations are provided in this namespace: `fast-active-tables` (the default), and
`post-filtered-active-tables`. You should be fine using the default, but refer to the documentation for those
functions for more details on the differences.
`metabase` is an instance of `DatabaseMetaData`."
{:arglists '([driver database metadata])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(declare fast-active-tables)
(defmethod active-tables :sql-jdbc [driver database metadata]
(fast-active-tables driver database metadata))
(defmulti excluded-schemas
"Return set of string names of schemas to skip syncing tables from."
{:arglists '([driver])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(defmethod excluded-schemas :sql-jdbc [_] nil)
;; TODO - why don't we just use JDBC `DatabaseMetaData` to do this? This is wacky
(defmulti database-type->base-type
"Given a native DB column type (as a keyword), return the corresponding `Field` `base-type`, which should derive from
`:type/*`. You can use `pattern-based-database-type->base-type` in this namespace to implement this using regex
patterns."
{:arglists '([driver database-type])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(defmulti column->special-type
"Attempt to determine the special-type of a field given the column name and native type. For example, the Postgres
driver can mark Postgres JSON type columns as `:type/SerializedJSON` special type.
`database-type` and `column-name` will be strings."
{:arglists '([driver database-type column-name])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(defmethod column->special-type :sql-jdbc [_ _ _] nil)
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Common |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn pattern-based-database-type->base-type
"Return a `database-type->base-type` function that matches types based on a sequence of pattern / base-type pairs."
[pattern->type]
(fn [column-type]
(let [column-type (name column-type)]
(loop [[[pattern base-type] & more] pattern->type]
(cond
(re-find pattern column-type) base-type
(seq more) (recur more))))))
(defn- ->spec [db-or-id-or-spec]
(if (u/id db-or-id-or-spec)
(sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec)
db-or-id-or-spec))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Sync Impl |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- db-tables
"Fetch a JDBC Metadata ResultSet of tables in the DB, optionally limited to ones belonging to a given
schema."
[driver ^DatabaseMetaData metadata ^String schema-or-nil ^String db-name-or-nil]
;; tablePattern "%" = match all tables
(with-open [rs (.getTables metadata db-name-or-nil (driver/escape-entity-name-for-metadata driver schema-or-nil) "%"
(into-array String ["TABLE" "VIEW" "FOREIGN TABLE" "MATERIALIZED VIEW" "EXTERNAL TABLE"]))]
(into []
(map (fn [row] (select-keys row [:table_name :remarks :table_schem])))
(jdbc/reducible-result-set rs {}))))
(defn- simple-select-probe
"Simple (ie. cheap) SELECT on a given table to test for access and get column metadata."
[driver schema table]
;; Using our SQL compiler here to get portable LIMIT
(sql.qp/format-honeysql driver
(sql.qp/apply-top-level-clause driver :limit
{:select [:*]
:from [(sql.qp/->honeysql driver (hx/identifier :table schema table))]}
{:limit 1})))
(defmulti execute-query-for-sync
"Execute given SQL query. Used during parts of sync where we don't have metadata populated and
can't use our default query processing pipeline."
{:arglists '([driver db-or-id-or-spec query])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(defmethod execute-query-for-sync :sql-jdbc
[_ db-or-id-or-spec query]
(jdbc/query (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec) query))
(defn have-select-privilege?
"Check if we have select privilege for given table"
[driver db-or-id-or-spec {:keys [table_name table_schem] :as table}]
(when (u/ignore-exceptions
(execute-query-for-sync driver db-or-id-or-spec
(simple-select-probe driver table_schem table_name)))
table))
(defn- all-schemas
[^DatabaseMetaData metadata]
(with-open [rs (.getSchemas metadata)]
(set (map :table_schem (jdbc/result-set-seq rs)))))
(defn fast-active-tables
"Default, fast implementation of `active-tables` best suited for DBs with lots of system tables (like Oracle). Fetch
list of schemas, then for each one not in `excluded-schemas`, fetch its Tables, and combine the results.
This is as much as 15x faster for Databases with lots of system tables than `post-filtered-active-tables` (4 seconds
vs 60)."
[driver, db-or-id-or-spec, ^DatabaseMetaData metadata, & [db-name-or-nil]]
(->> (set/difference (all-schemas metadata) (excluded-schemas driver))
(mapcat (fn [schema]
(db-tables driver metadata schema db-name-or-nil)))
(filter (partial have-select-privilege? driver db-or-id-or-spec))))
(defn post-filtered-active-tables
"Alternative implementation of `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, db-or-id-or-spec, ^DatabaseMetaData metadata, & [db-name-or-nil]]
(filter (every-pred (partial have-select-privilege? driver db-or-id-or-spec)
(comp not (partial contains? (excluded-schemas driver)) :table_schem))
(db-tables driver metadata nil db-name-or-nil)))
(defn get-catalogs
"Returns a set of all of the catalogs found via `metadata`"
[^DatabaseMetaData metadata]
(with-open [rs (.getCatalogs metadata)]
(set (map :table_cat (jdbc/metadata-result rs)))))
(defn- database-type->base-type-or-warn
"Given a `database-type` (e.g. `VARCHAR`) return the mapped Metabase type (e.g. `:type/Text`)."
[driver database-type]
(or (database-type->base-type driver (keyword database-type))
(do (log/warn (format "Don't know how to map column type '%s' to a Field base_type, falling back to :type/*."
database-type))
:type/*)))
(defn- calculated-special-type
"Get an appropriate special type for a column with `column-name` of type `database-type`."
[driver, ^String column-name, ^String database-type]
(when-let [special-type (column->special-type driver database-type column-name)]
(assert (isa? special-type :type/*)
(str "Invalid type: " special-type))
special-type))
(defn- fields-metadata
[^DatabaseMetaData metadata, driver, {^String schema :schema, ^String table-name :name :as table}, & [^String db-name-or-nil]]
(with-open [rs (.getColumns metadata
db-name-or-nil
(driver/escape-entity-name-for-metadata driver schema)
(driver/escape-entity-name-for-metadata driver table-name)
nil)]
(let [result (jdbc/result-set-seq rs)]
;; In some rare cases `:column_name` is blank (eg. SQLite's views with group by),
;; fallback to sniffing the type from a SELECT * query
(if (some (comp str/blank? :type_name) result)
(jdbc/with-db-connection [conn (->spec (:db_id table))]
(let [^Connection conn (:connection conn)
^ResultSetMetaData metadata (->> (simple-select-probe driver schema table-name)
first
(.executeQuery (.createStatement conn))
(.getMetaData))]
(doall
(for [i (range 1 (inc (.getColumnCount metadata)))]
{:type_name (.getColumnTypeName metadata i)
:column_name (.getColumnName metadata i)}))))
result))))
(defn describe-table-fields
"Returns a set of column metadata for `schema` and `table-name` using `metadata`. "
[^DatabaseMetaData metadata, driver, table, & [^String db-name-or-nil]]
(set
(for [[idx {database-type :type_name
column-name :column_name
remarks :remarks}] (m/indexed (fields-metadata metadata driver table db-name-or-nil))]
(merge
{:name column-name
:database-type database-type
:base-type (database-type->base-type-or-warn driver database-type)
:database-position idx}
(when (not (str/blank? remarks))
{:field-comment remarks})
(when-let [special-type (calculated-special-type driver column-name database-type)]
{:special-type special-type})))))
(defn add-table-pks
"Using `metadata` find any primary keys for `table` and assoc `:pk?` to true for those columns."
[^DatabaseMetaData metadata, table]
(with-open [rs (.getPrimaryKeys metadata nil nil (:name table))]
(let [pks (set (map :column_name (jdbc/metadata-result rs)))]
(update table :fields (fn [fields]
(set (for [field fields]
(if-not (contains? pks (:name field))
field
(assoc field :pk? true)))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Default SQL JDBC metabase.driver impls for sync methods |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn describe-database
"Default implementation of `driver/describe-database` for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
[driver db-or-id-or-spec]
(jdbc/with-db-metadata [metadata (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec)]
{:tables (set (for [table (active-tables driver db-or-id-or-spec metadata)]
(let [remarks (:remarks table)]
{:name (:table_name table)
:schema (:table_schem table)
:description (when-not (str/blank? remarks)
remarks)})))}))
(defn describe-table
"Default implementation of `driver/describe-table` for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
[driver db-or-id-or-spec table]
(jdbc/with-db-metadata [metadata (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec)]
(->> (assoc (select-keys table [:name :schema]) :fields (describe-table-fields metadata driver table))
;; find PKs and mark them
(add-table-pks metadata))))
(defn describe-table-fks
"Default implementation of `driver/describe-table-fks` for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
[driver db-or-id-or-spec table & [^String db-name-or-nil]]
(jdbc/with-db-metadata [metadata (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec)]
(with-open [rs (.getImportedKeys metadata db-name-or-nil, ^String (:schema table), ^String (:name table))]
(set
(for [result (jdbc/metadata-result rs)]
{:fk-column-name (:fkcolumn_name result)
:dest-table {:name (:pktable_name result)
:schema (:pktable_schem result)}
:dest-column-name (:pkcolumn_name result)})))))
(:require [metabase.driver.sql-jdbc.sync
[describe-database :as sync.describe-database]
[describe-table :as sync.describe-table]
[interface :as i]]
[potemkin :as p]))
(comment i/keep-me sync.describe-database/keep-me sync.describe-table/keep-me)
(p/import-vars
[i
active-tables
column->special-type
database-type->base-type
excluded-schemas
fallback-metadata-query
have-select-privilege?]
[sync.describe-table
add-table-pks
describe-table
describe-table-fields
describe-table-fks
get-catalogs
pattern-based-database-type->base-type]
[sync.describe-database
describe-database
fast-active-tables
post-filtered-active-tables])
src/metabase/driver/sql_jdbc/sync/common.clj 0 → 100644
+ 40
− 0
View file @ a81ae199
(ns metabase.driver.sql-jdbc.sync.common
(:require [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute])
(:import [java.sql Connection PreparedStatement ResultSet]))
(defn prepare-statement
"Create a PreparedStatement for metadata queries; set `TYPE_FORWARD_ONLY`/`CONCUR_READ_ONLY`/`FETCH_FORWARD` options
if possible. These queries return no rows."
^PreparedStatement [driver ^Connection conn ^String sql params]
;; `sql-jdbc.execute/prepared-statement` will set `TYPE_FORWARD_ONLY`/`CONCUR_READ_ONLY`/`FETCH_FORWARD` if
;; possible, although I'm not sure if that will make a difference if we don't actually realize the ResultSet
(doto ^PreparedStatement (sql-jdbc.execute/prepared-statement driver conn sql params)
(.setMaxRows 0)))
(defn reducible-results
"Creates an `IReduceInit` for a function that returns a `ResultSet`, and a function that is called once for each row.
`rs-thunk` should return a `ResultSet`; `rs->row-thunk` has the signature
(rs->row-thunk rs)-> row-thunk
`rs->row-thunk` is called once with the ResultSet, and should return a thunk; the resulting thunk is called once for
each row. Example:
(reducible-results
;; `rs-thunk` should return a `ResultSet`
#(.getSchemas metadata)
;; `rs->row-thunk` is called once with the `ResultSet`, and returns a thunk
(fn [rs]
;; the thunk is called once for each row to get results
(fn []
(.getString rs \"TABLE_SCHEM\"))))"
[rs-thunk rs->row-thunk]
(reify clojure.lang.IReduceInit
(reduce [_ rf init]
(with-open [^ResultSet rs (rs-thunk)]
(reduce
((take-while some?) rf)
init
(let [row-thunk (rs->row-thunk rs)]
(repeatedly #(when (.next rs)
(row-thunk)))))))))
src/metabase/driver/sql_jdbc/sync/describe_database.clj 0 → 100644
+ 124
− 0
View file @ a81ae199
(ns metabase.driver.sql-jdbc.sync.describe-database
"SQL JDBC impl for `describe-database`."
(:require [clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[metabase.driver :as driver]
[metabase.driver.sql-jdbc
[connection :as sql-jdbc.conn]
[execute :as sql-jdbc.execute]]
[metabase.driver.sql-jdbc.sync
[common :as common]
[interface :as i]]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.util.honeysql-extensions :as hx])
(:import [java.sql Connection DatabaseMetaData ResultSet]))
(defmethod i/excluded-schemas :sql-jdbc [_] nil)
(defn- all-schemas [^DatabaseMetaData metadata]
{:pre [(instance? DatabaseMetaData metadata)]}
(common/reducible-results
#(.getSchemas metadata)
(fn [^ResultSet rs]
#(.getString rs "TABLE_SCHEM"))))
(defn- syncable-schemas
[driver metadata]
(eduction (remove (set (i/excluded-schemas driver)))
(all-schemas metadata)))
(defn simple-select-probe-query
"Simple (ie. cheap) SELECT on a given table to test for access and get column metadata. Doesn't return
anything useful (only used to check whether we can execute a SELECT query)
(simple-select-probe-query :postgres \"public\" \"my_table\")
;; -> [\"SELECT TRUE FROM public.my_table WHERE 1 <> 1 LIMIT 0\"]"
[driver schema table]
{:pre [(string? table)]}
;; Using our SQL compiler here to get portable LIMIT (e.g. `SELECT TOP n ...` for SQL Server/Oracle)
(let [honeysql {:select [[(sql.qp/->honeysql driver true) :_]]
:from [(sql.qp/->honeysql driver (hx/identifier :table schema table))]
:where [:not= 1 1]}
honeysql (sql.qp/apply-top-level-clause driver :limit honeysql {:limit 0})]
(sql.qp/format-honeysql driver honeysql)))
(defn- execute-select-probe-query
"Execute the simple SELECT query defined above. The main goal here is to check whether we're able to execute a SELECT
query against the Table in question -- we don't care about the results themselves -- so the query and the logic
around executing it should be as simple as possible. We need to highly optimize this logic because it's executed for
every Table on every sync."
[driver ^Connection conn [sql & params]]
{:pre [(string? sql)]}
(with-open [stmt (common/prepare-statement driver conn sql params)]
;; attempting to execute the SQL statement will throw an Exception if we don't have permissions; otherwise it will
;; truthy wheter or not it returns a ResultSet, but we can ignore that since we have enough info to proceed at
;; this point.
(.execute stmt)))
(defmethod i/have-select-privilege? :sql-jdbc
[driver conn table-schema table-name]
;; Query completes = we have SELECT privileges
;; Query throws some sort of no permissions exception = no SELECT privileges
(let [sql-args (simple-select-probe-query driver table-schema table-name)]
(try
(execute-select-probe-query driver conn sql-args)
true
(catch Throwable _
false))))
(defn- db-tables
"Fetch a JDBC Metadata ResultSet of tables in the DB, optionally limited to ones belonging to a given
schema. Returns a reducible sequence of results."
[driver ^DatabaseMetaData metadata ^String schema-or-nil ^String db-name-or-nil]
(common/reducible-results
#(.getTables metadata db-name-or-nil (some->> schema-or-nil (driver/escape-entity-name-for-metadata driver)) "%"
(into-array String ["TABLE" "VIEW" "FOREIGN TABLE" "MATERIALIZED VIEW" "EXTERNAL TABLE"]))
(fn [^ResultSet rs]
(fn []
{:name (.getString rs "TABLE_NAME")
:schema (.getString rs "TABLE_SCHEM")
:description (when-let [remarks (.getString rs "REMARKS")]
(when-not (str/blank? remarks)
remarks))}))))
(defn fast-active-tables
"Default, fast implementation of `active-tables` best suited for DBs with lots of system tables (like Oracle). Fetch
list of schemas, then for each one not in `excluded-schemas`, fetch its Tables, and combine the results.
This is as much as 15x faster for Databases with lots of system tables than `post-filtered-active-tables` (4 seconds
vs 60)."
[driver ^Connection conn & [db-name-or-nil]]
{:pre [(instance? Connection conn)]}
(let [metadata (.getMetaData conn)]
(eduction
(comp (mapcat (fn [schema]
(db-tables driver metadata schema db-name-or-nil)))
(filter (fn [{table-schema :schema, table-name :name}]
(i/have-select-privilege? driver conn table-schema table-name))))
(syncable-schemas driver metadata))))
(defmethod i/active-tables :sql-jdbc
[driver connection]
(fast-active-tables driver connection))
(defn post-filtered-active-tables
"Alternative implementation of `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 ^Connection conn & [db-name-or-nil]]
{:pre [(instance? Connection conn)]}
(eduction
(filter (let [excluded (i/excluded-schemas driver)]
(fn [{table-schema :schema, table-name :name}]
(and (not (contains? excluded table-schema))
(i/have-select-privilege? driver conn table-schema table-name)))))
(db-tables driver (.getMetaData conn) nil db-name-or-nil)))
(defn describe-database
"Default implementation of `driver/describe-database` for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
[driver db-or-id-or-spec]
{:tables (with-open [conn (jdbc/get-connection (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec))]
;; try to set the Connection to `READ_UNCOMMITED` if possible, or whatever the next least-locking level
;; is. Not sure how much of a difference that makes since we're not running this inside a transaction,
;; but better safe than sorry
(sql-jdbc.execute/set-best-transaction-level! driver conn)
(into #{} (i/active-tables driver conn)))})
src/metabase/driver/sql_jdbc/sync/describe_table.clj 0 → 100644
+ 193
− 0
View file @ a81ae199
(ns metabase.driver.sql-jdbc.sync.describe-table
"SQL JDBC impl for `describe-table` and `describe-table-fks`."
(:require [clojure.java.jdbc :as jdbc]
[clojure.string :as str]
[clojure.tools.logging :as log]
[medley.core :as m]
[metabase
[driver :as driver]
[util :as u]]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql-jdbc.sync
[common :as common]
[interface :as i]]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.util.honeysql-extensions :as hx])
(:import [java.sql Connection DatabaseMetaData ResultSet]))
(defmethod i/column->special-type :sql-jdbc [_ _ _] nil)
(defn pattern-based-database-type->base-type
"Return a `database-type->base-type` function that matches types based on a sequence of pattern / base-type pairs.
`pattern->type` is a map of regex pattern to MBQL type keyword."
[pattern->type]
(fn database-type->base-type [column-type]
(let [column-type (name column-type)]
(some
(fn [[pattern base-type]]
(when (re-find pattern column-type)
base-type))
pattern->type))))
(defn get-catalogs
"Returns a set of all of the catalogs found via `metadata`"
[^DatabaseMetaData metadata]
(with-open [rs (.getCatalogs metadata)]
(set (map :table_cat (jdbc/metadata-result rs)))))
(defn- database-type->base-type-or-warn
"Given a `database-type` (e.g. `VARCHAR`) return the mapped Metabase type (e.g. `:type/Text`)."
[driver database-type]
(or (i/database-type->base-type driver (keyword database-type))
(do (log/warn (format "Don't know how to map column type '%s' to a Field base_type, falling back to :type/*."
database-type))
:type/*)))
(defn- calculated-special-type
"Get an appropriate special type for a column with `column-name` of type `database-type`."
[driver ^String column-name ^String database-type]
(when-let [special-type (i/column->special-type driver database-type column-name)]
(assert (isa? special-type :type/*)
(str "Invalid type: " special-type))
special-type))
(defmethod i/fallback-metadata-query :sql-jdbc
[driver schema table]
{:pre [(string? table)]}
;; Using our SQL compiler here to get portable LIMIT (e.g. `SELECT TOP n ...` for SQL Server/Oracle)
(let [honeysql {:select [:*]
:from [(sql.qp/->honeysql driver (hx/identifier :table schema table))]
:where [:not= 1 1]}
honeysql (sql.qp/apply-top-level-clause driver :limit honeysql {:limit 0})]
(sql.qp/format-honeysql driver honeysql)))
(defn- fallback-fields-metadata-from-select-query
"In some rare cases `:column_name` is blank (eg. SQLite's views with group by) fallback to sniffing the type from a
SELECT * query."
[driver ^Connection conn table-schema table-name]
;; some DBs (:sqlite) don't actually return the correct metadata for LIMIT 0 queries
(let [[sql & params] (i/fallback-metadata-query driver table-schema table-name)]
(reify clojure.lang.IReduceInit
(reduce [_ rf init]
(with-open [stmt (common/prepare-statement driver conn sql params)
rs (.executeQuery stmt)]
(let [metadata (.getMetaData rs)]
(reduce
((map (fn [^Integer i]
{:name (.getColumnName metadata i)
:database-type (.getColumnTypeName metadata i)})) rf)
init
(range 1 (inc (.getColumnCount metadata))))))))))
(defn- jdbc-fields-metadata
"Reducible metadata about the Fields belonging to a Table, fetching using JDBC DatabaseMetaData methods."
[driver ^Connection conn db-name-or-nil schema table-name]
(common/reducible-results #(.getColumns (.getMetaData conn)
db-name-or-nil
(some->> schema (driver/escape-entity-name-for-metadata driver))
(some->> table-name (driver/escape-entity-name-for-metadata driver))
nil)
(fn [^ResultSet rs]
#(merge
{:name (.getString rs "COLUMN_NAME")
:database-type (.getString rs "TYPE_NAME")}
(when-let [remarks (.getString rs "REMARKS")]
(when-not (str/blank? remarks)
{:field-comment remarks}))))))
(defn- fields-metadata
"Returns reducible metadata for the Fields in a `table`."
[driver ^Connection conn {schema :schema, table-name :name} & [^String db-name-or-nil]]
{:pre [(instance? Connection conn) (string? table-name)]}
(reify clojure.lang.IReduceInit
(reduce [_ rf init]
;; 1. Return all the Fields that come back from DatabaseMetaData that include type info.
;;
;; 2. Iff there are some Fields that don't have type info, concatenate
;; `fallback-fields-metadata-from-select-query`, which fetches the same Fields using a different method.
;;
;; 3. Filter out any duplicates between the two methods using `m/distinct-by`.
(let [has-fields-without-type-info? (volatile! false)
jdbc-metadata (eduction
(remove (fn [{:keys [database-type]}]
(when (str/blank? database-type)
(vreset! has-fields-without-type-info? true)
true)))
(jdbc-fields-metadata driver conn db-name-or-nil schema table-name))
fallback-metadata (reify clojure.lang.IReduceInit
(reduce [_ rf init]
(reduce
rf
init
(when @has-fields-without-type-info?
(fallback-fields-metadata-from-select-query driver conn schema table-name)))))]
;; VERY IMPORTANT! DO NOT REWRITE THIS TO BE LAZY! IT ONLY WORKS BECAUSE AS NORMAL-FIELDS GETS REDUCED,
;; HAS-FIELDS-WITHOUT-TYPE-INFO? WILL GET SET TO TRUE IF APPLICABLE AND THEN FALLBACK-FIELDS WILL RUN WHEN
;; IT'S TIME TO START EVALUATING THAT.
(reduce
((comp cat (m/distinct-by :name)) rf)
init
[jdbc-metadata fallback-metadata])))))
(defn describe-table-fields
"Returns a set of column metadata for `table` using JDBC Connection `conn`."
[driver conn table & [db-name-or-nil]]
(into
#{}
(map-indexed (fn [i {:keys [database-type], column-name :name, :as col}]
(merge
(u/select-non-nil-keys col [:name :database-type :field-comment])
{:base-type (database-type->base-type-or-warn driver database-type)
:database-position i}
(when-let [special-type (calculated-special-type driver column-name database-type)]
{:special-type special-type}))))
(fields-metadata driver conn table db-name-or-nil)))
(defn add-table-pks
"Using `metadata` find any primary keys for `table` and assoc `:pk?` to true for those columns."
[^DatabaseMetaData metadata table]
(let [pks (into #{} (common/reducible-results #(.getPrimaryKeys metadata nil nil (:name table))
(fn [^ResultSet rs]
#(.getString rs "COLUMN_NAME"))))]
(update table :fields (fn [fields]
(set (for [field fields]
(if-not (contains? pks (:name field))
field
(assoc field :pk? true))))))))
(defn- describe-table* [driver ^Connection conn table]
{:pre [(instance? Connection conn)]}
(->> (assoc (select-keys table [:name :schema])
:fields (describe-table-fields driver conn table))
;; find PKs and mark them
(add-table-pks (.getMetaData conn))))
(defn describe-table
"Default implementation of `driver/describe-table` for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
[driver db-or-id-or-spec-or-conn table]
(if (instance? Connection db-or-id-or-spec-or-conn)
(describe-table* driver db-or-id-or-spec-or-conn table)
(let [spec (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec-or-conn)]
(with-open [conn (jdbc/get-connection spec)]
(describe-table* driver conn table)))))
(defn- describe-table-fks*
[driver ^Connection conn {^String schema :schema, ^String table-name :name} & [^String db-name-or-nil]]
(into
#{}
(common/reducible-results #(.getImportedKeys (.getMetaData conn) db-name-or-nil schema table-name)
(fn [^ResultSet rs]
(fn []
{:fk-column-name (.getString rs "FKCOLUMN_NAME")
:dest-table {:name (.getString rs "PKTABLE_NAME")
:schema (.getString rs "PKTABLE_SCHEM")}
:dest-column-name (.getString rs "PKCOLUMN_NAME")})))))
(defn describe-table-fks
"Default implementation of `driver/describe-table-fks` for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
[driver db-or-id-or-spec-or-conn table & [db-name-or-nil]]
(if (instance? Connection db-or-id-or-spec-or-conn)
(describe-table-fks* driver db-or-id-or-spec-or-conn table db-name-or-nil)
(let [spec (sql-jdbc.conn/db->pooled-connection-spec db-or-id-or-spec-or-conn)]
(with-open [conn (jdbc/get-connection spec)]
(describe-table-fks* driver conn table db-name-or-nil)))))
src/metabase/driver/sql_jdbc/sync/interface.clj 0 → 100644
+ 56
− 0
View file @ a81ae199
(ns metabase.driver.sql-jdbc.sync.interface
(:require [metabase.driver :as driver]))
(defmulti active-tables
"Return a reducible sequence of maps containing information about the active tables/views, collections, or equivalent
that currently exist in a database. Each map should contain the key `:name`, which is the string name of the table.
For databases that have a concept of schemas, this map should also include the string name of the table's `:schema`.
Two different implementations are provided in this namespace: `fast-active-tables` (the default), and
`post-filtered-active-tables`. You should be fine using the default, but refer to the documentation for those
functions for more details on the differences.
`metabase` is an instance of `DatabaseMetaData`."
{:arglists '([driver ^java.sql.Connection connection])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(defmulti excluded-schemas
"Return set of string names of schemas to skip syncing tables from."
{:arglists '([driver])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(defmulti have-select-privilege?
"Check if we have SELECT privileges for given `table`."
{:arglists '([driver ^java.sql.Connection connection ^String table-schema ^String table-name])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(defmulti database-type->base-type
"Given a native DB column type (as a keyword), return the corresponding `Field` `base-type`, which should derive from
`:type/*`. You can use `pattern-based-database-type->base-type` in this namespace to implement this using regex
patterns."
{:arglists '([driver database-type])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(defmulti column->special-type
"Attempt to determine the special-type of a field given the column name and native type. For example, the Postgres
driver can mark Postgres JSON type columns as `:type/SerializedJSON` special type.
`database-type` and `column-name` will be strings."
{:arglists '([driver database-type column-name])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
(defmulti fallback-metadata-query
"SELECT columns from a given table so we can get column metadata. By default doesn't return any rows. This can be
overriden because SQLite is silly and only returns column information for views if the query returns a non-zero
number of rows.
(fallback-metadata-query :postgres \"public\" \"my_table\")
;; -> [\"SELECT * FROM public.my_table WHERE 1 <> 1 LIMIT 0\"]"
{:arglists '([driver schema table])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
src/metabase/plugins/jdbc_proxy.clj
+ 25
− 13
View file @ a81ae199
......@@ -11,6 +11,7 @@
;;; -------------------------------------------------- Proxy Driver --------------------------------------------------
;; TODO -- why not use `java.sql.Wrapper` here instead of defining a new protocol that basically does the same thing?
(p.types/defprotocol+ ^:private ProxyDriver
(wrapped-driver [this]
"Get the JDBC driver wrapped by a Metabase JDBC proxy driver."))
......@@ -42,17 +43,28 @@
(.jdbcCompliant driver))))
(defn create-and-register-proxy-driver!
"Create a new JDBC proxy driver to wrap driver with `class-name`. Registers the driver with JDBC, and deregisters the
class it wraps if that class is already registered."
"Create a new JDBC proxy driver to wrap driver with `class-name`, but only if that class WAS NOT loaded by the System
ClassLoader. Registers the driver with JDBC, and deregisters the class it wraps if that class is already
registered.
This is necessary because the DriverManager will not recognize any drivers that are *NOT* loaded by the System
ClassLoader."
[^String class-name]
(let [klass (Class/forName class-name true (classloader/the-classloader))
driver (proxy-driver (.newInstance klass))]
(log/debug (u/format-color 'blue (trs "Registering JDBC proxy driver for {0}..." klass)))
(DriverManager/registerDriver driver)
;; deregister the non-proxy version of the driver so it doesn't try to handle our URLs. Most JDBC drivers register
;; themseleves when the classes are loaded
(doseq [driver (enumeration-seq (DriverManager/getDrivers))
:when (instance? klass driver)]
(log/debug (u/format-color 'cyan (trs "Deregistering original JDBC driver {0}..." driver)))
(DriverManager/deregisterDriver driver))))
(let [klass (Class/forName class-name true (classloader/the-classloader))
loaded-by-system-classloader? (identical? (.getClassLoader klass) (ClassLoader/getSystemClassLoader))]
;; if the System ClassLoader loaded this class, don't create the proxy driver, because that can break things in
;; some situations -- Oracle for example doesn't seem to behave properly when you do this. This mainly affects dev
;; which merges driver dependencies into the core project deps.
(if loaded-by-system-classloader?
(log/debug (u/format-color 'cyan (trs "Not creating proxy JDBC driver for class {0} -- original driver was loaded by system ClassLoader"
class-name)))
(let [driver (proxy-driver (.newInstance klass))]
(log/debug (u/format-color 'blue (trs "Registering JDBC proxy driver for {0}..." class-name)))
(DriverManager/registerDriver driver)
;; deregister the non-proxy version of the driver so it doesn't try to handle our URLs. Most JDBC drivers register
;; themseleves when the classes are loaded
(doseq [driver (enumeration-seq (DriverManager/getDrivers))
:when (instance? klass driver)]
(log/debug (u/format-color 'cyan (trs "Deregistering original JDBC driver {0}..." driver)))
(DriverManager/deregisterDriver driver))))))
src/metabase/sync/interface.clj
+ 0
− 1
View file @ a81ae199
......@@ -20,7 +20,6 @@
"Schema for the expected output of `describe-database`."
{:tables #{DatabaseMetadataTable}})
(def TableMetadataField
"Schema for a given Field as provided in `describe-table`."
{:name su/NonBlankString
......
src/metabase/sync/sync_metadata/fields/fetch_metadata.clj
+ 3
− 3
View file @ a81ae199
......@@ -80,7 +80,7 @@
;;; +----------------------------------------------------------------------------------------------------------------+
(s/defn db-metadata :- #{i/TableMetadataField}
"Fetch metadata about Fields belonging to a given TABLE directly from an external database by calling its
driver's implementation of `describe-table`."
[database :- i/DatabaseInstance, table :- i/TableInstance]
"Fetch metadata about Fields belonging to a given `table` directly from an external database by calling its driver's
implementation of `describe-table`."
[database :- i/DatabaseInstance table :- i/TableInstance]
(:fields (fetch-metadata/table-metadata database table)))
src/metabase/util.clj
+ 1
− 1
View file @ a81ae199
......@@ -204,7 +204,7 @@
and [Common Lisp](http://www.lispworks.com/documentation/HyperSpec/Body/m_prog1c.htm#prog1).
Style note: Prefer `doto` when appropriate, e.g. when dealing with Java objects."
{:style/indent 1}
{:style/indent :defn}
[first-form & body]
`(let [~'<> ~first-form]
~@body
......
test/metabase/driver/postgres_test.clj
+ 20
− 0
View file @ a81ae199
......@@ -531,3 +531,23 @@
(mt/rows
(mt/run-mbql-query venues
{:filter [:= $id "2"]})))))))
(deftest dont-sync-tables-with-no-select-permissions-test
(testing "Make sure we only sync databases for which the current user has SELECT permissions"
(mt/test-driver :postgres
(drop-if-exists-and-create-db! "no-select-test")
(let [details (mt/dbdef->connection-details :postgres :db {:database-name "no-select-test"})
spec (sql-jdbc.conn/connection-details->spec :postgres details)]
(doseq [statement ["CREATE TABLE PUBLIC.table_with_perms (x INTEGER NOT NULL);"
"CREATE TABLE PUBLIC.table_with_no_perms (y INTEGER NOT NULL);"
"DROP USER IF EXISTS no_select_test_user;"
"CREATE USER no_select_test_user WITH PASSWORD '123456';"
"GRANT SELECT ON TABLE \"no-select-test\".PUBLIC.table_with_perms TO no_select_test_user;"]]
(jdbc/execute! spec [statement])))
(let [test-user-details (assoc (mt/dbdef->connection-details :postgres :db {:database-name "no-select-test"})
:user "no_select_test_user"
:password "123456")]
(mt/with-temp Database [database {:engine :postgres, :details test-user-details}]
(sync/sync-database! database)
(is (= #{"table_with_perms"}
(db/select-field :name Table :db_id (:id database)))))))))
test/metabase/driver/sql_jdbc/sync_test.clj → test/metabase/driver/sql_jdbc/sync/describe_database_test.clj
+ 106
− 0
View file @ a81ae199
(ns metabase.driver.sql-jdbc.sync-test
(:require [clojure
[string :as str]
[test :refer :all]]
[clojure.java.jdbc :as jdbc]
(ns metabase.driver.sql-jdbc.sync.describe-database-test
(:require [clojure.java.jdbc :as jdbc]
[clojure.test :refer :all]
[metabase
[driver :as driver]
[query-processor :as qp]
[sync :as sync]
[test :as mt]]
[metabase.driver.sql-jdbc
[connection :as sql-jdbc.conn]
[sync :as sql-jdbc.sync]]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql-jdbc.sync
[describe-database :as describe-database]
[interface :as i]]
[metabase.models.table :refer [Table]]
[metabase.test.data.one-off-dbs :as one-off-dbs]
[toucan.db :as db])
(:import java.sql.ResultSet))
(deftest simple-select-probe-query-test
(testing "simple-select-probe-query shouldn't actually return any rows"
(let [{:keys [name schema]} (Table (mt/id :venues))]
(is (= []
(mt/rows
(qp/process-query
(let [[sql] (describe-database/simple-select-probe-query (or driver/*driver* :h2) schema name)]
(mt/native-query {:query sql})))))))))
(defn- sql-jdbc-drivers-with-default-describe-database-impl
"All SQL JDBC drivers that use the default SQL JDBC implementation of `describe-database`. (As far as I know, this is
all of them.)"
......@@ -25,14 +33,30 @@
#(identical? (get-method driver/describe-database :sql-jdbc) (get-method driver/describe-database %))
(descendants driver/hierarchy :sql-jdbc))))
(defn- sql-jdbc-drivers-with-default-describe-table-impl
"All SQL JDBC drivers that use the default SQL JDBC implementation of `describe-table`. (As far as I know, this is
all of them.)"
[]
(set
(filter
#(identical? (get-method driver/describe-table :sql-jdbc) (get-method driver/describe-table %))
(descendants driver/hierarchy :sql-jdbc))))
(deftest fast-active-tables-test
(let [spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
(with-open [conn (jdbc/get-connection spec)]
;; We have to mock this to make it work with all DBs
(with-redefs [describe-database/all-schemas (constantly #{"PUBLIC"})]
(is (= ["CATEGORIES" "CHECKINS" "USERS" "VENUES"]
(->> (into [] (describe-database/fast-active-tables (or driver/*driver* :h2) conn))
(map :name)
sort)))))))
(deftest post-filtered-active-tables-test
(let [spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
(with-open [conn (jdbc/get-connection spec)]
(is (= ["CATEGORIES" "CHECKINS" "USERS" "VENUES"]
(->> (into [] (describe-database/post-filtered-active-tables :h2 conn))
(map :name)
sort))))))
(deftest describe-database-test
(is (= {:tables #{{:name "USERS", :schema "PUBLIC", :description nil}
{:name "VENUES", :schema "PUBLIC", :description nil}
{:name "CATEGORIES", :schema "PUBLIC", :description nil}
{:name "CHECKINS", :schema "PUBLIC", :description nil}}}
(describe-database/describe-database :h2 (mt/id)))))
(defn- describe-database-with-open-resultset-count
"Just like `describe-database`, but instead of returning the database description returns the number of ResultSet
......@@ -46,11 +70,11 @@
(with-redefs [jdbc/result-set-seq (fn [^ResultSet rs & more]
(swap! resultsets conj rs)
(apply orig-result-set-seq rs more))]
;; taking advantage of the fact that `sql-jdbc.sync/describe-database` can accept JBDC connections instead of
;; taking advantage of the fact that `describe-database/describe-database` can accept JBDC connections instead of
;; databases; by doing this we can keep the connection open and check whether resultsets are still open before
;; they would normally get closed
(jdbc/with-db-connection [conn (sql-jdbc.conn/db->pooled-connection-spec db)]
(sql-jdbc.sync/describe-database driver conn)
(describe-database/describe-database driver conn)
(reduce + (for [^ResultSet rs @resultsets]
(if (.isClosed rs) 0 1)))))))
......@@ -69,7 +93,7 @@
(sync/sync-database! (mt/db))
(is (= 1 (count-active-tables-in-db (mt/id))))
;; We have to mock this as H2 doesn't have the notion of a user connecting to it
(with-redefs [sql-jdbc.sync/have-select-privilege? (constantly false)]
(with-redefs [i/have-select-privilege? (constantly false)]
(sync/sync-database! (mt/db))
(is (= 0 (count-active-tables-in-db (mt/id)))
"We shouldn't sync tables for which we don't have select privilege"))))
......@@ -80,52 +104,3 @@
"See issues #4389, #6028, and #6467 (Oracle) and #7609 (Redshift)")
(is (= 0
(describe-database-with-open-resultset-count driver/*driver* (mt/db)))))))
(deftest simple-select-probe-test
(let [{:keys [name schema]} (Table (mt/id :venues))]
(is (= [[1 "Red Medicine" 4 10.0646 -165.374 3]]
(mt/rows
(qp/process-query
(mt/native-query {:query (first (#'sql-jdbc.sync/simple-select-probe (or driver/*driver* :h2) schema name))})))))))
(deftest database-types-fallback-test
(mt/test-drivers (sql-jdbc-drivers-with-default-describe-table-impl)
(let [org-result-set-seq jdbc/result-set-seq]
(with-redefs [jdbc/result-set-seq (fn [& args]
(map #(dissoc % :type_name) (apply org-result-set-seq args)))]
(is (= #{{:name "longitude" :base-type :type/Float}
{:name "category_id" :base-type :type/Integer}
{:name "price" :base-type :type/Integer}
{:name "latitude" :base-type :type/Float}
{:name "name" :base-type :type/Text}
{:name "id" :base-type :type/Integer}}
(->> (sql-jdbc.sync/describe-table driver/*driver* (mt/id) (Table (mt/id :venues)))
:fields
(map (fn [{:keys [name base-type]}]
{:name (str/lower-case name)
:base-type (if (or (isa? base-type :type/Integer)
(isa? base-type :type/Decimal)) ; H2 DBs returns the ID as BigInt, Oracle as Decimal;
:type/Integer
base-type)}))
set)))))))
(deftest calculated-special-type-test
(mt/test-drivers (sql-jdbc-drivers-with-default-describe-table-impl)
(with-redefs [sql-jdbc.sync/column->special-type (fn [_ _ column-name]
(when (= (str/lower-case column-name) "longitude")
:type/Longitude))]
(is (= [["longitude" :type/Longitude]]
(->> (sql-jdbc.sync/describe-table (or driver/*driver* :h2) (mt/id) (Table (mt/id :venues)))
:fields
(filter :special-type)
(map (juxt (comp str/lower-case :name) :special-type))))))))
(deftest fast-active-tables-test
(jdbc/with-db-metadata [metadata (sql-jdbc.conn/db->pooled-connection-spec (mt/db))]
;; We have to mock this to make it work with all DBs
(with-redefs [sql-jdbc.sync/all-schemas (constantly #{"PUBLIC"})]
(is (= ["CATEGORIES" "CHECKINS" "USERS" "VENUES"]
(->> metadata
(sql-jdbc.sync/fast-active-tables (or driver/*driver* :h2) (mt/db))
(map :table_name)
sort))))))
test/metabase/driver/sql_jdbc/sync/describe_table_test.clj 0 → 100644
+ 73
− 0
View file @ a81ae199
(ns metabase.driver.sql-jdbc.sync.describe-table-test
(:require [clojure
[string :as str]
[test :refer :all]]
[clojure.java.jdbc :as jdbc]
[metabase
[driver :as driver]
[test :as mt]]
[metabase.driver.sql-jdbc.sync
[describe-table :as describe-table]
[interface :as i]]
[metabase.models.table :refer [Table]]))
(defn- sql-jdbc-drivers-with-default-describe-table-impl
"All SQL JDBC drivers that use the default SQL JDBC implementation of `describe-table`. (As far as I know, this is
all of them.)"
[]
(set
(filter
#(identical? (get-method driver/describe-table :sql-jdbc) (get-method driver/describe-table %))
(descendants driver/hierarchy :sql-jdbc))))
(deftest describe-table-test
(is (= {:name "VENUES",
:fields
#{{:name "ID", :database-type "BIGINT", :base-type :type/BigInteger, :database-position 0, :pk? true}
{:name "NAME", :database-type "VARCHAR", :base-type :type/Text, :database-position 1}
{:name "CATEGORY_ID", :database-type "INTEGER", :base-type :type/Integer, :database-position 2}
{:name "LATITUDE", :database-type "DOUBLE", :base-type :type/Float, :database-position 3}
{:name "LONGITUDE", :database-type "DOUBLE", :base-type :type/Float, :database-position 4}
{:name "PRICE", :database-type "INTEGER", :base-type :type/Integer, :database-position 5}}}
(describe-table/describe-table :h2 (mt/id) {:name "VENUES"}))))
(deftest describe-table-fks-test
(is (= #{{:fk-column-name "CATEGORY_ID"
:dest-table {:name "CATEGORIES", :schema "PUBLIC"}
:dest-column-name "ID"}}
(describe-table/describe-table-fks :h2 (mt/id) {:name "VENUES"})))
(is (= #{{:fk-column-name "USER_ID", :dest-table {:name "USERS", :schema "PUBLIC"}, :dest-column-name "ID"}
{:fk-column-name "VENUE_ID", :dest-table {:name "VENUES", :schema "PUBLIC"}, :dest-column-name "ID"}}
(describe-table/describe-table-fks :h2 (mt/id) {:name "CHECKINS"}))))
(deftest database-types-fallback-test
(mt/test-drivers (sql-jdbc-drivers-with-default-describe-table-impl)
(let [org-result-set-seq jdbc/result-set-seq]
(with-redefs [jdbc/result-set-seq (fn [& args]
(map #(dissoc % :type_name) (apply org-result-set-seq args)))]
(is (= #{{:name "longitude" :base-type :type/Float}
{:name "category_id" :base-type :type/Integer}
{:name "price" :base-type :type/Integer}
{:name "latitude" :base-type :type/Float}
{:name "name" :base-type :type/Text}
{:name "id" :base-type :type/Integer}}
(->> (describe-table/describe-table driver/*driver* (mt/id) (Table (mt/id :venues)))
:fields
(map (fn [{:keys [name base-type]}]
{:name (str/lower-case name)
:base-type (if (or (isa? base-type :type/Integer)
(isa? base-type :type/Decimal)) ; H2 DBs returns the ID as BigInt, Oracle as Decimal;
:type/Integer
base-type)}))
set)))))))
(deftest calculated-special-type-test
(mt/test-drivers (sql-jdbc-drivers-with-default-describe-table-impl)
(with-redefs [i/column->special-type (fn [_ _ column-name]
(when (= (str/lower-case column-name) "longitude")
:type/Longitude))]
(is (= [["longitude" :type/Longitude]]
(->> (describe-table/describe-table (or driver/*driver* :h2) (mt/id) (Table (mt/id :venues)))
:fields
(filter :special-type)
(map (juxt (comp str/lower-case :name) :special-type))))))))
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