As a fallback get database type from select * (#11358)

frontend/src/metabase/dashboard/dashboard.js

@@ -1165,9 +1165,12 @@ const loadingDashCards = handleActions(
 
 const loadMetadataForDashboard = dashCards => (dispatch, getState) => {
   const metadata = getMetadata(getState());
+
   const queries = dashCards
-    .flatMap(dc => [dc.card].concat(dc.series))
+    .filter(dc => !isVirtualDashCard(dc))
+    .flatMap(dc => [dc.card].concat(dc.series || []))
     .map(card => new Question(card, metadata).query());
+
   return dispatch(loadMetadataForQueries(queries));
 };
 

modules/drivers/sqlite/test/metabase/driver/sqlite_test.clj

@@ -6,13 +6,21 @@
              [query-processor-test :as qp.test]
              [sync :as sync]
              [test :as mt]]
-            [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+            [metabase.driver.sql-jdbc
+             [connection :as sql-jdbc.conn]
+             [execute :as sql-jdbc.execute]]
             [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]]))
+             [util :as tu]]
+            [metabase.test.data.datasets :as datasets]
+            [toucan
+             [db :as db]
+             [hydrate :refer [hydrate]]]))
 
 (deftest timezone-id-test
   (mt/test-driver :sqlite
@@ -41,9 +49,74 @@
                 :filter   [:between $date "2014-03-04" "2014-03-07"]
                 :order-by [[:asc $date]]}))))))
 
+(defn- table-fingerprint
+  [{:keys [fields name]}]
+  {:name   name
+   :fields (map #(select-keys % [:name :base_type]) fields)})
+
+(deftest type-inference-for-views
+  (mt/test-driver :sqlite
+    (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);"
+                             "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]))
+          (sync/sync-database! database)
+          (is (= [{:name "src"
+                   :fields [{:name      "id"
+                             :base_type :type/Integer}
+                            {:name      "time"
+                             :base_type :type/Text}]}
+                  {:name "v_src"
+                   :fields [{:name      "id"
+                             :base_type :type/Integer}
+                            {:name      "time"
+                             :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,
+                             symbol VARCHAR,
+                             dt DATETIME,
+                             value FLOAT);"
+                             "INSERT INTO groupby_test (symbol, dt, value) VALUES ('T', '2020-01-01', 0);"
+                             "INSERT INTO groupby_test (symbol, dt, value) VALUES ('T', '2020-01-02', 2);"
+                             "INSERT INTO groupby_test (symbol, dt, value) VALUES ('T', '2020-01-03', 4);"
+                             "INSERT INTO groupby_test (symbol, dt, value) VALUES ('S', '2019-01-01', 10);"
+                             "CREATE VIEW IF NOT EXISTS v_groupby_test AS
+                              SELECT
+                               symbol,
+                               sum(value) as totalValue
+                              FROM groupby_test
+                              GROUP BY symbol
+                              ORDER by dt;"]]
+            (jdbc/execute! spec [statement]))
+          (sync/sync-database! database)
+          (is (= [{:name "groupby_test"
+                   :fields [{:name      "id"
+                             :base_type :type/Integer}
+                            {:name      "symbol"
+                             :base_type :type/Text}
+                            {:name      "dt"
+                             :base_type :type/DateTime}
+                            {:name      "value"
+                             :base_type :type/Float}]}
+                  {:name "v_groupby_test"
+                   :fields [{:name      "symbol"
+                             :base_type :type/Text}
+                            {:name      "totalValue"
+                             :base_type :type/Float}]}]
+                 (->> (hydrate (db/select Table :db_id db-id
+                                          {:where    [:in :name ["groupby_test" "v_groupby_test"]]
+                                           :order-by [:name]}) :fields)
+                      (map table-fingerprint)))))))))
+
 (defn- default-table-result [table-name]
-  {:name table-name
-   :schema nil
+  {:name        table-name
+   :schema      nil
    :description nil})
 
 (deftest timestamp-test-db

src/metabase/driver/sql_jdbc/sync.clj

@@ -9,8 +9,10 @@
             [metabase
              [driver :as driver]
              [util :as u]]
-            [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn])
-  (:import java.sql.DatabaseMetaData))
+            [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)                                            |
@@ -81,6 +83,11 @@
           (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                                                    |
@@ -150,23 +157,50 @@
       (str "Invalid type: " special-type))
     special-type))
 
+(defn simple-select-probe
+  "Perform a simple (and cheap) SELECT on a given table to test for access and get metadata."
+  [driver schema table]
+  (first
+   (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}))))
+
+(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 schema 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)
+                                                 (.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, {^String schema :schema, ^String table-name :name}, & [^String db-name-or-nil]]
-  (with-open [rs (.getColumns metadata db-name-or-nil schema table-name nil)]
-    (set
-     (for [[idx {database-type :type_name
-                 column-name   :column_name
-                 remarks       :remarks}] (m/indexed (jdbc/metadata-result rs))]
-       (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}))))))
+  [^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."
@@ -184,11 +218,6 @@
 ;;; |                            Default SQL JDBC metabase.driver impls for sync methods                             |
 ;;; +----------------------------------------------------------------------------------------------------------------+
 
-(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))
-
 (defn describe-database
   "Default implementation of `driver/describe-database` for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
   [driver db-or-id-or-spec]

test/metabase/driver/sql_jdbc/sync_test.clj

 (ns metabase.driver.sql-jdbc.sync-test
-  (:require [clojure.java.jdbc :as jdbc]
-            [clojure.test :refer :all]
+  (:require [clojure
+             [string :as str]
+             [test :refer :all]]
+            [clojure.java.jdbc :as jdbc]
             [metabase
              [driver :as driver]
+             [query-processor :as qp]
              [test :as mt]]
             [metabase.driver.sql-jdbc
              [connection :as sql-jdbc.conn]
-             [sync :as sql-jdbc.sync]])
+             [sync :as sql-jdbc.sync]]
+            [metabase.models.table :refer [Table]])
   (:import java.sql.ResultSet))
 
 (defn- sql-jdbc-drivers-with-default-describe-database-impl
@@ -18,6 +22,15 @@
     #(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))))
+
 (defn- describe-database-with-open-resultset-count
   "Just like `describe-database`, but instead of returning the database description returns the number of ResultSet
   objects the sync process left open. Make sure you wrap ResultSets with `with-open`! Otherwise some JDBC drivers like
@@ -44,3 +57,42 @@
                   "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 (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))))))))