Fixing tests under Redshift driver (metabase#12512)

Newly added metadata in the query remark was blowing up for parameter
queries other than dimension / field-id queries.

[ci redshift]

Author: Robert Roland

modules/drivers/redshift/src/metabase/driver/redshift.clj

@@ -14,6 +14,7 @@
              [execute :as sql-jdbc.execute]]
             [metabase.driver.sql-jdbc.execute.legacy-impl :as legacy]
             [metabase.driver.sql.query-processor :as sql.qp]
+            [metabase.mbql.util :as mbql.u]
             [metabase.query-processor
              [store :as qp.store]
              [util :as qputil]]
@@ -136,7 +137,7 @@
 (defmethod sql.qp/->honeysql [:redshift :replace]
   [driver [_ arg pattern replacement]]
   (hsql/call :replace (sql.qp/->honeysql driver arg) (splice-raw-string-value driver pattern)
-              (splice-raw-string-value driver replacement)))
+             (splice-raw-string-value driver replacement)))
 
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                         metabase.driver.sql-jdbc impls                                         |
@@ -165,7 +166,21 @@
 (defn query->field-values
   "Convert a MBQL query to a map of field->value"
   [query]
-  (into {} (map (fn [param] [(:name (qp.store/field (get-in param [:target 1 1]))) (:value param)]) (:user-parameters query))))
+  (into {}
+        (filter identity
+                (map
+                 (fn [param]
+                   (if (contains? param :name)
+                     [(:name param) (:value param)]
+
+                     (when-let [field-id (mbql.u/match-one
+                                          param
+                                          [:dimension field]
+                                          (let [field-id-or-name (mbql.u/field-clause->id-or-literal field)]
+                                            (when (integer? field-id-or-name)
+                                              field-id-or-name)))]
+                       [(:name (qp.store/field field-id)) (:value param)])))
+                 (:user-parameters query)))))
 
 (defmethod qputil/query->remark :redshift
   [_ {{:keys [executed-by query-hash card-id], :as info} :info, query-type :type :as query}]

modules/drivers/redshift/test/metabase/driver/redshift_test.clj

@@ -32,30 +32,31 @@
       @native-query)))
 
 (deftest remark-test
-  (let [expected (str/replace
-                  (str
-                   "-- /* partner: \"metabase\", {\"dashboard_id\":null,\"chart_id\":1234,\"optional_user_id\":1000,"
-                   "\"optional_account_id\":\"" (pubset/site-uuid) "\","
-                   "\"filter_values\":{\"id\":[\"1\",\"2\",\"3\"]}} */"
-                   " Metabase:: userID: 1000 queryType: MBQL queryHash: cb83d4f6eedc250edb0f2c16f8d9a21e5d42f322ccece1494c8ef3d634581fe2\n"
-                   "SELECT \"%schema%\".\"test_data_users\".\"id\" AS \"id\","
-                   " \"%schema%\".\"test_data_users\".\"name\" AS \"name\","
-                   " \"%schema%\".\"test_data_users\".\"last_login\" AS \"last_login\""
-                   " FROM \"%schema%\".\"test_data_users\""
-                   " WHERE (\"%schema%\".\"test_data_users\".\"id\" = 1 OR \"%schema%\".\"test_data_users\".\"id\" = 2"
-                   " OR \"%schema%\".\"test_data_users\".\"id\" = 3)"
-                   " LIMIT 2000")
-                  "%schema%" rstest/session-schema-name)]
-   (mt/test-driver
-    :redshift
-    (is (= expected
-           (query->native
-            (assoc
-             (mt/mbql-query users {:limit 2000})
-             :parameters [{:type "id", :target ["dimension" ["field-id" (mt/id :users :id)]], :value ["1" "2" "3"]}]
-             :info {:executed-by 1000
-                    :card-id 1234
-                    :context :ad-hoc
-                    :nested? false
-                    :query-hash (byte-array [-53, -125, -44, -10, -18, -36, 37, 14, -37, 15, 44, 22, -8, -39, -94, 30, 93, 66, -13, 34, -52, -20, -31, 73, 76, -114, -13, -42, 52, 88, 31, -30])})))
-        "if I run a Redshift query, does it get a remark added to it?"))))
+  (testing "single field user-specified value"
+   (let [expected (str/replace
+                   (str
+                    "-- /* partner: \"metabase\", {\"dashboard_id\":null,\"chart_id\":1234,\"optional_user_id\":1000,"
+                    "\"optional_account_id\":\"" (pubset/site-uuid) "\","
+                    "\"filter_values\":{\"id\":[\"1\",\"2\",\"3\"]}} */"
+                    " Metabase:: userID: 1000 queryType: MBQL queryHash: cb83d4f6eedc250edb0f2c16f8d9a21e5d42f322ccece1494c8ef3d634581fe2\n"
+                    "SELECT \"%schema%\".\"test_data_users\".\"id\" AS \"id\","
+                    " \"%schema%\".\"test_data_users\".\"name\" AS \"name\","
+                    " \"%schema%\".\"test_data_users\".\"last_login\" AS \"last_login\""
+                    " FROM \"%schema%\".\"test_data_users\""
+                    " WHERE (\"%schema%\".\"test_data_users\".\"id\" = 1 OR \"%schema%\".\"test_data_users\".\"id\" = 2"
+                    " OR \"%schema%\".\"test_data_users\".\"id\" = 3)"
+                    " LIMIT 2000")
+                   "%schema%" rstest/session-schema-name)]
+     (mt/test-driver
+      :redshift
+      (is (= expected
+             (query->native
+              (assoc
+               (mt/mbql-query users {:limit 2000})
+               :parameters [{:type "id", :target ["dimension" ["field-id" (mt/id :users :id)]], :value ["1" "2" "3"]}]
+               :info {:executed-by 1000
+                      :card-id 1234
+                      :context :ad-hoc
+                      :nested? false
+                      :query-hash (byte-array [-53, -125, -44, -10, -18, -36, 37, 14, -37, 15, 44, 22, -8, -39, -94, 30, 93, 66, -13, 34, -52, -20, -31, 73, 76, -114, -13, -42, 52, 88, 31, -30])})))
+          "if I run a Redshift query, does it get a remark added to it?")))))