Additional tests for generic SQL introspection functions

src/metabase/driver/generic_sql.clj

@@ -85,7 +85,6 @@
 
         (hsql/format ... :quoting (quote-style driver))")
 
-
   (set-timezone-sql ^String [this]
     "*OPTIONAL*. This should be a prepared JDBC SQL statement string to be used to set the timezone for the current transaction.
 
@@ -195,8 +194,7 @@
   "Execute a HONEYSQL-FROM query against DATABASE, DRIVER, and optionally TABLE."
   ([driver database honeysql-form]
    (jdbc/query (db->jdbc-connection-spec database)
-               (u/prog1 (honeysql-form->sql+args driver honeysql-form)
-                 (println "SQL:" <>))))
+               (honeysql-form->sql+args driver honeysql-form)))
   ([driver database table honeysql-form]
    (query driver database (merge {:from [(qualify+escape table)]}
                                  honeysql-form))))
@@ -245,11 +243,12 @@
   (let [table       (field/table field)
         db          (table/database table)
         field-k     (qualify+escape table field)
-        total-count (:count (first (query driver db table {:select [(hsql/call :count field-k)]
+        total-count (:count (first (query driver db table {:select [[(hsql/call :count field-k) :count]]
                                                            :where  [:not= field-k nil]})))
-        url-count   (:count (first (query driver db table {:select [(hsql/call :count field-k)]
+        url-count   (:count (first (query driver db table {:select [[(hsql/call :count field-k) :count]]
                                                            :where  [:like field-k (hx/literal "http%://_%.__%")]})))]
-    (if (and (> total-count 0)
+    (if (and total-count
+             (> total-count 0)
              url-count)
       (float (/ url-count total-count))
       0.0)))

src/metabase/driver/generic_sql/query_processor.clj

@@ -289,7 +289,7 @@
          (catch Throwable e
            (log/error (u/format-color 'red "Failed to set timezone: %s" (.getMessage e)))))))
 
-(defn- query!
+(defn- run-query
   "Run the query itself."
   [{sql :query, params :params} connection]
   (let [sql              (hx/unescape-dots sql)
@@ -318,7 +318,6 @@
   (try (f)
        (finally (.rollback connection))))
 
-;; TODO - maybe this should be called `execute-query!` ?
 (defn execute-query
   "Process and run a native (raw SQL) QUERY."
   [driver {:keys [database settings], query :native}]
@@ -329,4 +328,4 @@
           (do-with-auto-commit-disabled transaction-connection
             (fn []
               (maybe-set-timezone! driver settings transaction-connection)
-              (query! query transaction-connection))))))))
+              (run-query query transaction-connection))))))))

test/metabase/driver/generic_sql_test.clj

@@ -2,24 +2,27 @@
   (:require [expectations :refer :all]
             (metabase [db :as db]
                       [driver :as driver])
-            [metabase.driver.generic-sql :refer :all]
+            (metabase.driver [generic-sql :refer :all]
+                             h2)
             (metabase.models [field :refer [Field]]
                              [table :refer [Table], :as table])
             [metabase.test.data :refer :all]
+            (metabase.test.data [dataset-definitions :as defs]
+                                [datasets :as datasets])
             [metabase.test.util :refer [resolve-private-fns]])
   (:import metabase.driver.h2.H2Driver))
 
-(def users-table
-  (delay (Table :name "USERS")))
+(def ^:private users-table      (delay (Table :name "USERS")))
+(def ^:private venues-table     (delay (Table (id :venues))))
+(def ^:private users-name-field (delay (Field (id :users :name))))
 
-(def venues-table
-  (delay (Table (id :venues))))
-
-(def korma-users-table
-  (delay (korma-entity @users-table)))
-
-(def users-name-field
-  (delay (Field (id :users :name))))
+(def ^:private generic-sql-engines
+  (set (for [engine datasets/all-valid-engines
+             :let   [driver (driver/engine->driver engine)]
+             :when  (not= engine :bigquery)                                       ; bigquery doesn't use the generic sql implementations of things like `field-avg-length`
+             :when  (extends? ISQLDriver (class driver))]
+         (do (require (symbol (str "metabase.test.data." (name engine))) :reload) ; otherwise it gets all snippy if you try to do `lein test metabase.driver.generic-sql-test`
+             engine))))
 
 
 ;; DESCRIBE-DATABASE
@@ -75,3 +78,37 @@
                {:id (id :venues :name), :values nil}
                {:id (id :venues :price), :values [1 2 3 4]}]}
   (driver/analyze-table (H2Driver.) @venues-table (set (mapv :id (table/fields @venues-table)))))
+
+(resolve-private-fns metabase.driver.generic-sql field-avg-length field-values-lazy-seq table-rows-seq field-percent-urls)
+
+;;; FIELD-AVG-LENGTH
+(datasets/expect-with-engines generic-sql-engines
+  16
+  (field-avg-length datasets/*data-loader* (db/select-one 'Field :id (id :venues :name))))
+
+;;; FIELD-VALUES-LAZY-SEQ
+(datasets/expect-with-engines generic-sql-engines
+  ["Red Medicine"
+   "Stout Burgers & Beers"
+   "The Apple Pan"
+   "Wurstküche"
+   "Brite Spot Family Restaurant"]
+  (take 5 (field-values-lazy-seq datasets/*data-loader* (db/select-one 'Field :id (id :venues :name)))))
+
+
+;;; TABLE-ROWS-SEQ
+(datasets/expect-with-engines generic-sql-engines
+  [{:name "Red Medicine",                 :latitude 10.0646, :longitude -165.374, :price 3, :category_id  4, :id 1}
+   {:name "Stout Burgers & Beers",        :latitude 34.0996, :longitude -118.329, :price 2, :category_id 11, :id 2}
+   {:name "The Apple Pan",                :latitude 34.0406, :longitude -118.428, :price 2, :category_id 11, :id 3}
+   {:name "Wurstküche",                   :latitude 33.9997, :longitude -118.465, :price 2, :category_id 29, :id 4}
+   {:name "Brite Spot Family Restaurant", :latitude 34.0778, :longitude -118.261, :price 2, :category_id 20, :id 5}]
+  (take 5 (table-rows-seq datasets/*data-loader*
+                          (db/select-one 'Database :id (id))
+                          (db/select-one 'Table :id (id :venues)))))
+
+;;; FIELD-PERCENT-URLS
+(datasets/expect-with-engines generic-sql-engines
+  0.5
+  (dataset half-valid-urls
+    (field-percent-urls datasets/*data-loader* (db/select-one 'Field :id (id :urls :url)))))

test/metabase/test/data/dataset_definitions.clj

@@ -11,7 +11,7 @@
 ;; TODO - move this to interface
 ;; TODO - make rows be lazily loadable for DB definitions from a file
 (defmacro ^:private def-database-definition-edn [dbname]
-  `(def-database-definition ~(vary-meta dbname assoc :const true)
+  `(def-database-definition ~dbname
      ~@(edn/read-string (slurp (str edn-definitions-dir (name dbname) ".edn")))))
 
 ;; The O.G. "Test Database" dataset
@@ -25,5 +25,8 @@
 
 (def-database-definition-edn geographical-tips)
 
+;; A tiny dataset where half the NON-NULL values are valid URLs
+(def-database-definition-edn half-valid-urls)
+
 ;; A very tiny dataset with a list of places and a booleans
 (def-database-definition-edn places-cam-likes)

test/metabase/test/data/dataset_definitions/half-valid-urls.edn

+[["urls" [{:field-name "url"
+           :base-type  :TextField}]
+  [["http://www.camsaul.com"]
+   ["http://camsaul.com"]
+   ["https://en.wikipedia.org/wiki/Toucan"]
+   ["ABC"]
+   ["DEF"]
+   [nil]
+   ["https://en.wikipedia.org/wiki/Bird"]
+   ["EFG"]
+   [""]]]]