Skip to content
Snippets Groups Projects
Unverified Commit 0d174622 authored by Tom Robinson's avatar Tom Robinson Committed by GitHub
Browse files

Fix `query->database-and-table-ids` for `source-query` (#11760)

* Fix logic for getting card database/table when using nested queries. Resolves #11496

* Add tests for fix :wrench:



Co-authored-by: default avatarCam Saul <1455846+camsaul@users.noreply.github.com>
parent fcb3e2e1
Branches
Tags
No related merge requests found
......@@ -78,21 +78,20 @@
;; rethrow e if updating an existing average execution time failed
(throw e))))))
;; TODO - somewhat confusing that the Ad-hoc queries here use the keys `:table-id` and `:database-id` instead of the
;; `:database_id` and `:table_id` that come out of the Database. In the automagic dashboards code, for example, we
;; have special utility functions to account for both possiblities. Should we fix this?
(defn query->database-and-table-ids
"Return a map with `:database-id` and source `:table-id` that should be saved for a Card. Handles queries that use
other queries as their source (ones that come in with a `:source-table` like `card__100`) recursively, as well as
normal queries."
[{database-id :database, query-type :type, {:keys [source-table]} :query}]
other queries as their source (ones that come in with a `:source-table` like `card__100`, or `:source-query`)
recursively, as well as normal queries."
[{database-id :database, query-type :type, {:keys [source-table source-query]} :query}]
(cond
(= :native query-type) {:database-id database-id, :table-id nil}
(integer? source-table) {:database-id database-id, :table-id source-table}
(string? source-table) (let [[_ card-id] (re-find #"^card__(\d+)$" source-table)]
(db/select-one ['Card [:table_id :table-id] [:database_id :database-id]]
:id (Integer/parseInt card-id)))))
:id (Integer/parseInt card-id)))
(map? source-query) (query->database-and-table-ids {:database database-id
:type query-type
:query source-query})))
(defn adhoc-query
"Wrap query map into a Query object (mostly to fascilitate type dispatch)."
......
(ns metabase.models.query-test
(:require [clojure.test :refer :all]
[metabase
[models :refer [Card]]
[test :as mt]]
[metabase.models.query :as query]))
(deftest query->database-and-table-ids-test
(mt/with-temp Card [card {:dataset_query {:database (mt/id)
:type :query
:query {:source-table (mt/id :venues)}}}]
(doseq [[message {:keys [expected query]}]
{"A basic query"
{:expected {:database-id 1, :table-id 1}
:query {:database 1
:type :query
:query {:source-table 1}}}
"For native queries, table-id should be nil"
{:expected {:database-id 1, :table-id nil}
:query {:database 1
:type :native
:native {:query "SELECT * FROM some_table;"}}}
"If the query has a card__id source table, we should fetch database and table ID from the Card"
{:expected {:database-id (mt/id)
:table-id (mt/id :venues)}
:query {:database 1000
:type :query
:query {:source-table (format "card__%d" (:id card))}}}
"If the query has a source-query we should recursively look at the database/table ID of the source query"
{:expected {:database-id 5, :table-id 6}
:query {:database 5
:type :query
:query {:source-query {:source-table 6}}}}}]
(testing message
(is (= expected
(into {} (query/query->database-and-table-ids query))))))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment