diff --git a/src/metabase/driver/bigquery.clj b/src/metabase/driver/bigquery.clj
index b3292c3f978c740189260166e1d7f91e7fe0c699..2aac92b914f695c4550826bca99125d1dc9b3a0d 100644
--- a/src/metabase/driver/bigquery.clj
+++ b/src/metabase/driver/bigquery.clj
@@ -23,6 +23,7 @@
[metabase.driver.generic-sql.util.unprepare :as unprepare]
[metabase.query-processor
[annotate :as annotate]
+ [store :as qp.store]
[util :as qputil]]
[metabase.util
[date :as du]
@@ -468,24 +469,25 @@
(defn apply-source-table
"Copy of the Generic SQL implementation of `apply-source-table` that prepends the current dataset ID to the table
name."
- [honeysql-form {{table-name :name} :source-table}]
- {:pre [(seq table-name)]}
- (h/from honeysql-form (map->BigQueryIdentifier {:table-name table-name})))
+ [honeysql-form {source-table-id :source-table}]
+ (let [{table-name :name} (qp.store/table source-table-id)]
+ (h/from honeysql-form (map->BigQueryIdentifier {:table-name table-name}))))
(defn- apply-join-tables
"Copy of the Generic SQL implementation of `apply-join-tables`, but prepends the current dataset ID to join-alias."
- [honeysql-form {join-tables :join-tables, {source-table-name :name} :source-table}]
- (loop [honeysql-form honeysql-form, [{:keys [table-name pk-field source-field join-alias]} & more] join-tables]
- (let [honeysql-form
- (h/merge-left-join honeysql-form
- [(map->BigQueryIdentifier {:table-name table-name})
- (map->BigQueryIdentifier {:table-name join-alias})]
- [:=
- (map->BigQueryIdentifier {:table-name source-table-name, :field-name (:field-name source-field)})
- (map->BigQueryIdentifier {:table-name join-alias, :field-name (:field-name pk-field)})])]
- (if (seq more)
- (recur honeysql-form more)
- honeysql-form))))
+ [honeysql-form {join-tables :join-tables, source-table-id :source-table}]
+ (let [{source-table-name :name} (qp.store/table source-table-id)]
+ (loop [honeysql-form honeysql-form, [{:keys [table-name pk-field source-field join-alias]} & more] join-tables]
+ (let [honeysql-form
+ (h/merge-left-join honeysql-form
+ [(map->BigQueryIdentifier {:table-name table-name})
+ (map->BigQueryIdentifier {:table-name join-alias})]
+ [:=
+ (map->BigQueryIdentifier {:table-name source-table-name, :field-name (:field-name source-field)})
+ (map->BigQueryIdentifier {:table-name join-alias, :field-name (:field-name pk-field)})])]
+ (if (seq more)
+ (recur honeysql-form more)
+ honeysql-form)))))
(defn- apply-order-by [driver honeysql-form {subclauses :order-by}]
(loop [honeysql-form honeysql-form, [{:keys [field direction]} & more] subclauses]
@@ -515,10 +517,11 @@
* Incldues `table-name` in the resulting map (do not remember why we are doing so, perhaps it is needed to run the
query)"
[{{{:keys [dataset-id]} :details, :as database} :database
- {{table-name :name} :source-table} :query
+ {source-table-id :source-table} :query
:as outer-query}]
- {:pre [(map? database) (seq dataset-id) (seq table-name)]}
- (let [aliased-query (pre-alias-aggregations outer-query)]
+ {:pre [(map? database) (seq dataset-id)]}
+ (let [aliased-query (pre-alias-aggregations outer-query)
+ {table-name :name} (qp.store/table source-table-id)]
(binding [sqlqp/*query* aliased-query]
{:query (->> aliased-query
(sqlqp/build-honeysql-form bq-driver)
diff --git a/src/metabase/driver/druid/query_processor.clj b/src/metabase/driver/druid/query_processor.clj
index 1f0ebff8d8f7a6216d95ec36553caab3c0d8d26d..8bf38aa489e5d5ef5264611d5e1b98d2578981b7 100644
--- a/src/metabase/driver/druid/query_processor.clj
+++ b/src/metabase/driver/druid/query_processor.clj
@@ -11,6 +11,7 @@
[metabase.driver.druid.js :as js]
[metabase.query-processor
[annotate :as annotate]
+ [store :as qp.store]
[interface :as i]]
[metabase.util :as u]
[metabase.util.date :as du])
@@ -102,10 +103,9 @@
;;; ### handle-source-table
-(defn- handle-source-table [_ {{source-table-name :name} :source-table} query-context]
- {:pre [(or (string? source-table-name)
- (keyword? source-table-name))]}
- (assoc-in query-context [:query :dataSource] source-table-name))
+(defn- handle-source-table [_ {source-table-id :source-table} query-context]
+ (let [{source-table-name :name} (qp.store/table source-table-id)]
+ (assoc-in query-context [:query :dataSource] source-table-name)))
;;; ### handle-aggregation
diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj
index cc790d658475af4e400a5f74f7547acb4c56af42..d048a8597efaf6bfc9a927dbd0c7bb0f87d655c0 100644
--- a/src/metabase/driver/generic_sql/query_processor.clj
+++ b/src/metabase/driver/generic_sql/query_processor.clj
@@ -10,9 +10,11 @@
[driver :as driver]
[util :as u]]
[metabase.driver.generic-sql :as sql]
+ [metabase.mbql.util :as mbql.u]
[metabase.query-processor
[annotate :as annotate]
[interface :as i]
+ [store :as qp.store]
[util :as qputil]]
[metabase.util
[date :as du]
@@ -23,6 +25,7 @@
[metabase.query_processor.interface AgFieldRef BinnedField DateTimeField DateTimeValue Expression
ExpressionRef Field FieldLiteral JoinQuery JoinTable RelativeDateTimeValue TimeField TimeValue Value]))
+;; TODO - yet another `*query*` dynamic var. We should really consolidate them all so we only need a single one.
(def ^:dynamic *query*
"The outer query currently being processed."
nil)
@@ -283,9 +286,11 @@
"Returns a seq of honeysql join clauses, joining to `table-or-query-expr`. `jt-or-jq` can be either a `JoinTable` or
a `JoinQuery`"
[table-or-query-expr {:keys [table-name pk-field source-field schema join-alias] :as jt-or-jq}]
- (let [{{source-table-name :name, source-schema :schema} :source-table} *query*]
+ (let [source-table-id (mbql.u/query->source-table-id *query*)
+ {source-table-name :name, source-schema :schema} (qp.store/table source-table-id)]
[[table-or-query-expr (keyword join-alias)]
- [:= (hx/qualify-and-escape-dots source-schema source-table-name (:field-name source-field))
+ [:=
+ (hx/qualify-and-escape-dots source-schema source-table-name (:field-name source-field))
(hx/qualify-and-escape-dots join-alias (:field-name pk-field))]]))
(defmethod ->honeysql [Object JoinTable]
@@ -331,9 +336,9 @@
(defn apply-source-table
"Apply `source-table` clause to `honeysql-form`. Default implementation of `apply-source-table` for SQL drivers.
Override as needed."
- [_ honeysql-form {{table-name :name, schema :schema} :source-table}]
- {:pre [(seq table-name)]}
- (h/from honeysql-form (hx/qualify-and-escape-dots schema table-name)))
+ [_ honeysql-form {source-table-id :source-table}]
+ (let [{table-name :name, schema :schema} (qp.store/table source-table-id)]
+ (h/from honeysql-form (hx/qualify-and-escape-dots schema table-name))))
(declare apply-clauses)
diff --git a/src/metabase/driver/googleanalytics/query_processor.clj b/src/metabase/driver/googleanalytics/query_processor.clj
index 07c6aaca94fdca7f26292a8488876cf7f6751faf..9aa43275839f328f83879ebfe8de6aa976f28d7e 100644
--- a/src/metabase/driver/googleanalytics/query_processor.clj
+++ b/src/metabase/driver/googleanalytics/query_processor.clj
@@ -4,7 +4,9 @@
(:require [clojure.string :as s]
[clojure.tools.reader.edn :as edn]
[medley.core :as m]
- [metabase.query-processor.util :as qputil]
+ [metabase.query-processor
+ [store :as qp.store]
+ [util :as qputil]]
[metabase.util :as u]
[metabase.util.date :as du])
(:import [com.google.api.services.analytics.model GaData GaData$ColumnHeaders]
@@ -58,9 +60,9 @@
;;; ### source-table
-(defn- handle-source-table [{{source-table-name :name} :source-table}]
- {:pre [((some-fn keyword? string?) source-table-name)]}
- {:ids (str "ga:" source-table-name)})
+(defn- handle-source-table [{source-table-id :source-table}]
+ (let [{source-table-name :name} (qp.store/table source-table-id)]
+ {:ids (str "ga:" source-table-name)}))
;;; ### breakout
diff --git a/src/metabase/driver/mongo/query_processor.clj b/src/metabase/driver/mongo/query_processor.clj
index 444774ddccc7a0eec2529d77938a212c71397de7..6f5e0c1b0b9fccff154ad188961d4aeec5998777 100644
--- a/src/metabase/driver/mongo/query_processor.clj
+++ b/src/metabase/driver/mongo/query_processor.clj
@@ -11,7 +11,8 @@
[metabase.driver.mongo.util :refer [*mongo-connection*]]
[metabase.query-processor
[annotate :as annotate]
- [interface :as i]]
+ [interface :as i]
+ [store :as qp.store]]
[metabase.util :as u]
[metabase.util.date :as du]
[monger
@@ -19,8 +20,7 @@
[operators :refer :all]])
(:import java.sql.Timestamp
[java.util Date TimeZone]
- [metabase.query_processor.interface AgFieldRef DateTimeField DateTimeValue Field FieldLiteral
- RelativeDateTimeValue Value]
+ [metabase.query_processor.interface AgFieldRef DateTimeField DateTimeValue Field FieldLiteral RelativeDateTimeValue Value]
org.bson.types.ObjectId
org.joda.time.DateTime))
@@ -37,6 +37,7 @@
;; # DRIVER QP INTERFACE
+;; TODO - We already have a *query* dynamic var in metabase.query-processor.interface. Do we need this one too?
(def ^:dynamic ^:private *query* nil)
(defn- log-monger-form [form]
@@ -505,16 +506,16 @@
(defn mbql->native
"Process and run an MBQL query."
- [{database :database, {{source-table-name :name} :source-table} :query, :as query}]
- {:pre [(map? database)
- (string? source-table-name)]}
- (binding [*query* query]
- (let [{proj :projections, generated-pipeline :query} (generate-aggregation-pipeline (:query query))]
- (log-monger-form generated-pipeline)
- {:projections proj
- :query generated-pipeline
- :collection source-table-name
- :mbql? true})))
+ [{database :database, {source-table-id :source-table} :query, :as query}]
+ {:pre [(map? database)]}
+ (let [{source-table-name :name} (qp.store/table source-table-id)]
+ (binding [*query* query]
+ (let [{proj :projections, generated-pipeline :query} (generate-aggregation-pipeline (:query query))]
+ (log-monger-form generated-pipeline)
+ {:projections proj
+ :query generated-pipeline
+ :collection source-table-name
+ :mbql? true}))))
(defn execute-query
"Process and run a native MongoDB query."
diff --git a/src/metabase/driver/sparksql.clj b/src/metabase/driver/sparksql.clj
index 30287b96a4a398c4f6b6f5acd6e933096b1cb3cc..75486d54fac0a0ddd5742d1c22bb4cbe6c2b08fd 100644
--- a/src/metabase/driver/sparksql.clj
+++ b/src/metabase/driver/sparksql.clj
@@ -15,8 +15,10 @@
[generic-sql :as sql]
[hive-like :as hive-like]]
[metabase.driver.generic-sql.query-processor :as sqlqp]
- [metabase.models.table :refer [Table]]
- [metabase.query-processor.util :as qputil]
+ [metabase.mbql.util :as mbql.u]
+ [metabase.query-processor
+ [store :as qp.store]
+ [util :as qputil]]
[metabase.util
[honeysql-extensions :as hx]
[i18n :refer [trs tru]]])
@@ -34,13 +36,8 @@
(def ^:private source-table-alias "t1")
-(defn- find-source-table [query]
- (first (qputil/postwalk-collect #(instance? (type Table) %)
- identity
- query)))
-
(defn- resolve-table-alias [{:keys [schema-name table-name special-type field-name] :as field}]
- (let [source-table (find-source-table sqlqp/*query*)]
+ (let [source-table (qp.store/table (mbql.u/query->source-table-id sqlqp/*query*))]
(if (and (= schema-name (:schema source-table))
(= table-name (:name source-table)))
(-> (assoc field :schema-name nil)
@@ -63,12 +60,12 @@
:else field)))
(defn- apply-join-tables
- [honeysql-form {join-tables :join-tables, {source-table-name :name, source-schema :schema} :source-table}]
+ [honeysql-form {join-tables :join-tables}]
(loop [honeysql-form honeysql-form, [{:keys [table-name pk-field source-field schema join-alias]} & more] join-tables]
(let [honeysql-form (h/merge-left-join honeysql-form
[(hx/qualify-and-escape-dots schema table-name) (keyword join-alias)]
[:= (hx/qualify-and-escape-dots source-table-alias (:field-name source-field))
- (hx/qualify-and-escape-dots join-alias (:field-name pk-field))])]
+ (hx/qualify-and-escape-dots join-alias (:field-name pk-field))])]
(if (seq more)
(recur honeysql-form more)
honeysql-form))))
@@ -91,9 +88,9 @@
(h/limit items))))))
(defn- apply-source-table
- [honeysql-form {{table-name :name, schema :schema} :source-table}]
- {:pre [table-name]}
- (h/from honeysql-form [(hx/qualify-and-escape-dots schema table-name) source-table-alias]))
+ [honeysql-form {source-table-id :source-table}]
+ (let [{table-name :name, schema :schema} (qp.store/table source-table-id)]
+ (h/from honeysql-form [(hx/qualify-and-escape-dots schema table-name) source-table-alias])))
;;; ------------------------------------------- Other Driver Method Impls --------------------------------------------
diff --git a/src/metabase/events/activity_feed.clj b/src/metabase/events/activity_feed.clj
index c7cd977d409b738d17db3eb59d173ff2175503af..0b6aaae0db20e824436ad40b07a6003dc87296a2 100644
--- a/src/metabase/events/activity_feed.clj
+++ b/src/metabase/events/activity_feed.clj
@@ -5,6 +5,7 @@
[events :as events]
[query-processor :as qp]
[util :as u]]
+ [metabase.mbql.util :as mbql.u]
[metabase.models
[activity :as activity :refer [Activity]]
[card :refer [Card]]
@@ -41,20 +42,12 @@
;;; ------------------------------------------------ EVENT PROCESSING ------------------------------------------------
-(defn- inner-query->source-table-id
- "Recurse through INNER-QUERY source-queries as needed until we can return the ID of this query's source-table."
- [{:keys [source-table source-query]}]
- (or (when source-table
- (u/get-id source-table))
- (when source-query
- (recur source-query))))
-
(defn- process-card-activity! [topic object]
(let [details-fn #(select-keys % [:name :description])
query (u/ignore-exceptions (qp/expand (:dataset_query object)))
database-id (when-let [database (:database query)]
(u/get-id database))
- table-id (inner-query->source-table-id (:query query))]
+ table-id (mbql.u/query->source-table-id query)]
(activity/record-activity!
:topic topic
:object object
diff --git a/src/metabase/mbql/util.clj b/src/metabase/mbql/util.clj
index 7ee6f40756b0a9512ac926bada7c07e361fd1bc3..cf281a610fdd950ec1bd682e9005b426acc9c178 100644
--- a/src/metabase/mbql/util.clj
+++ b/src/metabase/mbql/util.clj
@@ -140,3 +140,25 @@
(if-not new-clause
outer-query
(update-in outer-query [:query :filter] combine-filter-clauses new-clause)))
+
+
+(defn query->source-table-id
+ "Return the source Table ID associated with `query`, if applicable; handles nested queries as well."
+ {:argslists '([outer-query])}
+ [{{source-table-id :source-table, source-query :source-query} :query, query-type :type, :as query}]
+ (cond
+ ;; for native queries, there's no source table to resolve
+ (not= query-type :query)
+ nil
+
+ ;; for MBQL queries with a *native* source query, it's the same story
+ (and (nil? source-table-id) source-query (:native source-query))
+ nil
+
+ ;; for MBQL queries with an MBQL source query, recurse on the source query and try again
+ (and (nil? source-table-id) source-query)
+ (recur (assoc query :query source-query))
+
+ ;; otherwise resolve the source Table
+ :else
+ source-table-id))
diff --git a/src/metabase/models/query/permissions.clj b/src/metabase/models/query/permissions.clj
index e8f7f5896d8eb995c45e72ba09a077070066c296..7b68dc1b750b8406084e4fa3d80a28e95d9bf084 100644
--- a/src/metabase/models/query/permissions.clj
+++ b/src/metabase/models/query/permissions.clj
@@ -5,7 +5,8 @@
(:require [clojure.tools.logging :as log]
[metabase.models
[interface :as i]
- [permissions :as perms]]
+ [permissions :as perms]
+ [table :refer [Table]]]
[metabase.query-processor.util :as qputil]
[metabase.util :as u]
[metabase.util
@@ -32,7 +33,7 @@
;; tables->permissions-path-set source-card-read-perms
(defn- query->source-and-join-tables
- "Return a sequence of all Tables (as TableInstance maps) referenced by QUERY."
+ "Return a sequence of all Tables (as TableInstance maps, or IDs) referenced by `query`."
[{:keys [source-table join-tables source-query native], :as query}]
(cond
;; if we come across a native query just put a placeholder (`::native`) there so we know we need to add native
@@ -46,14 +47,24 @@
(s/defn ^:private tables->permissions-path-set :- #{perms/ObjectPath}
"Given a sequence of `tables` referenced by a query, return a set of required permissions."
[database-or-id tables]
- (set (for [table tables]
- (if (= ::native table)
- ;; Any `::native` placeholders from above mean we need native ad-hoc query permissions for this DATABASE
- (perms/adhoc-native-query-path database-or-id)
- ;; anything else (i.e., a normal table) just gets normal table permissions
- (perms/object-path (u/get-id database-or-id)
- (:schema table)
- (or (:id table) (:table-id table)))))))
+ (let [table-ids (filter integer? tables)
+ table-id->schema (when (seq table-ids)
+ (db/select-id->field :schema Table :id [:in table-ids]))]
+ (set (for [table tables]
+ (cond
+ ;; Any `::native` placeholders from above mean we need native ad-hoc query permissions for this DATABASE
+ (= ::native table)
+ (perms/adhoc-native-query-path database-or-id)
+
+ ;; If Table is an ID then fetch its schema from the DB and require normal table perms
+ (integer? table)
+ (perms/object-path (u/get-id database-or-id) (table-id->schema table) table)
+
+ ;; for a TableInstance require normal table perms
+ :else
+ (perms/object-path (u/get-id database-or-id)
+ (:schema table)
+ (or (:id table) (:table-id table))))))))
(s/defn ^:private source-card-read-perms :- #{perms/ObjectPath}
"Calculate the permissions needed to run an ad-hoc query that uses a Card with `source-card-id` as its source
diff --git a/src/metabase/query_processor.clj b/src/metabase/query_processor.clj
index 67733ddef24d7ce23f5b549b8faf08c18bff7416..1445e3d0f778a12bcb066f6625a93f3844a34eb1 100644
--- a/src/metabase/query_processor.clj
+++ b/src/metabase/query_processor.clj
@@ -36,6 +36,7 @@
[resolve-driver :as resolve-driver]
[results-metadata :as results-metadata]
[source-table :as source-table]
+ [store :as store]
[validate :as validate]]
[metabase.query-processor.util :as qputil]
[metabase.util
@@ -94,7 +95,6 @@
mbql-to-native/mbql->native ; ▲▲▲ NATIVE-ONLY POINT ▲▲▲ Query converted from MBQL to native here; all functions *above* will only see the native query
annotate-and-sort/annotate-and-sort
perms/check-query-permissions
- log-query/log-expanded-query
dev/check-results-format
limit/limit
cumulative-ags/handle-cumulative-aggregations
@@ -104,8 +104,8 @@
resolve/resolve-middleware
add-dim/add-remapping
implicit-clauses/add-implicit-clauses
- source-table/resolve-source-table-middleware
expand/expand-middleware ; ▲▲▲ QUERY EXPANSION POINT ▲▲▲ All functions *above* will see EXPANDED query during PRE-PROCESSING
+ source-table/resolve-source-table-middleware
row-count-and-status/add-row-count-and-status ; ▼▼▼ RESULTS WRAPPING POINT ▼▼▼ All functions *below* will see results WRAPPED in `:data` during POST-PROCESSING
parameters/substitute-parameters
expand-macros/expand-macros
@@ -114,7 +114,9 @@
resolve-driver/resolve-driver ; ▲▲▲ DRIVER RESOLUTION POINT ▲▲▲ All functions *above* will have access to the driver during PRE- *and* POST-PROCESSING
bind-timezone/bind-effective-timezone
fetch-source-query/fetch-source-query
+ store/initialize-store
log-query/log-initial-query
+ ;; TODO - bind *query* here ?
cache/maybe-return-cached-results
log-query/log-results-metadata
validate/validate-query
diff --git a/src/metabase/query_processor/interface.clj b/src/metabase/query_processor/interface.clj
index 4c5b1518c58bf822b2e53d5222a4a3931ff2a3f5..e6630b4fa0eb30beb96d33704afdc11fb9e10c62 100644
--- a/src/metabase/query_processor/interface.clj
+++ b/src/metabase/query_processor/interface.clj
@@ -38,6 +38,9 @@
Not neccesarily bound when using various functions like `fk->` in the REPL."
nil)
+
+;;; ------------------------------------------------------ ETC -------------------------------------------------------
+
(defn ^:deprecated driver-supports?
"Does the currently bound `*driver*` support FEATURE?
(This returns `nil` if `*driver*` is unbound. `*driver*` is always bound when running queries the normal way,
diff --git a/src/metabase/query_processor/middleware/add_implicit_clauses.clj b/src/metabase/query_processor/middleware/add_implicit_clauses.clj
index 820409c2e09ae0d7f7cf88ba86f7418f1a0c51eb..8f76c283f373f023100fc88d3ddb60840208eefa 100644
--- a/src/metabase/query_processor/middleware/add_implicit_clauses.clj
+++ b/src/metabase/query_processor/middleware/add_implicit_clauses.clj
@@ -5,6 +5,7 @@
[metabase.query-processor
[interface :as i]
[sort :as sort]
+ [store :as qp.store]
[util :as qputil]]
[metabase.query-processor.middleware.resolve :as resolve]
[toucan
@@ -26,16 +27,17 @@
(defn- fields-for-source-table
"Return the all fields for SOURCE-TABLE, for use as an implicit `:fields` clause."
- [{{source-table-id :id, :as source-table} :source-table, :as inner-query}]
- ;; Sort the implicit FIELDS so the SQL (or other native query) that gets generated (mostly) approximates the 'magic' sorting
- ;; we do on the results. This is done so when the outer query we generate is a `SELECT *` the order doesn't change
- (for [field (sort/sort-fields inner-query (fetch-fields-for-souce-table-id source-table-id))
- :let [field (-> field
- resolve/convert-db-field
- (resolve/resolve-table {[nil source-table-id] source-table}))]]
- (if (qputil/datetime-field? field)
- (i/map->DateTimeField {:field field, :unit :default})
- field)))
+ [{source-table-id :source-table, :as inner-query}]
+ (let [{source-table-id :id, :as source-table} (qp.store/table source-table-id)]
+ ;; Sort the implicit FIELDS so the SQL (or other native query) that gets generated (mostly) approximates the 'magic' sorting
+ ;; we do on the results. This is done so when the outer query we generate is a `SELECT *` the order doesn't change
+ (for [field (sort/sort-fields inner-query (fetch-fields-for-souce-table-id source-table-id))
+ :let [field (-> field
+ resolve/convert-db-field
+ (resolve/resolve-table {[nil source-table-id] source-table}))]]
+ (if (qputil/datetime-field? field)
+ (i/map->DateTimeField {:field field, :unit :default})
+ field))))
(defn- should-add-implicit-fields? [{:keys [fields breakout source-table], aggregations :aggregation}]
(and source-table ; if query is using another query as its source then there will be no table to add nested fields for
diff --git a/src/metabase/query_processor/middleware/log.clj b/src/metabase/query_processor/middleware/log.clj
index 183668ba9705e625ba3cd1ae21c80d3a404efdc1..10d437d48823c021b9bb13cc18d928ed7c83a662 100644
--- a/src/metabase/query_processor/middleware/log.clj
+++ b/src/metabase/query_processor/middleware/log.clj
@@ -2,36 +2,10 @@
"Middleware for logging a query before it is processed.
(Various other middleware functions log the query as well in different stages.)"
(:require [clojure.tools.logging :as log]
- [clojure.walk :as walk]
[medley.core :as m]
- [metabase.query-processor
- [interface :as i]
- [util :as qputil]]
+ [metabase.query-processor.interface :as i]
[metabase.util :as u]))
-(defn- log-expanded-query* [query]
- (u/prog1 query
- (when (and (qputil/mbql-query? query)
- (not i/*disable-qp-logging*))
- (log/debug (u/format-color 'magenta "\nPREPROCESSED/EXPANDED: %s\n%s"
- (u/emoji "😻")
- (u/pprint-to-str
- ;; Remove empty kv pairs because otherwise expanded query is HUGE
- (walk/prewalk
- (fn [f]
- (if-not (map? f) f
- (m/filter-vals (complement nil?) (into {} f))))
- ;; obscure DB details when logging. Just log the name of driver because we don't care about its properties
- (-> query
- (assoc-in [:database :details] (u/emoji "😋 ")) ; :yum:
- (update :driver name)))))))))
-
-(defn log-expanded-query
- "Middleware for logging a query after it is expanded, but before it is processed."
- [qp]
- (comp qp log-expanded-query*))
-
-
(defn- log-initial-query* [query]
(u/prog1 query
(when-not i/*disable-qp-logging*
diff --git a/src/metabase/query_processor/middleware/mbql_to_native.clj b/src/metabase/query_processor/middleware/mbql_to_native.clj
index 2179256597c55e5526d57a5f2674f7b9a85f5a49..31270a4fc4fd57bff4c84ef9934b6d60d5af6c7c 100644
--- a/src/metabase/query_processor/middleware/mbql_to_native.clj
+++ b/src/metabase/query_processor/middleware/mbql_to_native.clj
@@ -5,14 +5,12 @@
[metabase
[driver :as driver]
[util :as u]]
- [metabase.query-processor
- [interface :as i]
- [util :as qputil]]))
+ [metabase.query-processor.interface :as i]))
(defn- query->native-form
"Return a `:native` query form for QUERY, converting it from MBQL if needed."
- [query]
- (u/prog1 (if-not (qputil/mbql-query? query)
+ [{query-type :type, :as query}]
+ (u/prog1 (if-not (= :query query-type)
(:native query)
(driver/mbql->native (:driver query) query))
(when-not i/*disable-qp-logging*
@@ -22,9 +20,9 @@
"Middleware that handles conversion of MBQL queries to native (by calling driver QP methods) so the queries
can be executed. For queries that are already native, this function is effectively a no-op."
[qp]
- (fn [query]
+ (fn [{query-type :type, :as query}]
(let [native-form (query->native-form query)
- native-query (if-not (qputil/mbql-query? query)
+ native-query (if-not (= query-type :query)
query
(assoc query :native native-form))
results (qp native-query)]
diff --git a/src/metabase/query_processor/middleware/resolve.clj b/src/metabase/query_processor/middleware/resolve.clj
index d26ddb8a57195f9e25d60d092606ed5d238ba178..2663b840855c5a34587f17e600b85f9a397fc8f5 100644
--- a/src/metabase/query_processor/middleware/resolve.clj
+++ b/src/metabase/query_processor/middleware/resolve.clj
@@ -10,7 +10,6 @@
[metabase
[db :as mdb]
[util :as u]]
- [metabase.util.date :as du]
[metabase.models
[database :refer [Database]]
[field :as field]
@@ -18,6 +17,7 @@
[table :refer [Table]]]
[metabase.query-processor
[interface :as i]
+ [store :as qp.store]
[util :as qputil]]
[metabase.util.date :as du]
[schema.core :as s]
@@ -25,8 +25,8 @@
[db :as db]
[hydrate :refer [hydrate]]])
(:import java.util.TimeZone
- [metabase.query_processor.interface DateTimeField DateTimeValue ExpressionRef Field FieldLiteral FieldPlaceholder
- RelativeDatetime RelativeDateTimeValue TimeField TimeValue Value ValuePlaceholder]))
+ [metabase.query_processor.interface DateTimeField DateTimeValue ExpressionRef Field FieldLiteral
+ FieldPlaceholder RelativeDatetime RelativeDateTimeValue TimeField TimeValue Value ValuePlaceholder]))
;;; ---------------------------------------------------- UTIL FNS ----------------------------------------------------
@@ -406,31 +406,32 @@
(if (:native source-query)
expanded-query-dict
(let [ ;; Resolve the nested query as if it were a top level query
- {nested-q :query :as nested-qd} (resolve-tables (assoc expanded-query-dict :query source-query))
- nested-source-table (get-in nested-qd [:query :source-table])
+ {nested-inner :query, :as nested-outer} (resolve-tables (assoc expanded-query-dict :query source-query))
+ nested-source-table-id (:source-table nested-inner)
;; Build a list of join tables found from the newly resolved nested query
- nested-joined-tables (fk-field-ids->joined-tables (:id nested-source-table)
- (:fk-field-ids nested-qd))
+ nested-joined-tables (fk-field-ids->joined-tables nested-source-table-id
+ (:fk-field-ids nested-outer))
;; Create the map of fk to table info from the resolved nested query
- fk-id+table-id->table (create-fk-id+table-id->table nested-source-table nested-joined-tables)
+ fk-id+table-id->table (create-fk-id+table-id->table (some-> nested-source-table-id qp.store/table)
+ nested-joined-tables)
;; Resolve the top level (original) breakout fields with the join information from the resolved nested query
- resolved-breakout (for [breakout (get-in expanded-query-dict [:query :breakout])]
- (resolve-table breakout fk-id+table-id->table))]
+ resolved-breakout (for [breakout (get-in expanded-query-dict [:query :breakout])]
+ (resolve-table breakout fk-id+table-id->table))]
(assoc-in expanded-query-dict [:query :source-query]
- (if (and (contains? nested-q :fields)
+ (if (and (contains? nested-inner :fields)
(seq resolved-breakout))
- (update nested-q :fields append-new-fields resolved-breakout)
- nested-q)))))
+ (update nested-inner :fields append-new-fields resolved-breakout)
+ nested-inner)))))
(defn- resolve-tables
"Resolve the `Tables` in an EXPANDED-QUERY-DICT."
- [{:keys [fk-field-ids], {{source-table-id :id :as source-table} :source-table} :query, :as expanded-query-dict}]
+ [{:keys [fk-field-ids], {source-table-id :source-table} :query, :as expanded-query-dict}]
(if-not source-table-id
;; if we have a `source-query`, recurse and resolve tables in that
(resolve-tables-in-nested-query expanded-query-dict)
;; otherwise we can resolve tables in the (current) top-level
(let [joined-tables (fk-field-ids->joined-tables source-table-id fk-field-ids)
- fk-id+table-id->table (create-fk-id+table-id->table source-table joined-tables)]
+ fk-id+table-id->table (create-fk-id+table-id->table (qp.store/table source-table-id) joined-tables)]
(as-> expanded-query-dict <>
(assoc-in <> [:query :join-tables] joined-tables)
(walk/postwalk #(resolve-table % fk-id+table-id->table) <>)))))
diff --git a/src/metabase/query_processor/middleware/source_table.clj b/src/metabase/query_processor/middleware/source_table.clj
index 262de2f3517519115c03823f9088df780c59d70f..dfd77808c5fe0fb58f691810f97ea3402465bc99 100644
--- a/src/metabase/query_processor/middleware/source_table.clj
+++ b/src/metabase/query_processor/middleware/source_table.clj
@@ -1,25 +1,16 @@
(ns metabase.query-processor.middleware.source-table
- (:require [metabase.models.table :refer [Table]]
+ (:require [metabase.mbql.util :as mbql.u]
+ [metabase.models.table :refer [Table]]
+ [metabase.query-processor.store :as qp.store]
+ [metabase.util.i18n :refer [trs]]
[toucan.db :as db]))
-(defn- resolve-source-table
- [{{source-table-id :source-table} :query, query-type :type, :as expanded-query-dict}]
- (cond
- (not= query-type :query)
- expanded-query-dict
-
- (nil? source-table-id)
- (update-in expanded-query-dict [:query :source-query] (fn [source-query]
- (if (:native source-query)
- source-query
- (:query (resolve-source-table (assoc expanded-query-dict
- :query source-query))))))
-
- :else
+(defn- resolve-source-table [query]
+ (when-let [source-table-id (mbql.u/query->source-table-id query)]
(let [source-table (or (db/select-one [Table :schema :name :id], :id source-table-id)
- (throw (Exception. (format "Query expansion failed: could not find source table %d."
- source-table-id))))]
- (assoc-in expanded-query-dict [:query :source-table] source-table))))
+ (throw (Exception. (str (trs "Cannot run query: could not find source table {0}." source-table-id)))))]
+ (qp.store/store-table! source-table)))
+ query)
(defn resolve-source-table-middleware
"Middleware that will take the source-table (an integer) and hydrate that source table from the the database and
diff --git a/src/metabase/query_processor/middleware/store.clj b/src/metabase/query_processor/middleware/store.clj
new file mode 100644
index 0000000000000000000000000000000000000000..68eb79b74fdfd2517477a0a23cffb4186c8fc092
--- /dev/null
+++ b/src/metabase/query_processor/middleware/store.clj
@@ -0,0 +1,11 @@
+(ns metabase.query-processor.middleware.store
+ "The store middleware is responsible for initializing a fresh QP Store, which caches resolved objects for the duration
+ of a query execution. See `metabase.query-processor.store` for more details."
+ (:require [metabase.query-processor.store :as qp.store]))
+
+(defn initialize-store
+ "Initialize the QP Store (resolved objects cache) for this query execution."
+ [qp]
+ (fn [query]
+ (qp.store/with-store
+ (qp query))))
diff --git a/src/metabase/query_processor/store.clj b/src/metabase/query_processor/store.clj
new file mode 100644
index 0000000000000000000000000000000000000000..2899160499510d4684129c79c2a086f815025f01
--- /dev/null
+++ b/src/metabase/query_processor/store.clj
@@ -0,0 +1,90 @@
+(ns metabase.query-processor.store
+ "The Query Processor Store caches resolved Tables and Fields for the duration of a query execution. Certain middleware
+ handles resolving things like the query's source Table and any Fields that are referenced in a query, and saves the
+ referenced objects in the store; other middleware and driver-specific query processor implementations use functions
+ in the store to fetch those objects as needed.
+
+ For example, a driver might be converting a Field ID clause (e.g. `[:field-id 10]`) to its native query language. It
+ can fetch the underlying Metabase FieldInstance by calling `field`:
+
+ (qp.store/field 10) ;; get Field 10
+
+ Of course, it would be entirely possible to call `(Field 10)` every time you needed information about that Field,
+ but fetching all Fields in a single pass and storing them for reuse is dramatically more efficient than fetching
+ those Fields potentially dozens of times in a single query execution."
+ (:require [schema.core :as s]
+ [metabase.models
+ [field :refer [Field]]
+ [table :refer [Table]]]
+ [metabase.util :as u]
+ [metabase.util.schema :as su]))
+
+;;; ---------------------------------------------- Setting up the Store ----------------------------------------------
+
+(def ^:private ^:dynamic *store*
+ "Dynamic var used as the QP store for a given query execution."
+ (atom nil))
+
+(defn do-with-store
+ "Execute `f` with a freshly-bound `*store*`."
+ [f]
+ (binding [*store* (atom {})]
+ (f)))
+
+(defmacro with-store
+ "Execute `body` with a freshly-bound QP `*store*`. The `store` middleware takes care of setting up a fresh store for
+ each query execution; you should have no need to use this macro yourself outside of that namespace."
+ {:style/indent 0}
+ [& body]
+ `(do-with-store (fn [] ~@body)))
+
+;; TODO - DATABASE ??
+
+(def ^:private TableInstanceWithRequiredStoreKeys
+ "Schema for Tables stored in the QP store (must be a `TableInstance` with at least the `:id`, `:schema`, and `:name`
+ keys)."
+ (s/both
+ (class Table)
+ {:id su/IntGreaterThanZero ; TODO - what's the point of storing ID if it's already the key?
+ :schema (s/maybe s/Str)
+ :name su/NonBlankString
+ s/Any s/Any}))
+
+
+;;; ------------------------------------------ Saving objects in the Store -------------------------------------------
+
+(s/defn store-table!
+ "Store a `table` in the QP Store for the duration of the current query execution. Throws an Exception if Table is
+ invalid or doesn't have all required keys."
+ [table :- TableInstanceWithRequiredStoreKeys]
+ (swap! *store* assoc-in [:tables (u/get-id table)] table))
+
+(s/defn store-field!
+ "Store a `field` in the QP Store for the duration of the current query execution. Throws an Exception if Field is
+ invalid or doesn't have all required keys."
+ [field :- (class Field)]
+ (swap! *store* assoc-in [:fields (u/get-id field)] field))
+
+(s/defn store-field-literal!
+ "Like `store-field!`, but designed for storing Fields referred to by `:field-literal` clauses, i.e. by name instead of
+ by ID."
+ [field-literal :- su/NonBlankString, field :- (class Field)]
+ (swap! *store* assoc-in [:field-literals field-literal] field))
+
+
+;;; ---------------------------------------- Fetching objects from the Store -----------------------------------------
+
+(s/defn table :- TableInstanceWithRequiredStoreKeys
+ "Fetch Table with `table-id` from the QP Store. Throws an Exception if valid item is not returned."
+ [table-id :- su/IntGreaterThanZero]
+ (get-in @*store* [:tables table-id]))
+
+(s/defn field :- (class Field)
+ "Fetch Field with `field-id` from the QP Store. Throws an Exception if valid item is not returned."
+ [field-id :- su/IntGreaterThanZero]
+ (get-in @*store* [:fields field-id]))
+
+(s/defn field-liteal :- (class Field)
+ "Fetch Field named by `field-literal` from the QP Store. Throws an Exception if valid item is not returned."
+ [field-literal :- su/NonBlankString]
+ (get-in @*store* [:field-literals field-literal]))
diff --git a/test/metabase/driver/googleanalytics_test.clj b/test/metabase/driver/googleanalytics_test.clj
index a68299ca2100bd983b077302b0b752edb270e1c7..a81582266b29468dc45a2fbf849ebd40a41b252d 100644
--- a/test/metabase/driver/googleanalytics_test.clj
+++ b/test/metabase/driver/googleanalytics_test.clj
@@ -7,7 +7,9 @@
[database :refer [Database]]
[field :refer [Field]]
[table :refer [Table]]]
- [metabase.query-processor.interface :as qpi]
+ [metabase.query-processor
+ [interface :as qpi]
+ [store :as qp.store]]
[metabase.test.data.users :as users]
[metabase.util :as u]
[metabase.util.date :as du]
@@ -70,7 +72,10 @@
:mbql? true})
(defn- mbql->native [query]
- (qp/mbql->native (update query :query (partial merge {:source-table {:name "0123456"}}))))
+ (binding [qp.store/*store* (atom {:tables {1 #metabase.models.table.TableInstance{:name "0123456"
+ :schema nil
+ :id 1}}})]
+ (qp/mbql->native (update query :query (partial merge {:source-table 1})))))
;; just check that a basic almost-empty MBQL query can be compiled
(expect
diff --git a/test/metabase/query_processor/middleware/fetch_source_query_test.clj b/test/metabase/query_processor/middleware/fetch_source_query_test.clj
index 2eb3b6a050a73673fdafa91408be50fb83740196..a64ef58d9bc5d1fecee1ed83c03c74d9cfa33a6b 100644
--- a/test/metabase/query_processor/middleware/fetch_source_query_test.clj
+++ b/test/metabase/query_processor/middleware/fetch_source_query_test.clj
@@ -66,7 +66,7 @@
;; `fetch-source-query`)
(expect
(default-expanded-results
- {:source-query {:source-table {:schema "PUBLIC", :name "VENUES", :id (data/id :venues)}
+ {:source-query {:source-table (data/id :venues)
:join-tables nil}})
(tt/with-temp Card [card {:dataset_query {:database (data/id)
:type :query
@@ -78,7 +78,8 @@
(expect
(let [date-field-literal {:field-name "date", :base-type :type/Date, :binning-strategy nil, :binning-param nil, :fingerprint nil}]
(default-expanded-results
- {:source-query {:source-table {:schema "PUBLIC" :name "CHECKINS" :id (data/id :checkins)}, :join-tables nil}
+ {:source-query {:source-table (data/id :checkins)
+ :join-tables nil}
:filter {:filter-type :between,
:field date-field-literal
:min-val {:value (tcoerce/to-timestamp (du/str->date-time "2015-01-01"))
@@ -115,7 +116,7 @@
(default-expanded-results
{:limit 25
:source-query {:limit 50
- :source-query {:source-table {:schema "PUBLIC", :name "VENUES", :id (data/id :venues)}
+ :source-query {:source-table (data/id :venues)
:limit 100
:join-tables nil}}})
(tt/with-temp* [Card [card-1 {:dataset_query {:database (data/id)