Skip to content
Snippets Groups Projects
Unverified Commit 04eb3949 authored by Ngoc Khuat's avatar Ngoc Khuat Committed by GitHub
Browse files

Add fingerprint as a driver feature (#40155)

parent 22880c82
No related branches found
No related tags found
No related merge requests found
......@@ -20,6 +20,7 @@
(driver/register! :googleanalytics)
(defmethod driver/database-supports? [:googleanalytics :basic-aggregations] [_driver _feature _db] false)
(defmethod driver/database-supports? [:googleanalytics :fingerprint] [_driver _feature _db] false)
;;; ----------------------------------------------- describe-database ------------------------------------------------
......
......@@ -15,6 +15,7 @@
[metabase.query-processor.compile :as qp.compile]
[metabase.query-processor.pipeline :as qp.pipeline]
[metabase.query-processor.store :as qp.store]
[metabase.sync.analyze.fingerprint :as fingerprint]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.test.util :as tu]
......@@ -47,6 +48,14 @@
:id 1}]})
(driver/mbql->native :googleanalytics (update query :query (partial merge {:source-table 1})))))
(deftest test-fingerprint-skipped-for-ga
(testing "Google Analytics doesn't support fingerprinting fields"
(let [fake-db (-> (mt/db)
(assoc :engine :googleanalytics))]
(with-redefs [fingerprint/fingerprint-table! (fn [_] (throw (Exception. "this should not be called!")))]
(is (= (fingerprint/empty-stats-map 0)
(fingerprint/fingerprint-fields-for-db! fake-db [(t2/select-one Table :id (mt/id :venues))] (fn [_ _]))))))))
(deftest ^:parallel basic-compilation-test
(testing "just check that a basic almost-empty MBQL query can be compiled"
(is (= (ga-query {})
......
......@@ -586,7 +586,10 @@
;; Does the driver support a faster `sync-fields` step by fetching all FK metadata in a single collection?
;; if so, `metabase.driver/describe-fields` must be implemented instead of `metabase.driver/describe-table`
:describe-fields})
:describe-fields
;; Does the driver support fingerprint the fields. Default is true
:fingerprint})
(defmulti database-supports?
"Does this driver and specific instance of a database support a certain `feature`?
......@@ -620,7 +623,8 @@
:date-arithmetics true
:temporal-extract true
:schemas true
:test/jvm-timezone-setting true}]
:test/jvm-timezone-setting true
:fingerprint true}]
(defmethod database-supports? [::driver feature] [_driver _feature _db] supported?))
(defmulti ^String escape-alias
......
......@@ -212,7 +212,6 @@
log-progress-fn :- LogProgressFn]
(fingerprint-fields-for-db!* database tables log-progress-fn (constantly true)))
;; TODO: Maybe the driver should have a function to tell you if it supports fingerprinting?
([database :- i/DatabaseInstance
tables :- [:maybe [:sequential i/TableInstance]]
log-progress-fn :- LogProgressFn
......@@ -220,10 +219,7 @@
(qp.store/with-metadata-provider (u/the-id database)
(reduce (fn [acc table]
(log-progress-fn (if *refingerprint?* "refingerprint-fields" "fingerprint-fields") table)
(let [results (if (= :googleanalytics (:engine database))
(empty-stats-map 0)
(fingerprint-fields! table))
new-acc (merge-with + acc results)]
(let [new-acc (merge-with + acc (fingerprint-fields! table))]
(if (continue? new-acc)
new-acc
(reduced new-acc))))
......@@ -235,8 +231,9 @@
[database :- i/DatabaseInstance
tables :- [:maybe [:sequential i/TableInstance]]
log-progress-fn :- LogProgressFn]
;; TODO: Maybe the driver should have a function to tell you if it supports fingerprinting?
(fingerprint-fields-for-db!* database tables log-progress-fn))
(if (driver/database-supports? (:engine database) :fingerprint database)
(fingerprint-fields-for-db!* database tables log-progress-fn)
(empty-stats-map 0)))
(def ^:private max-refingerprint-field-count
"Maximum number of fields to refingerprint. Balance updating our fingerprinting values while not spending too much
......
......@@ -271,14 +271,6 @@
(is (= (fingerprint/empty-stats-map 0)
(fingerprint/fingerprint-fields! (t2/select-one Table :id (data/id :venues))))))))
(deftest test-fingerprint-skipped-for-ga
(testing "Google Analytics doesn't support fingerprinting fields"
(let [fake-db (-> (data/db)
(assoc :engine :googleanalytics))]
(with-redefs [fingerprint/fingerprint-table! (fn [_] (throw (Exception. "this should not be called!")))]
(is (= (fingerprint/empty-stats-map 0)
(fingerprint/fingerprint-fields-for-db! fake-db [(t2/select-one Table :id (data/id :venues))] (fn [_ _]))))))))
(deftest fingerprint-test
(mt/test-drivers (mt/normal-drivers)
(testing "Fingerprints should actually get saved with the correct values"
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment