From dddf972b4c3c2ffd3c879c8c6d18cc38a077ed09 Mon Sep 17 00:00:00 2001
From: Simon Belak <simon@metabase.com>
Date: Fri, 6 Nov 2020 00:54:08 +0100
Subject: [PATCH] MBQL: correctly alias duplicate col names in breakouts
(#10511) (#13648)
* Rebase
* Typo
* Add repro for #10511
Co-authored-by: Nemanja <31325167+nemanjaglumac@users.noreply.github.com>
---
.../scenarios/question/nested.cy.spec.js | 45 +++++++++++++++++++
src/metabase/driver/sql/query_processor.clj | 12 ++---
.../nested_queries_test.clj | 18 ++++++++
3 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/frontend/test/metabase/scenarios/question/nested.cy.spec.js b/frontend/test/metabase/scenarios/question/nested.cy.spec.js
index 8d05e892de8..77493c827eb 100644
--- a/frontend/test/metabase/scenarios/question/nested.cy.spec.js
+++ b/frontend/test/metabase/scenarios/question/nested.cy.spec.js
@@ -144,3 +144,48 @@ describe("scenarios > question > nested (metabase#12568)", () => {
cy.get(".bar").should("have.length.of.at.least", 10);
});
});
+
+describe("scenarios > question > nested", () => {
+ before(() => {
+ restore();
+ signInAsAdmin();
+ });
+
+ it("should handle duplicate column names in nested queries (metabase#10511)", () => {
+ withSampleDataset(({ ORDERS, PRODUCTS }) => {
+ cy.request("POST", "/api/card", {
+ name: "10511",
+ dataset_query: {
+ database: 1,
+ query: {
+ filter: [">", ["field-literal", "count", "type/Integer"], 5],
+ "source-query": {
+ "source-table": 2,
+ aggregation: [["count"]],
+ breakout: [
+ ["datetime-field", ["field-id", ORDERS.CREATED_AT], "month"],
+ [
+ "datetime-field",
+ [
+ "fk->",
+ ["field-id", ORDERS.PRODUCT_ID],
+ ["field-id", PRODUCTS.CREATED_AT],
+ ],
+ "month",
+ ],
+ ],
+ },
+ },
+ type: "query",
+ },
+ display: "table",
+ visualization_settings: {},
+ }).then(({ body: { id: questionId } }) => {
+ cy.visit(`/question/${questionId}`);
+ cy.findByText("10511");
+ cy.findAllByText("June, 2016");
+ cy.findAllByText("13");
+ });
+ });
+ });
+});
diff --git a/src/metabase/driver/sql/query_processor.clj b/src/metabase/driver/sql/query_processor.clj
index a814fd96301..50d40681380 100644
--- a/src/metabase/driver/sql/query_processor.clj
+++ b/src/metabase/driver/sql/query_processor.clj
@@ -571,11 +571,13 @@
(defmethod apply-top-level-clause [:sql :breakout]
[driver _ honeysql-form {breakout-fields :breakout, fields-fields :fields :as query}]
- (as-> honeysql-form new-hsql
- (apply h/merge-select new-hsql (vec (for [field-clause breakout-fields
- :when (not (contains? (set fields-fields) field-clause))]
- (as driver field-clause))))
- (apply h/group new-hsql (map (partial ->honeysql driver) breakout-fields))))
+ (let [unique-name-fn (mbql.u/unique-name-generator)]
+ (as-> honeysql-form new-hsql
+ (apply h/merge-select new-hsql (->> breakout-fields
+ (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)))))
(defmethod apply-top-level-clause [:sql :fields]
[driver _ honeysql-form {fields :fields}]
diff --git a/test/metabase/query_processor_test/nested_queries_test.clj b/test/metabase/query_processor_test/nested_queries_test.clj
index 87d01574198..b237286fe1f 100644
--- a/test/metabase/query_processor_test/nested_queries_test.clj
+++ b/test/metabase/query_processor_test/nested_queries_test.clj
@@ -699,3 +699,21 @@
:order-by [[:asc !year.date]]
:limit 1}
:fields [!year.*date]}))))))
+
+;; https://github.com/metabase/metabase/issues/10511
+(deftest correctly-alias-duplicate-names-in-breakout-test
+ (mt/test-drivers (mt/normal-drivers-with-feature :nested-queries :expressions)
+ (testing "Do we correctly alias name clashes in breakout"
+ (is (= [[ "20th Century Cafe" "Café" 1 ]
+ [ "25°" "Burger" 1 ]
+ [ "33 Taps" "Bar" 1 ]]
+ (mt/formatted-rows [str str int]
+ (mt/run-mbql-query venues
+ {:source-query {:source-table $$venues
+ :aggregation [[:count]]
+ :breakout [$name [:joined-field "c" $categories.name]]
+ :joins [{:source-table $$categories
+ :alias "c"
+ :condition [:= $category_id [:joined-field "c" $categories.id]]}]}
+ :filter [:> [:field-literal "count" :type/Number] 0]
+ :limit 3})))))))
--
GitLab