Move prepare-identifer to test protocol

src/metabase/driver/generic_sql.clj

@@ -72,12 +72,6 @@
 
      Return `nil` to prevent FIELD from being aliased.")
 
-  ;; TODO This is only used by unit tests, I think; confirm this and move to `metabase.test.data.generic-sql`
-  (prepare-identifier [this, ^String identifier]
-    "*OPTIONAL*. Prepare an identifier, such as a Table or Field name, when it is used in a SQL query.
-     This is used by drivers like H2 to transform names to upper-case.
-     The default implementation is `identity`.")
-
   (prepare-value [this, ^Value value]
     "*OPTIONAL*. Prepare a value (e.g. a `String` or `Integer`) that will be used in a korma form. By default, this returns VALUE's `:value` as-is, which
      is eventually passed as a parameter in a prepared statement. Drivers such as BigQuery that don't support prepared statements can skip this
@@ -406,7 +400,6 @@
    :excluded-schemas        (constantly nil)
    :field->alias            (u/drop-first-arg name)
    :field-percent-urls      fast-field-percent-urls
-   :prepare-identifier      (u/drop-first-arg identity)
    :prepare-value           (u/drop-first-arg :value)
    :quote-style             (constantly :ansi)
    :set-timezone-sql        (constantly nil)

src/metabase/driver/h2.clj

@@ -217,7 +217,6 @@
           :column->base-type         column->base-type
           :connection-details->spec  connection-details->spec
           :date                      date
-          :prepare-identifier        (u/drop-first-arg s/upper-case)
           :string-length-fn          (u/drop-first-arg string-length-fn)
           :unix-timestamp->timestamp unix-timestamp->timestamp}))
 

test/metabase/driver/generic_sql_test.clj

@@ -17,12 +17,12 @@
 (def ^:private 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))))
+  (delay (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
@@ -82,7 +82,7 @@
 (resolve-private-fns metabase.driver.generic-sql field-avg-length field-values-lazy-seq table-rows-seq)
 
 ;;; FIELD-AVG-LENGTH
-(datasets/expect-with-engines generic-sql-engines
+(datasets/expect-with-engines @generic-sql-engines
   ;; Not sure why some databases give different values for this but they're close enough that I'll allow them
   (if (contains? #{:redshift :sqlserver} datasets/*engine*)
     15
@@ -90,7 +90,7 @@
   (field-avg-length datasets/*driver* (db/select-one 'Field :id (id :venues :name))))
 
 ;;; FIELD-VALUES-LAZY-SEQ
-(datasets/expect-with-engines generic-sql-engines
+(datasets/expect-with-engines @generic-sql-engines
   ["Red Medicine"
    "Stout Burgers & Beers"
    "The Apple Pan"
@@ -100,7 +100,7 @@
 
 
 ;;; TABLE-ROWS-SEQ
-(datasets/expect-with-engines generic-sql-engines
+(datasets/expect-with-engines @generic-sql-engines
   [{:name "Red Medicine",                 :price 3, :category_id  4, :id 1}
    {:name "Stout Burgers & Beers",        :price 2, :category_id 11, :id 2}
    {:name "The Apple Pan",                :price 2, :category_id 11, :id 3}
@@ -112,7 +112,7 @@
     (dissoc row :latitude :longitude))) ; different DBs use different precisions for these
 
 ;;; FIELD-PERCENT-URLS
-(datasets/expect-with-engines generic-sql-engines
+(datasets/expect-with-engines @generic-sql-engines
   0.5
   (dataset half-valid-urls
     (field-percent-urls datasets/*driver* (db/select-one 'Field :id (id :urls :url)))))

test/metabase/test/data/generic_sql.clj

@@ -54,6 +54,11 @@
   (korma-entity [this, ^DatabaseDefinition dbdef, ^TableDefinition tabledef]
     "*Optional* Return a korma-entity for TABLEDEF.")
 
+  (prepare-identifier [this, ^String identifier]
+    "*OPTIONAL*. Prepare an identifier, such as a Table or Field name, when it is used in a SQL query.
+     This is used by drivers like H2 to transform names to upper-case.
+     The default implementation is `identity`.")
+
   (pk-field-name ^String [this]
     "*Optional* Name of a PK field. Defaults to `\"id\"`.")
 
@@ -208,7 +213,7 @@
 (defn- do-insert!
   "Insert ROWS-OR-ROWS into TABLE-NAME for the DRIVER database defined by SPEC."
   [driver spec table-name row-or-rows]
-  (let [prepare-key (comp keyword (partial sql/prepare-identifier driver) name)
+  (let [prepare-key (comp keyword (partial prepare-identifier driver) name)
         rows        (if (sequential? row-or-rows)
                       row-or-rows
                       [row-or-rows])
@@ -280,6 +285,7 @@
    :korma-entity              default-korma-entity
    :load-data!                load-data-chunked!
    :pk-field-name             (constantly "id")
+   :prepare-identifier        (u/drop-first-arg identity)
    :qualified-name-components default-qualified-name-components
    :qualify+quote-name        default-qualify+quote-name
    :quote-name                default-quote-name})

test/metabase/test/data/h2.clj

 (ns metabase.test.data.h2
   "Code for creating / destroying an H2 database from a `DatabaseDefinition`."
+  ;; TODO - rework this namespace to use `u/drop-first-arg` where appropriate
   (:require [clojure.core.reducers :as r]
             [clojure.string :as s]
             (korma [core :as k]
@@ -77,12 +78,12 @@
                                          ;; we always want to use 'server' context when execute-sql! is called
                                          ;; (never try connect as GUEST, since we're not giving them priviledges to create tables / etc)
                                          (execute-sql! this :server dbdef sql))
-            :field-base-type->sql-type (fn [_ base-type]
-                                         (field-base-type->sql-type base-type))
+            :field-base-type->sql-type (u/drop-first-arg field-base-type->sql-type)
             :korma-entity              korma-entity
             :load-data!                generic/load-data-all-at-once!
             :pk-field-name             (constantly "ID")
             :pk-sql-type               (constantly "BIGINT AUTO_INCREMENT")
+            :prepare-identifier        (u/drop-first-arg s/upper-case)
             :quote-name                quote-name}))
 
   i/IDatasetLoader
@@ -90,7 +91,6 @@
          {:database->connection-details       database->connection-details
           :default-schema                     (constantly "PUBLIC")
           :engine                             (constantly :h2)
-          :format-name                        (fn [_ table-or-field-name]
-                                                (s/upper-case table-or-field-name))
+          :format-name                        (u/drop-first-arg s/upper-case)
           :has-questionable-timezone-support? (constantly true)
           :id-field-type                      (constantly :BigIntegerField)}))