Fix laziness in SQL QP query compilation (#14858)

* Revert test-data-loading tweaks for now

* Test fix 

* Test fix

* Test fixes

* Unskip repro for #14859

* Test fix 



Co-authored-by: Nemanja <31325167+nemanjaglumac@users.noreply.github.com>

enterprise/frontend/test/metabase-enterprise/sandboxes/sandboxes.cy.spec.js

@@ -275,7 +275,8 @@ describeWithToken("formatting > sandboxes", () => {
       cy.findByText("11"); // Sum of orders for user with ID #1
     });
 
-    it.skip("SB question with `case` CC should substitute the `else` argument's table (metabase-enterprise#548)", () => {
+    // Note: This issue was ported from EE repo - it was previously known as (metabase-enterprise#548)
+    it("SB question with `case` CC should substitute the `else` argument's table (metabase#14859)", () => {
       const QUESTION_NAME = "EE_548";
       const CC_NAME = "CC_548"; // Custom column
 
@@ -337,7 +338,7 @@ describeWithToken("formatting > sandboxes", () => {
           expect(xhr.response.body.error).not.to.exist;
         });
 
-        cy.findByText(CC_NAME);
+        cy.contains(CC_NAME);
       });
     });
 

frontend/test/metabase/scenarios/question/custom_column.cy.spec.js

@@ -445,7 +445,7 @@ describe("scenarios > question > custom columns", () => {
     cy.contains("37.65");
   });
 
-  it.skip("should handle using `case()` when referencing the same column names (metabase#14854)", () => {
+  it("should handle using `case()` when referencing the same column names (metabase#14854)", () => {
     const CC_NAME = "CE with case";
 
     cy.server();

src/metabase/driver/sql/query_processor.clj

@@ -448,7 +448,7 @@
 
 (defmethod ->honeysql [:sql :coalesce]
   [driver [_ & args]]
-  (apply hsql/call :coalesce (map (partial ->honeysql driver) args)))
+  (apply hsql/call :coalesce (mapv (partial ->honeysql driver) args)))
 
 (defmethod ->honeysql [:sql :replace]
   [driver [_ arg pattern replacement]]
@@ -456,7 +456,7 @@
 
 (defmethod ->honeysql [:sql :concat]
   [driver [_ & args]]
-  (apply hsql/call :concat (map (partial ->honeysql driver) args)))
+  (apply hsql/call :concat (mapv (partial ->honeysql driver) args)))
 
 (defmethod ->honeysql [:sql :substring]
   [driver [_ arg start length]]
@@ -474,7 +474,7 @@
                (when (:default options)
                  [[:else (:default options)]]))
        (apply concat)
-       (map (partial ->honeysql driver))
+       (mapv (partial ->honeysql driver))
        (apply hsql/call :case)))
 
 ;; actual handling of the name is done in the top-level clause handler for aggregations
@@ -592,7 +592,7 @@
                                           (remove (set fields-fields))
                                           (mapv (fn [field-clause]
                                                   (as driver field-clause unique-name-fn)))))
-      (apply h/group new-hsql (map (partial ->honeysql driver) breakout-fields)))))
+      (apply h/group new-hsql (mapv (partial ->honeysql driver) breakout-fields)))))
 
 (defmethod apply-top-level-clause [:sql :fields]
   [driver _ honeysql-form {fields :fields}]
@@ -674,11 +674,11 @@
 
 (defmethod ->honeysql [:sql :and]
   [driver [_ & subclauses]]
-  (apply vector :and (map (partial ->honeysql driver) subclauses)))
+  (apply vector :and (mapv (partial ->honeysql driver) subclauses)))
 
 (defmethod ->honeysql [:sql :or]
   [driver [_ & subclauses]]
-  (apply vector :or (map (partial ->honeysql driver) subclauses)))
+  (apply vector :or (mapv (partial ->honeysql driver) subclauses)))
 
 (def ^:private clause-needs-null-behaviour-correction?
   (comp #{:contains :starts-with :ends-with} first))
@@ -764,7 +764,7 @@
 
 (defmethod apply-top-level-clause [:sql :order-by]
   [driver _ honeysql-form {subclauses :order-by}]
-  (reduce h/merge-order-by honeysql-form (map (partial ->honeysql driver) subclauses)))
+  (reduce h/merge-order-by honeysql-form (mapv (partial ->honeysql driver) subclauses)))
 
 ;;; -------------------------------------------------- limit & page --------------------------------------------------
 

test/metabase/query_processor_test/expressions_test.clj

@@ -330,23 +330,21 @@
                   (mt/formatted-rows [int])
                   ffirst))))))
 
