Merge pull request #9758 from metabase/fix-load-from-h2-2

Fix load-from-h2 behavior for DashboardCards 

src/metabase/cmd/load_from_h2.clj

@@ -14,14 +14,11 @@
    mysql -u root -e 'DROP DATABASE IF EXISTS metabase; CREATE DATABASE metabase;'
    MB_DB_TYPE=mysql MB_DB_HOST=localhost MB_DB_PORT=3305 MB_DB_USER=root MB_DB_DBNAME=metabase lein run load-from-h2
    ```"
-  (:require [clojure
-             [set :as set]
-             [string :as str]]
-            [clojure.java
+  (:require [clojure.java
              [io :as io]
              [jdbc :as jdbc]]
+            [clojure.string :as str]
             [colorize.core :as color]
-            [medley.core :as m]
             [metabase
              [db :as mdb]
              [util :as u]]
@@ -58,6 +55,7 @@
              [table :refer [Table]]
              [user :refer [User]]
              [view-log :refer [ViewLog]]]
+            [metabase.util.i18n :refer [trs]]
             [toucan.db :as db])
   (:import java.sql.SQLException))
 
@@ -120,37 +118,50 @@
 
 ;;; ------------------------------------------- Fetching & Inserting Rows --------------------------------------------
 
-(defn- insert-entity! [target-db-conn {:keys [table], :as entity} objs]
-  ;; TODO - I don't think the print+flush is working as intended :/
-  (print (u/format-color 'blue "Transfering %d instances of %s..." (count objs) (:name entity)))
-  (flush)
-  ;; 1) `:sizeX` and `:sizeY` come out of H2 as `:sizex` and `:sizey` because of automatic lowercasing; fix the
-  ;;    names of these before putting into the new DB
+(defn- objects->colums+values
+  "Given a sequence of objects/rows fetched from the H2 DB, return a the `columns` that should be used in the `INSERT`
+  statement, and a sequence of rows (as seqeunces)."
+  [objs]
+  ;; 1) `:sizeX` and `:sizeY` come out of H2 as `:sizex` and `:sizey` because of automatic lowercasing; fix the names
+  ;;    of these before putting into the new DB
+  ;;
   ;; 2) Need to wrap the column names in quotes because Postgres automatically lowercases unquoted identifiers
-  (let [ks   (keys (set/rename-keys (first objs) {:sizex :sizeX, :sizey :sizeY}))
-        cols (for [k ks]
-               ((db/quote-fn) (name k)))]
-    ;; The connection closes prematurely on occasion when we're inserting thousands of rows at once. Break into
-    ;; smaller chunks so connection stays alive
-    (doseq [chunk (partition-all 300 objs)
-            :let  [vals (for [row chunk]
-                          (map row ks))]]
-      (print (color/blue \.))
-      (flush)
-      (try
-        (jdbc/insert-multi! target-db-conn table cols vals)
-        (catch SQLException e
-          (jdbc/print-sql-exception-chain e)
-          (throw e)))))
+  (let [source-keys (keys (first objs))
+        dest-keys   (for [k source-keys]
+                      ((db/quote-fn) (name (case k
+                                             :sizex :sizeX
+                                             :sizey :sizeY
+                                             k))))]
+    {:cols dest-keys
+     :vals (for [row objs]
+             (map (comp u/jdbc-clob->str row) source-keys))}))
+
+(def ^:private chunk-size 100)
+
+(defn- insert-chunk! [target-db-conn table-name chunkk]
+  (print (color/blue \.))
+  (flush)
+  (try
+    (let [{:keys [cols vals]} (objects->colums+values chunkk)]
+      (jdbc/insert-multi! target-db-conn table-name cols vals))
+    (catch SQLException e
+      (jdbc/print-sql-exception-chain e)
+      (throw e))))
+
+(defn- insert-entity! [target-db-conn {table-name :table, entity-name :name} objs]
+  (print (u/format-color 'blue "Transfering %d instances of %s..." (count objs) entity-name))
+  (flush)
+  ;; The connection closes prematurely on occasion when we're inserting thousands of rows at once. Break into
+  ;; smaller chunks so connection stays alive
+  (doseq [chunk (partition-all chunk-size objs)]
+    (insert-chunk! target-db-conn table-name chunk))
   (println-ok))
 
-
 (defn- load-data! [target-db-conn h2-connection-string-or-nil]
   (jdbc/with-db-connection [h2-conn (h2-details h2-connection-string-or-nil)]
-    (doseq [e     entities
-            :let  [rows (for [row (jdbc/query h2-conn [(str "SELECT * FROM " (name (:table e)))])]
-                          (m/map-vals u/jdbc-clob->str row))]
-            :when (seq rows)]
+    (doseq [{table-name :table, :as e} entities
+            :let                       [rows (jdbc/query h2-conn [(str "SELECT * FROM " (name table-name))])]
+            :when                      (seq rows)]
       (insert-entity! target-db-conn e rows))))
 
 
@@ -225,6 +236,9 @@
   [h2-connection-string-or-nil]
   (mdb/setup-db!)
 
+  (assert (#{:postgres :mysql} (mdb/db-type))
+    (str (trs "Metabase can only transfer data from H2 to Postgres or MySQL/MariaDB.")))
+
   (jdbc/with-db-transaction [target-db-conn (mdb/jdbc-details)]
     (jdbc/db-set-rollback-only! target-db-conn)
 

test/metabase/cmd/load_from_h2_test.clj

 (ns metabase.cmd.load-from-h2-test
   (:require [expectations :refer [expect]]
+            [flatland.ordered.map :as ordered-map]
+            [metabase
+             [db :as mdb]
+             [util :as u]]
             [metabase.cmd.load-from-h2 :as load-from-h2]
-            [metabase.util :as u]
-            [toucan.models :as models]))
+            [toucan
+             [db :as db]
+             [models :as models]]))
 
 ;; Make sure load-from-h2 works with or without `file:` prefix
 (expect
@@ -48,3 +53,40 @@
 (expect
   (all-model-names)
   (migrated-model-names))
+
+;; make sure `objects->colums+values` properly handles the columns with weird casing: `sizeX` and `sizeY`
+(expect
+  {:cols ["\"id\"" "\"row\"" "\"sizeX\"" "\"sizeY\""]
+   :vals [[281 0 18 9]]}
+  (binding [db/*quoting-style* :ansi]
+    (-> (#'load-from-h2/objects->colums+values
+         ;; using ordered-map so the results will be in a predictable order
+         [(ordered-map/ordered-map
+           :id    281
+           :row   0
+           :sizex 18
+           :sizey 9)])
+        (update :cols vec))))
+
+;; make sure `objects->colums+values` properly de-CLOBs and CLOBs (by calling `u/jdbc-clob->str`)
+
+(defrecord ^:private FakeClob [s])
+
+(expect
+  {:cols ["\"created_at\"" "\"description\"" "\"parameter_mappings\"" "\"visualization_settings\""]
+   :vals [[#inst "2019-04-05T21:26:39.936-00:00"
+           "This is a description"
+           []
+           {}]]}
+  (binding [db/*quoting-style* :ansi]
+    (with-redefs [mdb/db-type      (constantly :postgres)
+                  u/jdbc-clob->str #(cond-> %
+                                      (instance? FakeClob %) :s)]
+      (-> (#'load-from-h2/objects->colums+values
+           [(ordered-map/ordered-map
+             :created_at             #inst "2019-04-05T21:26:39.936000000-00:00"
+             :description            (FakeClob. "This is a description")
+             :parameter_mappings     []
+             :visualization_settings {})])
+          (update :cols vec)
+          (update :vals vec)))))