Skip to content
Snippets Groups Projects
Unverified Commit dad00648 authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

BigQuery should qualify columns from source query with `source` in the `SELECT` clause (#20434) 

* Fix BigQuery not qualifying source columns

* Don't fix on 42 for the deprecated BigQuery driver

* You know what I'll fix it after all. Why not.

* Oops I updated the wrong part of the test
parent 16dc860e
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj
+ 2
− 6
View file @ dad00648
......@@ -511,12 +511,8 @@
;; if the Field is from a join or source table, record this fact so that we know never to qualify it with the
;; project ID no matter what
(binding [*field-is-from-join-or-source-query?* (not (integer? source-table))]
;; if this Field is from a source table DO NOT qualify it at all.
(let [field-clause (cond-> field-clause
(= source-table ::add/source)
(mbql.u/update-field-options assoc ::add/source-table ::add/none))]
(-> (parent-method driver field-clause)
(with-temporal-type (temporal-type field-clause)))))))
(-> (parent-method driver field-clause)
(with-temporal-type (temporal-type field-clause))))))
(defmethod sql.qp/->honeysql [:bigquery-cloud-sdk :relative-datetime]
[driver clause]
......
modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/query_processor_test.clj
+ 17
− 14
View file @ dad00648
......@@ -787,8 +787,8 @@
:breakout [!month.date]}
:limit 2})]
(mt/with-native-query-testing-context query
(is (sql= {:select '[count AS count
count (*) AS count_2]
(is (sql= {:select '[source.count AS count
count (*) AS count_2]
:from [(let [prefix (project-id-prefix-if-set)]
{:select ['date_trunc (list (symbol (str prefix 'v3_test_data.checkins.date)) 'month) 'AS 'date
'count '(*) 'AS 'count]
......@@ -819,17 +819,20 @@
{:name "CE", :display-name "CE"}]]
:limit 10}))))))))
(deftest no-qualify-breakout-field-name-with-subquery-test
(mt/test-driver :bigquery-cloud-sdk
(testing "Breakout field name is not qualified if it is from source query (#18742)"
(is (sql= '{:select [source AS source
count (*) AS count]
:from [(select 1 as val "2" as source)
source]
:group-by [source]
:order-by [source ASC]}
(mt/mbql-query checkins
{:aggregation [[:count]]
:breakout [[:field "source" {:base-type :type/Text}]],
:source-query {:native "select 1 as `val`, '2' as `source`"}}))))))
(testing "Make sure columns name `source` in source query work correctly (#18742)"
(let [query (mt/mbql-query checkins
{:aggregation [[:count]]
:breakout [[:field "source" {:base-type :type/Text}]],
:source-query {:native "select 1 as `val`, '2' as `source`"}})]
(is (sql= '{:select [source.source AS source
count (*) AS count]
:from [(select 1 as val "2" as source)
source]
:group-by [source]
:order-by [source ASC]}
query))
(mt/with-native-query-testing-context query
(is (= [["2" 1]]
(mt/rows (qp/process-query query)))))))))
test/metabase/query_processor_test/explicit_joins_test.clj
+ 39
− 36
View file @ dad00648
......@@ -690,39 +690,42 @@
(qp/process-query query))))))))))
(deftest join-against-multiple-saved-questions-with-same-column-test
(testing "Should be able to join multiple against saved questions on the same column (#15863)"
(mt/dataset sample-dataset
(let [q1 (mt/mbql-query products {:breakout [$category], :aggregation [[:count]]})
q2 (mt/mbql-query products {:breakout [$category], :aggregation [[:sum $price]]})
q3 (mt/mbql-query products {:breakout [$category], :aggregation [[:avg $rating]]})
metadata (fn [query]
{:post [(some? %)]}
(-> query qp/process-query :data :results_metadata :columns))
query-card (fn [query]
{:dataset_query query, :result_metadata (metadata query)})]
(mt/with-temp* [Card [{card-1-id :id} (query-card q1)]
Card [{card-2-id :id} (query-card q2)]
Card [{card-3-id :id} (query-card q3)]]
(let [query (mt/mbql-query products
{:source-table (format "card__%d" card-1-id)
:joins [{:fields :all
:source-table (format "card__%d" card-2-id)
:condition [:=
$category
&Q2.category]
:alias "Q2"}
{:fields :all
:source-table (format "card__%d" card-3-id)
:condition [:=
$category
&Q3.category]
:alias "Q3"}]})]
(mt/with-native-query-testing-context query
(let [results (qp/process-query query)]
(is (= ["Category" "Count" "Q2 → Category" "Q2 → Sum" "Q3 → Category" "Q3 → Avg"]
(map :display_name (get-in results [:data :results_metadata :columns]))))
(is (= [["Doohickey" 42 "Doohickey" 2185.89 "Doohickey" 3.73]
["Gadget" 53 "Gadget" 3019.2 "Gadget" 3.43]
["Gizmo" 51 "Gizmo" 2834.88 "Gizmo" 3.64]
["Widget" 54 "Widget" 3109.31 "Widget" 3.15]]
(mt/formatted-rows [str int str 2.0 str 2.0] results)))))))))))
(testing "Should be able to join multiple against saved questions on the same column (#15863, #20362, #20413)"
(mt/test-drivers (mt/normal-drivers-with-feature :nested-queries :left-join)
(mt/dataset sample-dataset
(let [q1 (mt/mbql-query products {:breakout [$category], :aggregation [[:count]]})
q2 (mt/mbql-query products {:breakout [$category], :aggregation [[:sum $price]]})
q3 (mt/mbql-query products {:breakout [$category], :aggregation [[:avg $rating]]})
metadata (fn [query]
{:post [(some? %)]}
(-> query qp/process-query :data :results_metadata :columns))
query-card (fn [query]
{:dataset_query query, :result_metadata (metadata query)})]
(mt/with-temp* [Card [{card-1-id :id} (query-card q1)]
Card [{card-2-id :id} (query-card q2)]
Card [{card-3-id :id} (query-card q3)]]
(let [query (mt/mbql-query products
{:source-table (format "card__%d" card-1-id)
:joins [{:fields :all
:source-table (format "card__%d" card-2-id)
:condition [:=
$category
&Q2.category]
:alias "Q2"}
{:fields :all
:source-table (format "card__%d" card-3-id)
:condition [:=
$category
&Q3.category]
:alias "Q3"}]
:order-by [[:asc $category]]})]
(mt/with-native-query-testing-context query
(let [results (qp/process-query query)]
(when (#{:postgres :h2} driver/*driver*)
(is (= ["Category" "Count" "Q2 → Category" "Q2 → Sum" "Q3 → Category" "Q3 → Avg"]
(map :display_name (get-in results [:data :results_metadata :columns])))))
(is (= [["Doohickey" 42 "Doohickey" 2185.89 "Doohickey" 3.73]
["Gadget" 53 "Gadget" 3019.2 "Gadget" 3.43]
["Gizmo" 51 "Gizmo" 2834.88 "Gizmo" 3.64]
["Widget" 54 "Widget" 3109.31 "Widget" 3.15]]
(mt/formatted-rows [str int str 2.0 str 2.0] results))))))))))))
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment