diff --git a/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj b/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
index 36abfdb311b4a61dc171a847f7ce2a4b0688aa31..61d7a710cc95245128fdcb55477b1bc62c6e37d1 100644
--- a/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
+++ b/modules/drivers/presto-jdbc/test/metabase/test/data/presto_jdbc.clj
@@ -163,7 +163,7 @@
(testing "Make sure logic to strip out NOT NULL and PRIMARY KEY stuff works as expected"
(let [db-def (tx/get-dataset-definition defs/test-data)
table-def (-> db-def :table-definitions second)]
- (is (= "CREATE TABLE \"test_data\".\"default\".\"categories\" (\"id\" INTEGER, \"name\" VARCHAR) ;"
+ (is (= "CREATE TABLE \"test_data\".\"default\".\"test_data_categories\" (\"id\" INTEGER, \"name\" VARCHAR) ;"
(sql.tx/create-table-sql :presto-jdbc db-def table-def))))))
(defmethod ddl.i/format-name :presto-jdbc
diff --git a/src/metabase/lib/join.cljc b/src/metabase/lib/join.cljc
index 9c6abe6edc0e2d6d097de0c2291ce2223c06d1e2..27555533288214c20c80fd62e093a59e589d6667 100644
--- a/src/metabase/lib/join.cljc
+++ b/src/metabase/lib/join.cljc
@@ -198,8 +198,10 @@
unique-name-fn :- fn?
col :- :map]
(assoc col
- :lib/source-column-alias (:name col)
- :lib/desired-column-alias (unique-name-fn (joined-field-desired-alias (:alias join) (:name col)))))
+ :lib/source-column-alias ((some-fn :lib/source-column-alias :name) col)
+ :lib/desired-column-alias (unique-name-fn (joined-field-desired-alias
+ (:alias join)
+ ((some-fn :lib/source-column-alias :name) col)))))
(defmethod lib.metadata.calculation/returned-columns-method :mbql/join
[query
@@ -287,11 +289,11 @@
"Create an MBQL join map from something that can conceptually be joined against. A `Table`? An MBQL or native query? A
Saved Question? You should be able to join anything, and this should return a sensible MBQL join map."
([joinable]
- (join-clause-method joinable))
+ (-> (join-clause-method joinable)
+ (u/assoc-default :fields :all)))
([joinable conditions]
(-> (join-clause joinable)
- (u/assoc-default :fields :all)
(with-join-conditions conditions))))
(mu/defn with-join-fields :- PartialJoin
diff --git a/src/metabase/lib/stage.cljc b/src/metabase/lib/stage.cljc
index 83fc4c573aabbf9ed73f4bcee372e45df0c90444..26c1266234928c0e50cf1223cec73c33a5efc834 100644
--- a/src/metabase/lib/stage.cljc
+++ b/src/metabase/lib/stage.cljc
@@ -85,8 +85,8 @@
(for [breakout (lib.breakout/breakouts-metadata query stage-number)]
(assoc breakout
:lib/source :source/breakouts
- :lib/source-column-alias (:name breakout)
- :lib/desired-column-alias (unique-name-fn (:name breakout))))))
+ :lib/source-column-alias ((some-fn :lib/source-column-alias :name) breakout)
+ :lib/desired-column-alias (unique-name-fn (lib.field/desired-alias query breakout))))))
(mu/defn ^:private aggregations-columns :- [:maybe lib.metadata.calculation/ColumnsWithUniqueAliases]
[query :- ::lib.schema/query
@@ -99,6 +99,8 @@
:lib/source-column-alias (:name ag)
:lib/desired-column-alias (unique-name-fn (:name ag))))))
+;;; TODO -- maybe the bulk of this logic should be moved into [[metabase.lib.field]], like we did for breakouts and
+;;; aggregations above.
(mu/defn ^:private fields-columns :- [:maybe lib.metadata.calculation/ColumnsWithUniqueAliases]
[query :- ::lib.schema/query
stage-number :- :int
diff --git a/test/metabase/actions/test_util.clj b/test/metabase/actions/test_util.clj
index 09cc5621c2dc0ea23b62a4e81ca35f8fdfc96ef5..261e955b425ff03551c12a5336cf4b13d824f51a 100644
--- a/test/metabase/actions/test_util.clj
+++ b/test/metabase/actions/test_util.clj
@@ -2,7 +2,10 @@
(:require
[clojure.java.jdbc :as jdbc]
[clojure.test :refer :all]
+ [metabase.driver :as driver]
+ [metabase.driver.ddl.interface :as ddl.i]
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
+ [metabase.driver.sql.query-processor :as sql.qp]
[metabase.http-client :as client]
[metabase.models :refer [Action Card Database]]
[metabase.models.action :as action]
@@ -14,6 +17,7 @@
[metabase.test.data.users :as test.users]
[metabase.test.initialize :as initialize]
[metabase.test.util :as tu]
+ [metabase.util.honey-sql-2 :as h2x]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
@@ -111,6 +115,23 @@
[& body]
`(do-with-dataset-definition (tx/dataset-definition ~(str (gensym))) (fn [] ~@body)))
+(defn- delete-categories-1-query []
+ (sql.qp/format-honeysql
+ 2
+ (sql.qp/quote-style driver/*driver*)
+ {:delete-from [(h2x/identifier :table (ddl.i/format-name driver/*driver* "categories"))]
+ :where [:=
+ (h2x/identifier :field (ddl.i/format-name driver/*driver* "id"))
+ [:inline 1]]}))
+
+(deftest ^:parallel delete-categories-1-query-test
+ (are [driver query] (= query
+ (binding [driver/*driver* driver]
+ (delete-categories-1-query)))
+ :h2 ["DELETE FROM \"CATEGORIES\" WHERE \"ID\" = 1"]
+ :postgres ["DELETE FROM \"categories\" WHERE \"id\" = 1"]
+ :mysql ["DELETE FROM `categories` WHERE `id` = 1"]))
+
(deftest with-actions-test-data-test
(datasets/test-drivers (qp.test/normal-drivers-with-feature :actions/custom)
(dotimes [i 2]
@@ -124,7 +145,7 @@
(testing "delete row"
(is (= [1]
(jdbc/execute! (sql-jdbc.conn/db->pooled-connection-spec (data/id))
- "DELETE FROM CATEGORIES WHERE ID = 1;"))))
+ (delete-categories-1-query)))))
(testing "after"
(is (= [[74]]
(row-count))))))))))
diff --git a/test/metabase/lib/join_test.cljc b/test/metabase/lib/join_test.cljc
index f7ea077d2b4e55ef7f5e036724677bbaf824967c..709b713bbce0e4871db07530062e993bb9ec8a9a 100644
--- a/test/metabase/lib/join_test.cljc
+++ b/test/metabase/lib/join_test.cljc
@@ -55,6 +55,16 @@
:args [(lib/ref (meta/field-metadata :venues :category-id))
(lib/ref (meta/field-metadata :categories :id))]}]))))))
+(deftest ^:parallel join-clause-test
+ (testing "Should have :fields :all by default (#32419)"
+ (is (=? {:lib/type :mbql/join
+ :stages [{:lib/type :mbql.stage/mbql
+ :lib/options {:lib/uuid string?}
+ :source-table (meta/id :orders)}]
+ :lib/options {:lib/uuid string?}
+ :fields :all}
+ (lib/join-clause (meta/table-metadata :orders))))))
+
(deftest ^:parallel join-saved-question-test
(is (=? {:lib/type :mbql/query
:database (meta/id)
diff --git a/test/metabase/lib/stage_test.cljc b/test/metabase/lib/stage_test.cljc
index 3eb2a0033734dd243c5f566831fe551a9daab3ab..cc9fe272ecb5c1aff92d6583066ec55b3a2ee8bc 100644
--- a/test/metabase/lib/stage_test.cljc
+++ b/test/metabase/lib/stage_test.cljc
@@ -3,6 +3,7 @@
[clojure.test :refer [deftest is testing]]
[malli.core :as mc]
[metabase.lib.core :as lib]
+ [metabase.lib.join :as lib.join]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.schema :as lib.schema]
[metabase.lib.stage :as lib.stage]
@@ -10,8 +11,6 @@
[metabase.lib.test-util :as lib.tu]
#?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))))
-(comment lib/keep-me)
-
#?(:cljs
(comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
@@ -249,3 +248,100 @@
:lib/source :source/expressions}]
(filter #(= (:name %) expr-name)
(lib.metadata.calculation/visible-columns query)))))))
+
+(defn- metadata-for-breakouts-from-joins-test-query
+ "A query against `ORDERS` with joins against `PRODUCTS` and `PEOPLE`, and breakouts on columns from both of those
+ joins."
+ []
+ (-> (lib/query meta/metadata-provider (meta/table-metadata :orders))
+ (lib/join (-> (lib/join-clause (meta/table-metadata :products))
+ (lib/with-join-alias "P1")
+ (lib/with-join-conditions [(lib/= (meta/field-metadata :orders :product-id)
+ (-> (meta/field-metadata :products :id)
+ (lib/with-join-alias "P1")))])))
+ (lib/join (-> (lib/join-clause (meta/table-metadata :people))
+ (lib/with-join-alias "People")
+ (lib/with-join-conditions [(lib/= (meta/field-metadata :orders :user-id)
+ (-> (meta/field-metadata :people :id)
+ (lib/with-join-alias "People")))])))
+ (lib/breakout (-> (meta/field-metadata :products :category)
+ (lib/with-join-alias "P1")))
+ (lib/breakout (-> (meta/field-metadata :people :source)
+ (lib/with-join-alias "People")))
+ (lib/aggregate (lib/count))))
+
+(deftest ^:parallel metadata-for-breakouts-from-joins-test
+ (testing "metadata for breakouts of joined columns should be calculated correctly (#29907)"
+ (let [query (metadata-for-breakouts-from-joins-test-query)]
+ (is (= [{:name "CATEGORY"
+ :lib/source-column-alias "CATEGORY"
+ ::lib.join/join-alias "P1"
+ :lib/desired-column-alias "P1__CATEGORY"}
+ {:name "SOURCE"
+ :lib/source-column-alias "SOURCE"
+ ::lib.join/join-alias "People"
+ :lib/desired-column-alias "People__SOURCE"}
+ {:name "count"
+ :lib/source-column-alias "count"
+ :lib/desired-column-alias "count"}]
+ (map #(select-keys % [:name :lib/source-column-alias ::lib.join/join-alias :lib/desired-column-alias])
+ (lib.metadata.calculation/returned-columns query)))))))
+
+(defn- metadata-for-breakouts-from-joins-test-query-2
+ "A query against `REVIEWS` joining `PRODUCTS`."
+ []
+ (-> (lib/query meta/metadata-provider (meta/table-metadata :reviews))
+ (lib/join (-> (lib/join-clause (meta/table-metadata :products))
+ (lib/with-join-alias "P2")
+ (lib/with-join-conditions [(lib/= (meta/field-metadata :reviews :product-id)
+ (-> (meta/field-metadata :products :id)
+ (lib/with-join-alias "P2")))])))
+ (lib/breakout (-> (meta/field-metadata :products :category)
+ (lib/with-join-alias "P2")))
+ (lib/aggregate (lib/avg (meta/field-metadata :reviews :rating)))
+ lib/append-stage))
+
+(deftest ^:parallel metadata-for-breakouts-from-joins-test-2
+ (testing "metadata for breakouts of joined columns should be calculated correctly (#29907)"
+ (let [query (metadata-for-breakouts-from-joins-test-query-2)]
+ (is (= [{:name "CATEGORY", :lib/source-column-alias "P2__CATEGORY", :lib/desired-column-alias "P2__CATEGORY"}
+ {:name "avg", :lib/source-column-alias "avg", :lib/desired-column-alias "avg"}]
+ (map #(select-keys % [:name :lib/source-column-alias ::lib.join/join-alias :lib/desired-column-alias])
+ (lib.metadata.calculation/returned-columns query)))))))
+
+(defn- metadata-for-breakouts-from-joins-from-previous-stage-test-query
+ "[[metadata-for-breakouts-from-joins-test-query]] but with an additional stage and a join
+ against [[metadata-for-breakouts-from-joins-test-query-2]]. This means there are two joins against `PRODUCTS`, one
+ from the first stage and one from the nested query in the join in the second stage."
+ []
+ (-> (metadata-for-breakouts-from-joins-test-query)
+ lib/append-stage
+ (lib/join (-> (lib/join-clause (metadata-for-breakouts-from-joins-test-query-2))
+ (lib/with-join-alias "Q2")
+ (lib/with-join-conditions [(lib/= (-> (meta/field-metadata :products :category)
+ (lib/with-join-alias "P1"))
+ (-> (meta/field-metadata :products :category)
+ (lib/with-join-alias "Q2")))])))))
+
+(deftest ^:parallel metadata-for-breakouts-from-joins-from-previous-stage-test
+ (testing "metadata for breakouts of columns from join in previous stage should be calculated correctly (#29907)"
+ (let [query (metadata-for-breakouts-from-joins-from-previous-stage-test-query)]
+ (is (= [{:name "CATEGORY"
+ :lib/source-column-alias "P1__CATEGORY"
+ :lib/desired-column-alias "P1__CATEGORY"}
+ {:name "SOURCE"
+ :lib/source-column-alias "People__SOURCE"
+ :lib/desired-column-alias "People__SOURCE"}
+ {:name "count"
+ :lib/source-column-alias "count"
+ :lib/desired-column-alias "count"}
+ {:name "CATEGORY"
+ :lib/source-column-alias "P2__CATEGORY"
+ ::lib.join/join-alias "Q2"
+ :lib/desired-column-alias "Q2__P2__CATEGORY"}
+ {:name "avg"
+ :lib/source-column-alias "avg"
+ ::lib.join/join-alias "Q2"
+ :lib/desired-column-alias "Q2__avg"}]
+ (map #(select-keys % [:name :lib/source-column-alias ::lib.join/join-alias :lib/desired-column-alias])
+ (lib.metadata.calculation/returned-columns query)))))))
diff --git a/test/metabase/query_processor/middleware/constraints_test.clj b/test/metabase/query_processor/middleware/constraints_test.clj
index 089c0b0bd90831ca2262bfd3b538770a1bfb7303..0326066fa5e02690607f53d779e2ebab1da2d051 100644
--- a/test/metabase/query_processor/middleware/constraints_test.clj
+++ b/test/metabase/query_processor/middleware/constraints_test.clj
@@ -7,12 +7,12 @@
(defn- add-default-userland-constraints [query]
(:pre (mt/test-qp-middleware qp.constraints/add-default-userland-constraints query)))
-(deftest no-op-without-middleware-options-test
+(deftest ^:parallel no-op-without-middleware-options-test
(testing "don't do anything to queries without [:middleware :add-default-userland-constraints?] set"
(is (= {}
(add-default-userland-constraints {})))))
-(deftest add-constraints-test
+(deftest ^:parallel add-constraints-test
(testing "if it is *truthy* add the constraints"
(is (= {:middleware {:add-default-userland-constraints? true},
:constraints {:max-results @#'qp.constraints/max-results
@@ -20,13 +20,13 @@
(add-default-userland-constraints
{:middleware {:add-default-userland-constraints? true}})))))
-(deftest no-op-if-option-is-false-test
+(deftest ^:parallel no-op-if-option-is-false-test
(testing "don't do anything if it's not truthy"
(is (= {:middleware {:add-default-userland-constraints? false}}
(add-default-userland-constraints
{:middleware {:add-default-userland-constraints? false}})))))
-(deftest dont-overwrite-existing-constraints-test
+(deftest ^:parallel dont-overwrite-existing-constraints-test
(testing "if it already has constraints, don't overwrite those!"
(is (= {:middleware {:add-default-userland-constraints? true}
:constraints {:max-results @#'qp.constraints/max-results
@@ -35,7 +35,7 @@
{:constraints {:max-results-bare-rows 1}
:middleware {:add-default-userland-constraints? true}})))))
-(deftest max-results-bare-rows-should-be-less-than-max-results-test
+(deftest ^:parallel max-results-bare-rows-should-be-less-than-max-results-test
(testing "if you specify just `:max-results` it should make sure `:max-results-bare-rows` is <= `:max-results`"
(is (= {:middleware {:add-default-userland-constraints? true}
:constraints {:max-results 5
diff --git a/test/metabase/query_processor/middleware/limit_test.clj b/test/metabase/query_processor/middleware/limit_test.clj
index 6ff1ecb545e26eaf3a5d8ca8744464e82f336d1c..631e9c0d67538f0492e9731b541f1f7defec8b5c 100644
--- a/test/metabase/query_processor/middleware/limit_test.clj
+++ b/test/metabase/query_processor/middleware/limit_test.clj
@@ -20,7 +20,7 @@
(is (= test-max-results
(-> (limit {:type :native}) mt/rows count)))))
-(deftest disable-max-results-test
+(deftest ^:parallel disable-max-results-test
(testing "Apply `absolute-max-results` limit in the default case"
(let [query {:type :query
:query {}}]
diff --git a/test/metabase/test_runner.clj b/test/metabase/test_runner.clj
index 698c44e6fc5d622b677e7c865e48f6bdba22bcf5..9b2e1e31223d0a82202bfe31d5a5b6cd0ab528f0 100644
--- a/test/metabase/test_runner.clj
+++ b/test/metabase/test_runner.clj
@@ -84,7 +84,7 @@
"test_resources"])
(defn- default-options []
- {:namespace-pattern #"^metabase.*test$"
+ {:namespace-pattern #"^metabase.*"
:exclude-directories excluded-directories
:test-warn-time 1500})