-;; Test for https://github.com/metabase/metabase/issues/12305
 (deftest expression-with-slashes
   (mt/test-drivers (mt/normal-drivers-with-feature :expressions)
-    (is (= [[1 "Red Medicine"           4 10.0646 -165.374 3 4.0]
-            [2 "Stout Burgers & Beers" 11 34.0996 -118.329 2 3.0]
-            [3 "The Apple Pan"         11 34.0406 -118.428 2 3.0]]
-           (mt/formatted-rows [int str int 4.0 4.0 int float]
-             (mt/run-mbql-query venues
-               {:expressions {:TEST/my-cool-new-field [:+ $price 1]}
-                :limit       3
-                :order-by    [[:asc $id]]})))
-        "Make sure an expression with a / in its name works")))
-
-;; https://github.com/metabase/metabase/issues/12762
+    (testing "Make sure an expression with a / in its name works (#12305)"
+      (is (= [[1 "Red Medicine"           4 10.0646 -165.374 3 4.0]
+              [2 "Stout Burgers & Beers" 11 34.0996 -118.329 2 3.0]
+              [3 "The Apple Pan"         11 34.0406 -118.428 2 3.0]]
+             (mt/formatted-rows [int str int 4.0 4.0 int float]
+               (mt/run-mbql-query venues
+                 {:expressions {:TEST/my-cool-new-field [:+ $price 1]}
+                  :limit       3
+                  :order-by    [[:asc $id]]})))))))
+
 (deftest expression-using-aggregation-test
   (mt/test-drivers (mt/normal-drivers-with-feature :expressions)
-    (testing "Can we use aggregations from previous steps in expressions"
+    (testing "Can we use aggregations from previous steps in expressions (#12762)"
       (is (= [["20th Century Cafe" 2 2 0]
               [ "25°" 2 2 0 ]
               ["33 Taps" 2 2 0]]
@@ -359,3 +357,21 @@
                   :expressions  {:price-range [:- [:field-literal "max" :type/Number]
                                                [:field-literal "min" :type/Number]]}
                   :limit        3})))))))
+
+(deftest fk-field-and-duplicate-names-test
+  ;; Redshift hangs on sample-dataset -- See #14784
+  (mt/test-drivers (disj (mt/normal-drivers-with-feature :expressions :foreign-keys) :redshift)
+    (testing "Expressions with `fk->` fields and duplicate names should work correctly (#14854)"
+      (mt/dataset sample-dataset
+        (let [results (mt/run-mbql-query orders
+                        {:expressions {"CE" [:case
+                                             [[[:> $discount 0] $created_at]]
+                                             {:default $product_id->products.created_at}]}
+                         :order-by    [[:asc $id]]
+                         :limit       2})]
+          (is (= ["ID" "User ID" "Product ID" "Subtotal" "Tax" "Total" "Discount" "Created At" "Quantity" "CE"]
+                 (map :display_name (mt/cols results))))
+          (is (= [[1 1  14  37.7  2.1  39.7 nil "2019-02-11T21:40:27.892Z" 2 "2017-12-31T14:41:56.87Z"]
+                  [2 1 123 110.9  6.1 117.0 nil "2018-05-15T08:04:04.58Z"  3 "2017-11-16T13:53:14.232Z"]]
+                 (mt/formatted-rows [int int int 1.0 1.0 1.0 identity str int str]
+                   results))))))))

test/metabase/query_processor_test/implicit_joins_test.clj

@@ -139,7 +139,7 @@
                     :filter      [:= $receiver_id->users.name "Rasta Toucan"]}))))))))
 
 (deftest implicit-joins-with-expressions-test
-  ;; Redshift excluded for now since the sample dataset seems to hang for Redshift.
+  ;; Redshift excluded for now since the sample dataset seems to hang for Redshift -- see #14784
   (mt/test-drivers (disj (mt/normal-drivers-with-feature :foreign-keys :expressions) :redshift)
     (testing "Should be able to run query with multiple implicit joins and breakouts"
       (mt/dataset sample-dataset