diff --git a/modules/drivers/googleanalytics/src/metabase/driver/googleanalytics.clj b/modules/drivers/googleanalytics/src/metabase/driver/googleanalytics.clj index d78e56c56252394c334176dcd6fd9d2fd1da3a93..190b711aa51d39b1370177b008f0d6d27f2f8c72 100644 --- a/modules/drivers/googleanalytics/src/metabase/driver/googleanalytics.clj +++ b/modules/drivers/googleanalytics/src/metabase/driver/googleanalytics.clj @@ -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 ------------------------------------------------ diff --git a/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj b/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj index 47fc480721af04326b6d1f8ace4e474a318fa1ca..d8a0d3bddc564192199b5ddb070538930ba9ced6 100644 --- a/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj +++ b/modules/drivers/googleanalytics/test/metabase/driver/googleanalytics_test.clj @@ -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 {}) diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj index 4b2af349a75ffc19491dd8e65714e2476b67bf8e..fb3024a05540c379c7c1fcd20646356f95633ae4 100644 --- a/src/metabase/driver.clj +++ b/src/metabase/driver.clj @@ -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 diff --git a/src/metabase/sync/analyze/fingerprint.clj b/src/metabase/sync/analyze/fingerprint.clj index 71141a17d91fe43411eee3883368185d6d45ee92..f8cd2000ab57cb69d4ab7056a262d7b9ae78d0ae 100644 --- a/src/metabase/sync/analyze/fingerprint.clj +++ b/src/metabase/sync/analyze/fingerprint.clj @@ -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 diff --git a/test/metabase/sync/analyze/fingerprint_test.clj b/test/metabase/sync/analyze/fingerprint_test.clj index 5a4c0d66ac4352a121a2145b2da1d8a50f22f6b4..78007f6565a9cf0ba1b05bcd001c7dcb5e147964 100644 --- a/test/metabase/sync/analyze/fingerprint_test.clj +++ b/test/metabase/sync/analyze/fingerprint_test.clj @@ -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"